<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Tue, Jun 20, 2017 at 2:06 PM Lang Hames via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: lhames<br>
Date: Tue Jun 20 16:06:00 2017<br>
New Revision: 305850<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=305850&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=305850&view=rev</a><br>
Log:<br>
Preserve CXX method overrides in ASTImporter<br>
<br>
Summary:<br>
The ASTImporter should import CXX method overrides from the source context<br>
when it imports a method decl.<br>
<br>
Reviewers: spyffe, rsmith, doug.gregor<br>
<br>
Reviewed By: spyffe<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D34371" rel="noreferrer" target="_blank">https://reviews.llvm.org/D34371</a><br>
<br>
Modified:<br>
  Â  cfe/trunk/lib/AST/ASTDumper.cpp<br>
  Â  cfe/trunk/lib/AST/ASTImporter.cpp<br>
  Â  cfe/trunk/tools/clang-import-test/clang-import-test.cpp<br>
<br>
Modified: cfe/trunk/lib/AST/ASTDumper.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDumper.cpp?rev=305850&r1=305849&r2=305850&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDumper.cpp?rev=305850&r1=305849&r2=305850&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/AST/ASTDumper.cpp (original)<br>
+++ cfe/trunk/lib/AST/ASTDumper.cpp Tue Jun 20 16:06:00 2017<br>
@@ -1184,6 +1184,28 @@ void ASTDumper::VisitFunctionDecl(const<br>
  Â  Â  Â  Â  I != E; ++I)<br>
  Â  Â  Â dumpCXXCtorInitializer(*I);<br>
<br>
+  if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(D))<br>
+  Â  if (MD->size_overridden_methods() != 0) {<br></blockquote><div><br>I'd probably make sure CXXMethodDecl has a range-based accessor for overridden_methods and call it once here, check begin != end, then use it in the range-based for later. That way it'd avoid multiple lookups of the CXXMethodDecl in the ASTContext.<br><br>Probably not a big deal/efficiency concern, but just a thought :)<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+  Â  Â  auto dumpOverride =<br>
+  Â  Â  Â  [=](const CXXMethodDecl *D) {<br>
+  Â  Â  Â  Â  SplitQualType T_split = D->getType().split();<br>
+  Â  Â  Â  Â  OS << D << " " << D->getParent()->getName() << "::"<br>
+  Â  Â  Â  Â  Â  Â << D->getName() << " '"<br>
+  Â  Â  Â  Â  Â  Â << QualType::getAsString(T_split) << "'";<br>
+  Â  Â  Â  };<br>
+<br>
+  Â  Â  dumpChild([=] {<br>
+  Â  Â  Â  auto FirstOverrideItr = MD->begin_overridden_methods();<br>
+  Â  Â  Â  OS << "Overrides: [ ";<br>
+  Â  Â  Â  dumpOverride(*FirstOverrideItr);<br></blockquote><div><br>Why is this one ^ pulled out separately from the rest of the loop?<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+  Â  Â  Â  for (const auto *Override :<br>
+  Â  Â  Â  Â  Â  Â  Â llvm::make_range(FirstOverrideItr + 1,<br>
+  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  MD->end_overridden_methods()))<br>
+  Â  Â  Â  Â  dumpOverride(Override);<br>
+  Â  Â  Â  OS << " ]";<br>
+  Â  Â  });<br>
+  Â  }<br>
+<br>
  Â if (D->doesThisDeclarationHaveABody())<br>
  Â  Â dumpStmt(D->getBody());<br>
 }<br>
<br>
Modified: cfe/trunk/lib/AST/ASTImporter.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=305850&r1=305849&r2=305850&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=305850&r1=305849&r2=305850&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)<br>
+++ cfe/trunk/lib/AST/ASTImporter.cpp Tue Jun 20 16:06:00 2017<br>
@@ -319,6 +319,9 @@ namespace clang {<br>
  Â  Â bool ImportArrayChecked(const InContainerTy &InContainer, OIter Obegin) {<br>
  Â  Â  Â return ImportArrayChecked(InContainer.begin(), InContainer.end(), Obegin);<br>
  Â  Â }<br>
+<br>
+  Â  // Importing overrides.<br>
+  Â  void ImportOverrides(CXXMethodDecl *ToMethod, CXXMethodDecl *FromMethod);<br>
  Â };<br>
 }<br>
<br>
@@ -2025,6 +2028,9 @@ Decl *ASTNodeImporter::VisitFunctionDecl<br>
  Â // Add this function to the lexical context.<br>
  Â LexicalDC->addDeclInternal(ToFunction);<br>
<br>
+  if (auto *FromCXXMethod = dyn_cast<CXXMethodDecl>(D))<br>
+  Â  ImportOverrides(cast<CXXMethodDecl>(ToFunction), FromCXXMethod);<br>
+<br>
  Â return ToFunction;<br>
 }<br>
<br>
@@ -5499,6 +5505,14 @@ Expr *ASTNodeImporter::VisitSubstNonType<br>
  Â  Â  Â  Â Replacement);<br>
 }<br>
<br>
+void ASTNodeImporter::ImportOverrides(CXXMethodDecl *ToMethod,<br>
+  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  CXXMethodDecl *FromMethod) {<br>
+  for (auto *FromOverriddenMethod : FromMethod->overridden_methods())<br>
+  Â  ToMethod->addOverriddenMethod(<br>
+  Â  Â  cast<CXXMethodDecl>(Importer.Import(const_cast<CXXMethodDecl*>(<br>
+  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  FromOverriddenMethod))));<br>
+}<br>
+<br>
 ASTImporter::ASTImporter(ASTContext &ToContext, FileManager &ToFileManager,<br>
  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  ASTContext &FromContext, FileManager &FromFileManager,<br>
  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  bool MinimalImport)<br>
<br>
Modified: cfe/trunk/tools/clang-import-test/clang-import-test.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-import-test/clang-import-test.cpp?rev=305850&r1=305849&r2=305850&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-import-test/clang-import-test.cpp?rev=305850&r1=305849&r2=305850&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/tools/clang-import-test/clang-import-test.cpp (original)<br>
+++ cfe/trunk/tools/clang-import-test/clang-import-test.cpp Tue Jun 20 16:06:00 2017<br>
@@ -17,7 +17,9 @@<br>
 #include "clang/Basic/TargetInfo.h"<br>
 #include "clang/Basic/TargetOptions.h"<br>
 #include "clang/CodeGen/ModuleBuilder.h"<br>
+#include "clang/Frontend/ASTConsumers.h"<br>
 #include "clang/Frontend/CompilerInstance.h"<br>
+#include "clang/Frontend/MultiplexConsumer.h"<br>
 #include "clang/Frontend/TextDiagnosticBuffer.h"<br>
 #include "clang/Lex/Lexer.h"<br>
 #include "clang/Lex/Preprocessor.h"<br>
@@ -51,6 +53,10 @@ static llvm::cl::list<std::string><br>
  Â  Â  Â  Â  Â  Â  Â llvm::cl::desc("Argument to pass to the CompilerInvocation"),<br>
  Â  Â  Â  Â  Â  Â  Â llvm::cl::CommaSeparated);<br>
<br>
+static llvm::cl::opt<bool><br>
+DumpAST("dump-ast", llvm::cl::init(false),<br>
+  Â  Â  Â  llvm::cl::desc("Dump combined AST"));<br>
+<br>
 namespace init_convenience {<br>
 class TestDiagnosticConsumer : public DiagnosticConsumer {<br>
 private:<br>
@@ -233,7 +239,7 @@ std::unique_ptr<CompilerInstance> BuildI<br>
 }<br>
<br>
 llvm::Error ParseSource(const std::string &Path, CompilerInstance &CI,<br>
-  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  CodeGenerator &CG) {<br>
+  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  ASTConsumer &Consumer) {<br>
  Â SourceManager &SM = CI.getSourceManager();<br>
  Â const FileEntry *FE = CI.getFileManager().getFile(Path);<br>
  Â if (!FE) {<br>
@@ -241,13 +247,14 @@ llvm::Error ParseSource(const std::strin<br>
  Â  Â  Â  Â llvm::Twine("Couldn't open ", Path), std::error_code());<br>
  Â }<br>
  Â SM.setMainFileID(SM.createFileID(FE, SourceLocation(), SrcMgr::C_User));<br>
-  ParseAST(CI.getPreprocessor(), &CG, CI.getASTContext());<br>
+  ParseAST(CI.getPreprocessor(), &Consumer, CI.getASTContext());<br>
  Â return llvm::Error::success();<br>
 }<br>
<br>
 llvm::Expected<std::unique_ptr<CompilerInstance>><br>
 Parse(const std::string &Path,<br>
-  Â  Â  llvm::ArrayRef<std::unique_ptr<CompilerInstance>> Imports) {<br>
+  Â  Â  llvm::ArrayRef<std::unique_ptr<CompilerInstance>> Imports,<br>
+  Â  Â  bool ShouldDumpAST) {<br>
  Â std::vector<const char *> ClangArgv(ClangArgs.size());<br>
  Â std::transform(ClangArgs.begin(), ClangArgs.end(), ClangArgv.begin(),<br>
  Â  Â  Â  Â  Â  Â  Â  Â  [](const std::string &s) -> const char * { return s.data(); });<br>
@@ -261,14 +268,20 @@ Parse(const std::string &Path,<br>
  Â if (Imports.size())<br>
  Â  Â AddExternalSource(*CI, Imports);<br>
<br>
+  std::vector<std::unique_ptr<ASTConsumer>> ASTConsumers;<br>
+<br>
  Â auto LLVMCtx = llvm::make_unique<llvm::LLVMContext>();<br>
-  std::unique_ptr<CodeGenerator> CG =<br>
-  Â  Â  init_convenience::BuildCodeGen(*CI, *LLVMCtx);<br>
-  CG->Initialize(CI->getASTContext());<br>
+  ASTConsumers.push_back(init_convenience::BuildCodeGen(*CI, *LLVMCtx));<br>
+<br>
+  if (ShouldDumpAST)<br>
+  Â  ASTConsumers.push_back(CreateASTDumper("", true, false, false));<br>
<br>
  Â CI->getDiagnosticClient().BeginSourceFile(CI->getLangOpts(),<br>
  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â &CI->getPreprocessor());<br>
-  if (llvm::Error PE = ParseSource(Path, *CI, *CG)) {<br>
+  MultiplexConsumer Consumers(std::move(ASTConsumers));<br>
+  Consumers.Initialize(CI->getASTContext());<br>
+<br>
+  if (llvm::Error PE = ParseSource(Path, *CI, Consumers)) {<br>
  Â  Â return std::move(PE);<br>
  Â }<br>
  Â CI->getDiagnosticClient().EndSourceFile();<br>
@@ -288,7 +301,8 @@ int main(int argc, const char **argv) {<br>
  Â llvm::cl::ParseCommandLineOptions(argc, argv);<br>
  Â std::vector<std::unique_ptr<CompilerInstance>> ImportCIs;<br>
  Â for (auto I : Imports) {<br>
-  Â  llvm::Expected<std::unique_ptr<CompilerInstance>> ImportCI = Parse(I, {});<br>
+  Â  llvm::Expected<std::unique_ptr<CompilerInstance>> ImportCI =<br>
+  Â  Â  Parse(I, {}, false);<br>
  Â  Â if (auto E = ImportCI.takeError()) {<br>
  Â  Â  Â llvm::errs() << llvm::toString(std::move(E));<br>
  Â  Â  Â exit(-1);<br>
@@ -310,7 +324,7 @@ int main(int argc, const char **argv) {<br>
  Â  Â }<br>
  Â }<br>
  Â llvm::Expected<std::unique_ptr<CompilerInstance>> ExpressionCI =<br>
-  Â  Â  Parse(Expression, Direct ? ImportCIs : IndirectCIs);<br>
+  Â  Â  Parse(Expression, Direct ? ImportCIs : IndirectCIs, DumpAST);<br>
  Â if (auto E = ExpressionCI.takeError()) {<br>
  Â  Â llvm::errs() << llvm::toString(std::move(E));<br>
  Â  Â exit(-1);<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div>