This revision was automatically updated to reflect the committed changes.
Closed by commit rG47e7ecdd7d36: [lldb] Introduce separate scratch ASTs for
debug info types and types imported… (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
Changed prior to commit:
https://reviews.llvm.org/D92759?vs=310535&id=310958#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92759/new/
https://reviews.llvm.org/D92759
Files:
lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
lldb/test/API/commands/expression/import-std-module/non-module-type-separation/Makefile
lldb/test/API/commands/expression/import-std-module/non-module-type-separation/TestNonModuleTypeSeparation.py
lldb/test/API/commands/expression/import-std-module/non-module-type-separation/main.cpp
lldb/unittests/Symbol/TestTypeSystemClang.cpp
Index: lldb/unittests/Symbol/TestTypeSystemClang.cpp
===================================================================
--- lldb/unittests/Symbol/TestTypeSystemClang.cpp
+++ lldb/unittests/Symbol/TestTypeSystemClang.cpp
@@ -730,3 +730,15 @@
EXPECT_FALSE(method->isDirectMethod());
EXPECT_EQ(method->getDeclName().getObjCSelector().getAsString(), "foo");
}
+
+TEST(TestScratchTypeSystemClang, InferSubASTFromLangOpts) {
+ LangOptions lang_opts;
+ EXPECT_EQ(
+ ScratchTypeSystemClang::DefaultAST,
+ ScratchTypeSystemClang::InferIsolatedASTKindFromLangOpts(lang_opts));
+
+ lang_opts.Modules = true;
+ EXPECT_EQ(
+ ScratchTypeSystemClang::IsolatedASTKind::CppModules,
+ ScratchTypeSystemClang::InferIsolatedASTKindFromLangOpts(lang_opts));
+}
Index: lldb/test/API/commands/expression/import-std-module/non-module-type-separation/main.cpp
===================================================================
--- /dev/null
+++ lldb/test/API/commands/expression/import-std-module/non-module-type-separation/main.cpp
@@ -0,0 +1,17 @@
+#include <vector>
+
+struct DbgInfoClass {
+ std::vector<int> ints;
+};
+
+int main(int argc, char **argv) {
+ std::vector<int> a = {3, 1, 2};
+
+ // Create a std::vector of a class from debug info with one element.
+ std::vector<DbgInfoClass> dbg_info_vec;
+ dbg_info_vec.resize(1);
+ // And that class has a std::vector of integers that comes from the C++
+ // module.
+ dbg_info_vec.back().ints.push_back(1);
+ return 0; // Set break point at this line.
+}
Index: lldb/test/API/commands/expression/import-std-module/non-module-type-separation/TestNonModuleTypeSeparation.py
===================================================================
--- /dev/null
+++ lldb/test/API/commands/expression/import-std-module/non-module-type-separation/TestNonModuleTypeSeparation.py
@@ -0,0 +1,88 @@
+"""
+Test that LLDB is separating C++ module types and debug information types
+in the scratch AST.
+"""
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestCase(TestBase):
+
+ mydir = TestBase.compute_mydir(__file__)
+
+ @add_test_categories(["libc++"])
+ @skipIf(compiler=no_match("clang"))
+ def test(self):
+ """
+ This test is creating ValueObjects with both C++ module and debug
+ info types for std::vector<int>. We can't merge these types into
+ the same AST, so for this test to pass LLDB should split the types
+ up depending on whether they come from a module or not.
+ """
+ self.build()
+
+ lldbutil.run_to_source_breakpoint(self,
+ "// Set break point at this line.",
+ lldb.SBFileSpec("main.cpp"))
+
+ children = [
+ ValueCheck(value="3"),
+ ValueCheck(value="1"),
+ ValueCheck(value="2"),
+ ]
+
+ # The debug info vector type doesn't know about the default template
+ # arguments, so they will be printed.
+ debug_info_vector_type = "std::vector<int, std::allocator<int> >"
+
+ # The C++ module vector knows its default template arguments and will
+ # suppress them.
+ module_vector_type = "std::vector<int>"
+
+ # First muddy the scratch AST with non-C++ module types.
+ self.expect_expr("a", result_type=debug_info_vector_type,
+ result_children=children)
+ self.expect_expr("dbg_info_vec",
+ result_type="std::vector<DbgInfoClass, std::allocator<DbgInfoClass> >",
+ result_children=[
+ ValueCheck(type="DbgInfoClass", children=[
+ ValueCheck(name="ints", type=debug_info_vector_type, children=[
+ ValueCheck(value="1")
+ ])
+ ])
+ ])
+
+ # Enable the C++ module import and get the module vector type.
+ self.runCmd("settings set target.import-std-module true")
+ self.expect_expr("a", result_type=module_vector_type,
+ result_children=children)
+
+ # Test mixed debug info/module types
+ self.expect_expr("dbg_info_vec",
+ result_type="std::vector<DbgInfoClass>",
+ result_children=[
+ ValueCheck(type="DbgInfoClass", children=[
+ ValueCheck(name="ints", type=module_vector_type, children=[
+ ValueCheck(value="1")
+ ])
+ ])
+ ])
+
+
+ # Turn off the C++ module import and use debug info types again.
+ self.runCmd("settings set target.import-std-module false")
+ self.expect_expr("a", result_type=debug_info_vector_type,
+ result_children=children)
+
+ # Test the types that were previoiusly mixed debug info/module types.
+ self.expect_expr("dbg_info_vec",
+ result_type="std::vector<DbgInfoClass, std::allocator<DbgInfoClass> >",
+ result_children=[
+ ValueCheck(type="DbgInfoClass", children=[
+ ValueCheck(name="ints", type=debug_info_vector_type, children=[
+ ValueCheck(value="1")
+ ])
+ ])
+ ])
Index: lldb/test/API/commands/expression/import-std-module/non-module-type-separation/Makefile
===================================================================
--- /dev/null
+++ 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
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
===================================================================
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -1110,6 +1110,9 @@
/// 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);
@@ -1117,15 +1120,59 @@
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,7 +1190,28 @@
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.
@@ -1151,6 +1219,12 @@
/// 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
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===================================================================
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -9558,13 +9558,42 @@
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,8 +9605,10 @@
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()) {
@@ -9585,7 +9616,13 @@
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 @@
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];
+}
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
@@ -380,6 +380,11 @@
/// 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; }
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -184,7 +184,7 @@
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 @@
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 @@
if (target == nullptr)
return false;
- TypeSystemClang *context = ScratchTypeSystemClang::GetForTarget(*target);
+ TypeSystemClang *context = GetScratchContext(*target);
if (!context)
return false;
@@ -1638,8 +1638,7 @@
if (target == nullptr)
return;
- TypeSystemClang *scratch_ast_context =
- ScratchTypeSystemClang::GetForTarget(*target);
+ TypeSystemClang *scratch_ast_context = GetScratchContext(*target);
if (!scratch_ast_context)
return;
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -71,20 +71,22 @@
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) {
Index: lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
@@ -443,7 +443,9 @@
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-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
