[PATCH] D59485: [ASTImporter] Add an ImportInternal method to allow customizing Import behavior.

Aleksei Sidorin via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Mar 24 09:30:40 PDT 2019


a_sidorin added a comment.

Hello Raphael,

Thank you for the explanation. I have +1 to Gabor's question to understand if this functionality can actually be added to the common ASTImporter.



================
Comment at: clang/include/clang/AST/ASTImporter.h:149
+    /// decl on its own.
+    virtual Expected<Decl *> ImportInternal(Decl *From);
+
----------------
I'd suggest to rename it to `ImportImpl`.


================
Comment at: clang/unittests/AST/ASTImporterTest.cpp:590
+          new RedirectingImporter(ToContext, ToFileManager, FromContext,
+                                  FromFileManager, MinimalImport, LookupTabl));
+    };
----------------
LookupTable?


================
Comment at: clang/unittests/AST/ASTImporterTest.cpp:578
+    auto *ND = dyn_cast<NamedDecl>(FromD);
+    if (!ND)
+      return ASTImporter::ImportInternal(FromD);
----------------
I think it's better to merge these conditions: `if (!ND || ND->getName() != "shouldNotBeImported")`


================
Comment at: clang/unittests/AST/ASTImporterTest.cpp:583
+    for (Decl *D : getToContext().getTranslationUnitDecl()->decls()) {
+      if (auto ND = dyn_cast<NamedDecl>(D))
+        if (ND->getName() == "realDecl")
----------------
`auto *` (pointer sign is needed).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59485/new/

https://reviews.llvm.org/D59485





More information about the cfe-commits mailing list