[lldb] Introduce separate scratch ASTs for debug info types and types imported from C++ modules.

Right now we have one large AST for all types in LLDB. All ODR violations in
types we reconstruct are resolved by just letting the ASTImporter handle the
conflicts (either by merging types or somehow trying to introduce a duplicated
declaration in the AST). This works ok for the normal types we build from debug
information as most of them are just simple CXXRecordDecls or empty template
declarations.

However, with a loaded `std` C++ module we have alternative versions of pretty
much all declarations in the `std` namespace that are much more fleshed out than
the debug information declarations. They have all the information that is lost
when converting to DWARF, such as default arguments, template default arguments,
the actual uninstantiated template declarations and so on.

When we merge these C++ module types into the big scratch AST (that might
already contain debug information types) we give the ASTImporter the tricky task
of somehow creating a consistent AST out of all these declarations. Usually this
ends in a messy AST that contains a mostly broken mix of both module and debug
info declarations. The ASTImporter in LLDB is also importing types with the
MinimalImport setting, which usually means the only information we have when
merging two types is often just the name of the declaration and the information
that it contains some child declarations. This makes it pretty much impossible
to even implement a better merging logic (as the names of C++ module
declarations and debug info declarations are identical).

This patch works around this whole merging problem by separating C++ module
types from debug information types. This is done by splitting up the single
scratch AST into two: One default AST for debug information and a dedicated AST
for C++ module types.

The C++ module AST is implemented as a 'specialised AST' that lives within the
default ScratchTypeSystemClang. When we select the scratch AST we can explicitly
request that we want such a isolated sub-AST of the scratch AST. I kept the
infrastructure more general as we probably can use the same mechanism for other
features that introduce conflicting types (such as programs that are compiled
with a custom -wchar-size= option).

There are just two places where we explicitly have request the C++ module AST:
When we export persistent declarations (`$mytype`) and when we create our
persistent result variable (`$0`, `$1`, ...). There are a few formatters that
were previously assuming that there is only one scratch AST which I cleaned up
in a preparation revision here (D92757).

Reviewed By: aprantl

Differential Revision: https://reviews.llvm.org/D92759

Author: Teemperor

lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp

@@ -443,7 +443,9 @@ void ASTResultSynthesizer::CommitPersistentDecls() {
     return;
 
   auto *persistent_vars = llvm::cast<ClangPersistentVariables>(state);
-  TypeSystemClang *scratch_ctx = ScratchTypeSystemClang::GetForTarget(m_target);
+
+  TypeSystemClang *scratch_ctx = ScratchTypeSystemClang::GetForTarget(
+      m_target, m_ast_context->getLangOpts());
 
   for (clang::NamedDecl *decl : m_decls) {
     StringRef name = decl->getName();

lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp

@@ -71,20 +71,22 @@ ClangASTSource::~ClangASTSource() {
 
   if (!m_target)
     return;
-  // We are in the process of destruction, don't create clang ast context on
-  // demand by passing false to
-  // Target::GetScratchTypeSystemClang(create_on_demand).
-  TypeSystemClang *scratch_clang_ast_context =
-      ScratchTypeSystemClang::GetForTarget(*m_target, false);
 
-  if (!scratch_clang_ast_context)
-    return;
+  // Unregister the current ASTContext as a source for all scratch
+  // ASTContexts in the ClangASTImporter. Without this the scratch AST might
+  // query the deleted ASTContext for additional type information.
+  // We unregister from *all* scratch ASTContexts in case a type got exported
+  // to a scratch AST that isn't the best fitting scratch ASTContext.
+  TypeSystemClang *scratch_ast = ScratchTypeSystemClang::GetForTarget(
+      *m_target, ScratchTypeSystemClang::DefaultAST, false);
 
-  clang::ASTContext &scratch_ast_context =
-      scratch_clang_ast_context->getASTContext();
+  if (!scratch_ast)
+    return;
 
-  if (m_ast_context != &scratch_ast_context && m_ast_importer_sp)
-    m_ast_importer_sp->ForgetSource(&scratch_ast_context, m_ast_context);
+  ScratchTypeSystemClang *default_scratch_ast =
+      llvm::cast<ScratchTypeSystemClang>(scratch_ast);
+  // Unregister from the default scratch AST (and all sub-ASTs).
+  default_scratch_ast->ForgetSource(m_ast_context, *m_ast_importer_sp);
 }
 
 void ClangASTSource::StartTranslationUnit(ASTConsumer *Consumer) {

lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp

@@ -184,7 +184,7 @@ ClangExpressionDeclMap::TargetInfo ClangExpressionDeclMap::GetTargetInfo() {
 TypeFromUser ClangExpressionDeclMap::DeportType(TypeSystemClang &target,
                                                 TypeSystemClang &source,
                                                 TypeFromParser parser_type) {
-  assert(&target == ScratchTypeSystemClang::GetForTarget(*m_target));
+  assert(&target == GetScratchContext(*m_target));
   assert((TypeSystem *)&source == parser_type.GetTypeSystem());
   assert(&source.getASTContext() == m_ast_context);
 
@@ -222,7 +222,7 @@ bool ClangExpressionDeclMap::AddPersistentVariable(const NamedDecl *decl,
     if (target == nullptr)
       return false;
 
-    auto *clang_ast_context = ScratchTypeSystemClang::GetForTarget(*target);
+    auto *clang_ast_context = GetScratchContext(*target);
     if (!clang_ast_context)
       return false;
 
@@ -260,7 +260,7 @@ bool ClangExpressionDeclMap::AddPersistentVariable(const NamedDecl *decl,
   if (target == nullptr)
     return false;
 
-  TypeSystemClang *context = ScratchTypeSystemClang::GetForTarget(*target);
+  TypeSystemClang *context = GetScratchContext(*target);
   if (!context)
     return false;
 
@@ -1638,8 +1638,7 @@ void ClangExpressionDeclMap::AddOneGenericVariable(NameSearchContext &context,
   if (target == nullptr)
     return;
 
-  TypeSystemClang *scratch_ast_context =
-      ScratchTypeSystemClang::GetForTarget(*target);
+  TypeSystemClang *scratch_ast_context = GetScratchContext(*target);
   if (!scratch_ast_context)
     return;
 

lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h

@@ -380,6 +380,11 @@ class ClangExpressionDeclMap : public ClangASTSource {
   /// Deallocate struct variables
   void DisableStructVars() { m_struct_vars.reset(); }
 
+  TypeSystemClang *GetScratchContext(Target &target) {
+    return ScratchTypeSystemClang::GetForTarget(target,
+                                                m_ast_context->getLangOpts());
+  }
+
   /// Get this parser's ID for use in extracting parser- and JIT-specific data
   /// from persistent variables.
   uint64_t GetParserID() { return (uint64_t) this; }

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

@@ -9558,13 +9558,42 @@ TypeSystemClang::DeclContextGetTypeSystemClang(const CompilerDeclContext &dc) {
   return nullptr;
 }
 
+namespace {
+/// A specialized scratch AST used within ScratchTypeSystemClang.
+/// These are the ASTs backing the different IsolatedASTKinds. They behave
+/// like a normal ScratchTypeSystemClang but they don't own their own
+/// persistent  storage or target reference.
+class SpecializedScratchAST : public TypeSystemClang {
+public:
+  /// \param name The display name of the TypeSystemClang instance.
+  /// \param triple The triple used for the TypeSystemClang instance.
+  /// \param ast_source The ClangASTSource that should be used to complete
+  ///                   type information.
+  SpecializedScratchAST(llvm::StringRef name, llvm::Triple triple,
+                        std::unique_ptr<ClangASTSource> ast_source)
+      : TypeSystemClang(name, triple),
+        m_scratch_ast_source_up(std::move(ast_source)) {
+    // Setup the ClangASTSource to complete this AST.
+    m_scratch_ast_source_up->InstallASTContext(*this);
+    llvm::IntrusiveRefCntPtr<clang::ExternalASTSource> proxy_ast_source(
+        m_scratch_ast_source_up->CreateProxy());
+    SetExternalSource(proxy_ast_source);
+  }
+
+  /// The ExternalASTSource that performs lookups and completes types.
+  std::unique_ptr<ClangASTSource> m_scratch_ast_source_up;
+};
+} // namespace
+
+char ScratchTypeSystemClang::ID;
+const llvm::NoneType ScratchTypeSystemClang::DefaultAST = llvm::None;
+
 ScratchTypeSystemClang::ScratchTypeSystemClang(Target &target,
                                                llvm::Triple triple)
-    : TypeSystemClang("scratch ASTContext", triple),
+    : TypeSystemClang("scratch ASTContext", triple), m_triple(triple),
       m_target_wp(target.shared_from_this()),
       m_persistent_variables(new ClangPersistentVariables) {
-  m_scratch_ast_source_up = std::make_unique<ClangASTSource>(
-      target.shared_from_this(), m_persistent_variables->GetClangASTImporter());
+  m_scratch_ast_source_up = CreateASTSource();
   m_scratch_ast_source_up->InstallASTContext(*this);
   llvm::IntrusiveRefCntPtr<clang::ExternalASTSource> proxy_ast_source(
       m_scratch_ast_source_up->CreateProxy());
@@ -9576,16 +9605,24 @@ void ScratchTypeSystemClang::Finalize() {
   m_scratch_ast_source_up.reset();
 }
 
-TypeSystemClang *ScratchTypeSystemClang::GetForTarget(Target &target,
-                                                      bool create_on_demand) {
+TypeSystemClang *
+ScratchTypeSystemClang::GetForTarget(Target &target,
+                                     llvm::Optional<IsolatedASTKind> ast_kind,
+                                     bool create_on_demand) {
   auto type_system_or_err = target.GetScratchTypeSystemForLanguage(
       lldb::eLanguageTypeC, create_on_demand);
   if (auto err = type_system_or_err.takeError()) {
     LLDB_LOG_ERROR(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_TARGET),
                    std::move(err), "Couldn't get scratch TypeSystemClang");
     return nullptr;
   }
-  return llvm::dyn_cast<TypeSystemClang>(&type_system_or_err.get());
+  ScratchTypeSystemClang &scratch_ast =
+      llvm::cast<ScratchTypeSystemClang>(type_system_or_err.get());
+  // If no dedicated sub-AST was requested, just return the main AST.
+  if (ast_kind == DefaultAST)
+    return &scratch_ast;
+  // Search the sub-ASTs.
+  return &scratch_ast.GetIsolatedAST(*ast_kind);
 }
 
 UserExpression *ScratchTypeSystemClang::GetUserExpression(
@@ -9630,3 +9667,41 @@ PersistentExpressionState *
 ScratchTypeSystemClang::GetPersistentExpressionState() {
   return m_persistent_variables.get();
 }
+
+void ScratchTypeSystemClang::ForgetSource(ASTContext *src_ctx,
+                                          ClangASTImporter &importer) {
+  // Remove it as a source from the main AST.
+  importer.ForgetSource(&getASTContext(), src_ctx);
+  // Remove it as a source from all created sub-ASTs.
+  for (const auto &a : m_isolated_asts)
+    importer.ForgetSource(&a.second->getASTContext(), src_ctx);
+}
+
+std::unique_ptr<ClangASTSource> ScratchTypeSystemClang::CreateASTSource() {
+  return std::make_unique<ClangASTSource>(
+      m_target_wp.lock()->shared_from_this(),
+      m_persistent_variables->GetClangASTImporter());
+}
+
+static llvm::StringRef
+GetSpecializedASTName(ScratchTypeSystemClang::IsolatedASTKind feature) {
+  switch (feature) {
+  case ScratchTypeSystemClang::IsolatedASTKind::CppModules:
+    return "scratch ASTContext for C++ module types";
+  }
+  llvm_unreachable("Unimplemented ASTFeature kind?");
+}
+
+TypeSystemClang &ScratchTypeSystemClang::GetIsolatedAST(
+    ScratchTypeSystemClang::IsolatedASTKind feature) {
+  auto found_ast = m_isolated_asts.find(feature);
+  if (found_ast != m_isolated_asts.end())
+    return *found_ast->second;
+
+  // Couldn't find the requested sub-AST, so create it now.
+  std::unique_ptr<TypeSystemClang> new_ast;
+  new_ast.reset(new SpecializedScratchAST(GetSpecializedASTName(feature),
+                                          m_triple, CreateASTSource()));
+  m_isolated_asts[feature] = std::move(new_ast);
+  return *m_isolated_asts[feature];
+}

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h

@@ -1110,22 +1110,69 @@ class TypeSystemClang : public TypeSystem {
 /// The TypeSystemClang instance used for the scratch ASTContext in a
 /// lldb::Target.
 class ScratchTypeSystemClang : public TypeSystemClang {
+  /// LLVM RTTI support
+  static char ID;
+
 public:
   ScratchTypeSystemClang(Target &target, llvm::Triple triple);
 
   ~ScratchTypeSystemClang() override = default;
 
   void Finalize() override;
 
+  /// The different kinds of isolated ASTs within the scratch TypeSystem.
+  ///
+  /// These ASTs are isolated from the main scratch AST and are each
+  /// dedicated to a special language option/feature that makes the contained
+  /// AST nodes incompatible with other AST nodes.
+  enum class IsolatedASTKind {
+    /// The isolated AST for declarations/types from expressions that imported
+    /// type information from a C++ module. The templates from a C++ module
+    /// often conflict with the templates we generate from debug information,
+    /// so we put these types in their own AST.
+    CppModules
+  };
+
+  /// Alias for requesting the default scratch TypeSystemClang in GetForTarget.
+  // This isn't constexpr as gtest/llvm::Optional comparison logic is trying
+  // to get the address of this for pretty-printing.
+  static const llvm::NoneType DefaultAST;
+
+  /// Infers the appropriate sub-AST from Clang's LangOptions.
+  static llvm::Optional<IsolatedASTKind>
+  InferIsolatedASTKindFromLangOpts(const clang::LangOptions &l) {
+    // If modules are activated we want the dedicated C++ module AST.
+    // See IsolatedASTKind::CppModules for more info.
+    if (l.Modules)
+      return IsolatedASTKind::CppModules;
+    return DefaultAST;
+  }
+
   /// Returns the scratch TypeSystemClang for the given target.
   /// \param target The Target which scratch TypeSystemClang should be returned.
+  /// \param ast_kind Allows requesting a specific sub-AST instead of the
+  ///                 default scratch AST. See also `IsolatedASTKind`.
   /// \param create_on_demand If the scratch TypeSystemClang instance can be
   /// created by this call if it doesn't exist yet. If it doesn't exist yet and
   /// this parameter is false, this function returns a nullptr.
   /// \return The scratch type system of the target or a nullptr in case an
   ///         error occurred.
+  static TypeSystemClang *
+  GetForTarget(Target &target,
+               llvm::Optional<IsolatedASTKind> ast_kind = DefaultAST,
+               bool create_on_demand = true);
+
+  /// Returns the scratch TypeSystemClang for the given target. The returned
+  /// TypeSystemClang will be the scratch AST or a sub-AST, depending on which
+  /// fits best to the passed LangOptions.
+  /// \param target The Target which scratch TypeSystemClang should be returned.
+  /// \param lang_opts The LangOptions of a clang ASTContext that the caller
+  ///                  wants to export type information from. This is used to
+  ///                  find the best matching sub-AST that will be returned.
   static TypeSystemClang *GetForTarget(Target &target,
-                                       bool create_on_demand = true);
+                                       const clang::LangOptions &lang_opts) {
+    return GetForTarget(target, InferIsolatedASTKindFromLangOpts(lang_opts));
+  }
 
   UserExpression *
   GetUserExpression(llvm::StringRef expr, llvm::StringRef prefix,
@@ -1143,14 +1190,41 @@ class ScratchTypeSystemClang : public TypeSystemClang {
   CreateUtilityFunction(std::string text, std::string name) override;
 
   PersistentExpressionState *GetPersistentExpressionState() override;
+
+  /// Unregisters the given ASTContext as a source from the scratch AST (and
+  /// all sub-ASTs).
+  /// \see ClangASTImporter::ForgetSource
+  void ForgetSource(clang::ASTContext *src_ctx, ClangASTImporter &importer);
+
+  // llvm casting support
+  bool isA(const void *ClassID) const override {
+    return ClassID == &ID || TypeSystemClang::isA(ClassID);
+  }
+  static bool classof(const TypeSystem *ts) { return ts->isA(&ID); }
+
 private:
+  std::unique_ptr<ClangASTSource> CreateASTSource();
+  /// Returns the requested sub-AST.
+  /// Will lazily create the sub-AST if it hasn't been created before.
+  TypeSystemClang &GetIsolatedAST(IsolatedASTKind feature);
+
+  /// The target triple.
+  /// This was potentially adjusted and might not be identical to the triple
+  /// of `m_target_wp`.
+  llvm::Triple m_triple;
   lldb::TargetWP m_target_wp;
   /// The persistent variables associated with this process for the expression
   /// parser.
   std::unique_ptr<ClangPersistentVariables> m_persistent_variables;
   /// The ExternalASTSource that performs lookups and completes minimally
   /// imported types.
   std::unique_ptr<ClangASTSource> m_scratch_ast_source_up;
+
+  /// Map from IsolatedASTKind to their actual TypeSystemClang instance.
+  /// This map is lazily filled with sub-ASTs and should be accessed via
+  /// `GetSubAST` (which lazily fills this map).
+  std::unordered_map<IsolatedASTKind, std::unique_ptr<TypeSystemClang>>
+      m_isolated_asts;
 };
 
 } // namespace lldb_private

lldb/test/API/commands/expression/import-std-module/non-module-type-separation/Makefile

@@ -0,0 +1,3 @@
+USE_LIBCPP := 1
+CXX_SOURCES := main.cpp
+include Makefile.rules