r305850 - Preserve CXX method overrides in ASTImporter

David Blaikie via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 26 19:02:54 PDT 2017


On Tue, Jun 20, 2017 at 2:06 PM Lang Hames via cfe-commits <
cfe-commits at 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 at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170627/984cfc85/attachment-0001.html>


More information about the cfe-commits mailing list