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