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

Raphael Isemann via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 22 06:28:16 PDT 2019


teemperor added a comment.

> Well, I still don't understand how LLDB synthesis the AST. 
>  So you have a C++ module in a .pcm file. This means you could create an AST from that with ASTReader from it's .clang_ast section, right? In that case the AST should be complete with all type information. If this was the case then I don't see why it is not possible to use clang::ASTImporter to import templates and specializations, since we do exactly that in CTU.
> 
> Or do you guys create the AST from the debug info, from the .debug_info section of .pcm module file? And this is why AST is incomplete? I've got this from https://youtu.be/EgkZ8PTNSHQ?list=PL85Cf-ok7HpppFWlp2wX_-A1dkyFVlaEB&t=1565
>  If this is the case, then comes my naiive quiestion: Why don't you use the ASTReader?

There are no C++ modules involved in our use case beside the generic `std` module. No user-written code is read from a module and we don't have any PCM file beside the `std` module we build for the expression evaluator.

We use the ASTReader to directly read the `std` module contents into the expression evaluation ASTContext, but this doesn't give us the decls for the instantiations the user has made (e.g. `std::vector<Foo>`). We only have these user instantiations in the 'normal' debug info where we have a rather minimal AST (again, no modules are not involved in building this debug info AST). The `InternalImport` in the LLDB patch just stitches the module AST and the debug info AST together when we import something that we also have (in better quality from the C++ module) in the target ASTContext.



================
Comment at: clang/unittests/AST/ASTImporterTest.cpp:581
+struct RedirectingImporter : public ASTImporter {
+  using ASTImporter::ASTImporter;
+  // Returns a ImporterConstructor that constructs this class.
----------------
balazske wrote:
> Is this `using` needed?
Yeah, otherwise we have to provide our own constructor that essentially just forwards to the ASTImporter constructor.


================
Comment at: clang/unittests/AST/ASTImporterTest.cpp:588
+              bool MinimalImport, ASTImporterLookupTable *LookupTabl) {
+      return static_cast<ASTImporter *>(
+          new RedirectingImporter(ToContext, ToFileManager, FromContext,
----------------
balazske wrote:
> The `static_cast` should not be needed.
Thanks!


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

https://reviews.llvm.org/D59485





More information about the cfe-commits mailing list