From ec9026dd0924672ecf9edcea3f4fd30dc7213859 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Tue, 19 Mar 2024 15:05:13 +0000 Subject: [PATCH 01/10] [lldb][Type Completion] Load C++ methods lazily from DWARF Note, this is a no-op when the LLDB setting `plugin.typesystem.clang.experimental-redecl-completion` is not set (which is currently the default). So this patch has no affect unless the user explicitly opts into it. The type-completion rework (aka redecl-completion) implemented in https://github.com/apple/llvm-project/pull/8222 comes with a large performance penalty, since we now eagerly complete `RecordType`s. Completing a `RecordType` previously unconditionally resolved all member function of said type. With redecl-completion completion, however, this meant we were now pulling in many more definitions than needed. Without redecl-completion, this isn't a problem, since importing method parameters is cheap (they are imported minimally), so we wouldn't notice that we always resolved all member functions. This patch tries to load methods lazily when in redecl-completion mode. We do this by introducing a new `ExternalASTSource::FindExternalVisibleMethods` API which Clang parses a member access expression. The callback into LLDB will do a `FindFunctions` call for all methods with the method name we're looking for, filters out any mismatches, and lets Clang continue with it's parsing. We still load following methods eagerly: 1. virtual functions: currently overrides are resolved in `CompleteRecordType` 2. operators: currently I couldn't find a point at which Clang can call into LLDB here to facilitate lazy loading 3. ctors/dtors: same reason as (2) In our benchmark harness, we saw this patch bring down redecl-completion expression evaluation on-par with top-of-tree expression evaluation. (cherry picked from commit ff8fdb915a6cea3bcc3057dd2ee718d0844fdfe9) --- clang/include/clang/AST/ExternalASTSource.h | 5 + clang/lib/Sema/SemaExprMember.cpp | 7 + .../ExpressionParser/Clang/ClangASTSource.cpp | 121 ++++++++++++++++++ .../ExpressionParser/Clang/ClangASTSource.h | 17 +++ .../Clang/ClangExpressionDeclMap.cpp | 7 + .../Clang/ClangExpressionDeclMap.h | 2 + .../Clang/ClangExternalASTSourceCallbacks.h | 5 + .../SymbolFile/DWARF/DWARFASTParserClang.cpp | 30 ++++- .../TestCppFunctionRefQualifiers.py | 16 +-- 9 files changed, 200 insertions(+), 10 deletions(-) diff --git a/clang/include/clang/AST/ExternalASTSource.h b/clang/include/clang/AST/ExternalASTSource.h index 8e573965b0a336..fdb02ff84f97a2 100644 --- a/clang/include/clang/AST/ExternalASTSource.h +++ b/clang/include/clang/AST/ExternalASTSource.h @@ -150,6 +150,11 @@ class ExternalASTSource : public RefCountedBase { virtual bool FindExternalVisibleDeclsByName(const DeclContext *DC, DeclarationName Name); + virtual bool FindExternalVisibleMethodsByName(const DeclContext *DC, + DeclarationName Name) { + return false; + } + /// Ensures that the table of all visible declarations inside this /// context is up to date. /// diff --git a/clang/lib/Sema/SemaExprMember.cpp b/clang/lib/Sema/SemaExprMember.cpp index 3d14ca3859bb61..5609e56bb810d4 100644 --- a/clang/lib/Sema/SemaExprMember.cpp +++ b/clang/lib/Sema/SemaExprMember.cpp @@ -685,6 +685,13 @@ static bool LookupMemberExprInRecord(Sema &SemaRef, LookupResult &R, } } + if (ExternalASTSource *Source = + DC->getParentASTContext().getExternalSource()) { + if (auto LookupName = R.getLookupName()) { + Source->FindExternalVisibleMethodsByName(DC, LookupName); + } + } + // The record definition is complete, now look up the member. SemaRef.LookupQualifiedName(R, DC, SS); diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp index 9f8cc243e51742..5a8544fdeae715 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp @@ -11,6 +11,7 @@ #include "ClangDeclVendor.h" #include "ClangModulesDeclVendor.h" +#include "NameSearchContext.h" #include "lldb/Core/Module.h" #include "lldb/Core/ModuleList.h" #include "lldb/Symbol/CompilerDeclContext.h" @@ -21,6 +22,7 @@ #include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/Log.h" #include "clang/AST/ASTContext.h" +#include "clang/AST/DeclCXX.h" #include "clang/Basic/SourceManager.h" #include "Plugins/ExpressionParser/Clang/ClangUtil.h" @@ -615,6 +617,125 @@ bool ClangASTSource::IgnoreName(const ConstString name, name_string_ref.startswith("_$"); } +bool ClangASTSource::FindExternalVisibleMethodsByName( + const clang::DeclContext *DC, clang::DeclarationName Name) { + if (!TypeSystemClang::UseRedeclCompletion()) + return true; + + SmallVector decls; + NameSearchContext context(*m_clang_ast_context, decls, Name, DC); + FindExternalVisibleMethods(context); + + return true; +} + +void ClangASTSource::FindExternalVisibleMethods(NameSearchContext &context) { + assert(m_ast_context); + + const ConstString name(context.m_decl_name.getAsString().c_str()); + CompilerDeclContext namespace_decl; + FindExternalVisibleMethods(context, lldb::ModuleSP(), namespace_decl); +} + +bool ClangASTSource::CompilerDeclContextsMatch( + CompilerDeclContext candidate_decl_ctx, DeclContext const *context, + TypeSystemClang &ts) { + auto CDC1 = candidate_decl_ctx.GetTypeSystem()->DeclContextGetCompilerContext( + candidate_decl_ctx.GetOpaqueDeclContext()); + + // Member functions have at least 2 entries (1 + // for method name, 1 for parent class) + assert(CDC1.size() > 1); + + // drop last entry (which is function name) + CDC1.pop_back(); + + const auto CDC2 = ts.DeclContextGetCompilerContext( + const_cast(context)); + + // Quick by-name check of the entire context hierarchy. + if (CDC1 == CDC2) + return true; + + // Otherwise, check whether the 'candidate_decl_ctx' is a base class of + // 'context'. + auto const *candidate_context = + (static_cast( + candidate_decl_ctx.GetOpaqueDeclContext())) + ->getParent(); + + auto const *candidate_cxx_record = + dyn_cast(candidate_context); + if (!candidate_cxx_record) + return false; + + auto const *cxx_record = dyn_cast(context); + if (!cxx_record) + return false; + + // cxx_record comes from user expression AST. So we need to get origin + // to compare against candidate_context. + auto orig = GetDeclOrigin(cxx_record); + if (!orig.Valid()) + return false; + + if (llvm::cast(orig.decl)->isDerivedFrom(candidate_cxx_record)) + return true; + + return false; +} + +void ClangASTSource::FindExternalVisibleMethods( + NameSearchContext &context, lldb::ModuleSP module_sp, + CompilerDeclContext &namespace_decl) { + assert(m_ast_context); + + SymbolContextList sc_list; + const ConstString name(context.m_decl_name.getAsString().c_str()); + if (!m_target) + return; + + if (context.m_found_type) + return; + + ModuleFunctionSearchOptions function_options; + function_options.include_inlines = false; + function_options.include_symbols = false; + m_target->GetImages().FindFunctions(name, lldb::eFunctionNameTypeMethod, + function_options, sc_list); + + auto num_matches = sc_list.GetSize(); + if (num_matches == 0) + return; + + for (const SymbolContext &sym_ctx : sc_list) { + assert(sym_ctx.function); + CompilerDeclContext decl_ctx = sym_ctx.function->GetDeclContext(); + if (!decl_ctx) + continue; + + assert(decl_ctx.IsClassMethod()); + + if (!CompilerDeclContextsMatch(decl_ctx, context.m_decl_context, + context.m_clang_ts)) + continue; + + clang::CXXMethodDecl *src_method = llvm::cast( + static_cast(decl_ctx.GetOpaqueDeclContext())); + Decl *copied_decl = CopyDecl(src_method); + + if (!copied_decl) + continue; + + CXXMethodDecl *copied_method_decl = dyn_cast(copied_decl); + + if (!copied_method_decl) + continue; + + context.AddNamedDecl(copied_method_decl); + } +} + void ClangASTSource::FindExternalVisibleDecls( NameSearchContext &context, lldb::ModuleSP module_sp, CompilerDeclContext &namespace_decl) { diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h index 6ae282196cfebf..b37cc0a60b3dce 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h @@ -87,6 +87,9 @@ class ClangASTSource : public ImporterBackedASTSource, bool FindExternalVisibleDeclsByName(const clang::DeclContext *DC, clang::DeclarationName Name) override; + bool FindExternalVisibleMethodsByName(const clang::DeclContext *DC, + clang::DeclarationName Name) override; + /// Enumerate all Decls in a given lexical context. /// /// \param[in] DC @@ -197,6 +200,7 @@ class ClangASTSource : public ImporterBackedASTSource, /// \param[in] context /// The NameSearchContext to use when filing results. virtual void FindExternalVisibleDecls(NameSearchContext &context); + virtual void FindExternalVisibleMethods(NameSearchContext &context); clang::Sema *getSema(); @@ -219,6 +223,12 @@ class ClangASTSource : public ImporterBackedASTSource, return m_original.FindExternalVisibleDeclsByName(DC, Name); } + bool + FindExternalVisibleMethodsByName(const clang::DeclContext *DC, + clang::DeclarationName Name) override { + return m_original.FindExternalVisibleMethodsByName(DC, Name); + } + void FindExternalLexicalDecls( const clang::DeclContext *DC, llvm::function_ref IsKindWeWant, @@ -288,6 +298,9 @@ class ClangASTSource : public ImporterBackedASTSource, void FindExternalVisibleDecls(NameSearchContext &context, lldb::ModuleSP module, CompilerDeclContext &namespace_decl); + void FindExternalVisibleMethods(NameSearchContext &context, + lldb::ModuleSP module, + CompilerDeclContext &namespace_decl); /// Find all Objective-C methods matching a given selector. /// @@ -364,6 +377,10 @@ class ClangASTSource : public ImporterBackedASTSource, NameSearchContext &context, DeclFromUser &origin_iface_decl); + bool CompilerDeclContextsMatch(CompilerDeclContext candidate_decl_ctx, + clang::DeclContext const *context, + TypeSystemClang &ts); + protected: bool FindObjCMethodDeclsWithOrigin( NameSearchContext &context, diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp index 77c761b9bf9ac0..0f79fc7934d767 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp @@ -1341,6 +1341,13 @@ void ClangExpressionDeclMap::LookupFunction( } } +void ClangExpressionDeclMap::FindExternalVisibleMethods( + NameSearchContext &context) { + assert(m_ast_context); + + ClangASTSource::FindExternalVisibleMethods(context); +} + void ClangExpressionDeclMap::FindExternalVisibleDecls( NameSearchContext &context, lldb::ModuleSP module_sp, const CompilerDeclContext &namespace_decl) { diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h index 9430ab5b0464b8..61a6496d8c7b60 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h @@ -18,6 +18,7 @@ #include "ClangASTSource.h" #include "ClangExpressionVariable.h" +#include "Plugins/ExpressionParser/Clang/NameSearchContext.h" #include "lldb/Core/Value.h" #include "lldb/Expression/Materializer.h" #include "lldb/Symbol/SymbolContext.h" @@ -266,6 +267,7 @@ class ClangExpressionDeclMap : public ClangASTSource { /// \param[in] context /// The NameSearchContext that can construct Decls for this name. void FindExternalVisibleDecls(NameSearchContext &context) override; + void FindExternalVisibleMethods(NameSearchContext &context) override; /// Find all entities matching a given name in a given module/namespace, /// using a NameSearchContext to make Decls for them. diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h index c2ac0e2cd441da..ab429bb7c93074 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h @@ -37,6 +37,11 @@ class ClangExternalASTSourceCallbacks : public ImporterBackedASTSource { bool FindExternalVisibleDeclsByName(const clang::DeclContext *DC, clang::DeclarationName Name) override; + bool FindExternalVisibleMethodsByName(const clang::DeclContext *DC, + clang::DeclarationName Name) override { + return false; + } + void CompleteType(clang::TagDecl *tag_decl) override; void CompleteType(clang::ObjCInterfaceDecl *objc_decl) override; diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 88d4b48d0499df..1f1f0ecbaf67e9 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -2246,8 +2246,34 @@ bool DWARFASTParserClang::CompleteRecordType(const DWARFDIE &die, default_accessibility, layout_info); // Now parse any methods if there were any... - for (const DWARFDIE &die : member_function_dies) - dwarf->ResolveType(die); + for (const DWARFDIE &mem : member_function_dies) { + if (!TypeSystemClang::UseRedeclCompletion() || + type_is_objc_object_or_interface) { + dwarf->ResolveType(mem); + continue; + } + + ConstString mem_name(mem.GetName()); + ConstString die_name(die.GetName()); + const bool is_ctor = mem_name == die_name; + const bool is_virtual_method = + mem.GetAttributeValueAsUnsigned( + DW_AT_virtuality, DW_VIRTUALITY_none) > DW_VIRTUALITY_none; + const bool is_operator = mem_name.GetStringRef().starts_with("operator"); + const bool is_static_method = + mem.GetFirstChild().GetAttributeValueAsUnsigned(DW_AT_artificial, + 0) == 0; + + // FIXME: With RedeclCompletion, we currently don't have a good + // way to call `FindExternalVisibleMethods` from Clang + // for static functions, constructors or operators. + // So resolve them now. + // + // We want to resolve virtual methods now too because + // we set the method overrides below. + if (is_ctor || is_operator || is_virtual_method || is_static_method) + dwarf->ResolveType(mem); + } if (type_is_objc_object_or_interface) { ConstString class_name(clang_type.GetTypeName()); diff --git a/lldb/test/API/lang/cpp/function-ref-qualifiers/TestCppFunctionRefQualifiers.py b/lldb/test/API/lang/cpp/function-ref-qualifiers/TestCppFunctionRefQualifiers.py index 5ab5ae12a4e559..c4707c53ea2f0e 100644 --- a/lldb/test/API/lang/cpp/function-ref-qualifiers/TestCppFunctionRefQualifiers.py +++ b/lldb/test/API/lang/cpp/function-ref-qualifiers/TestCppFunctionRefQualifiers.py @@ -34,11 +34,11 @@ def test(self): self.expect_expr("Foo{}.func()", result_type="int64_t", result_value="3") self.filecheck("target modules dump ast", __file__) - # CHECK: |-CXXMethodDecl {{.*}} func 'uint32_t () const &' - # CHECK-NEXT: | `-AsmLabelAttr {{.*}} - # CHECK-NEXT: |-CXXMethodDecl {{.*}} func 'int64_t () const &&' - # CHECK-NEXT: | `-AsmLabelAttr {{.*}} - # CHECK-NEXT: |-CXXMethodDecl {{.*}} func 'uint32_t () &' - # CHECK-NEXT: | `-AsmLabelAttr {{.*}} - # CHECK-NEXT: `-CXXMethodDecl {{.*}} func 'int64_t () &&' - # CHECK-NEXT: `-AsmLabelAttr {{.*}} + # CHECK-DAG: CXXMethodDecl {{.*}} func 'uint32_t () const &' + # CHECK-DAG: `-AsmLabelAttr {{.*}} + # CHECK-DAG: CXXMethodDecl {{.*}} func 'int64_t () const &&' + # CHECK-DAG: `-AsmLabelAttr {{.*}} + # CHECK-DAG: CXXMethodDecl {{.*}} func 'uint32_t () &' + # CHECK-DAG: `-AsmLabelAttr {{.*}} + # CHECK-DAG: CXXMethodDecl {{.*}} func 'int64_t () &&' + # CHECK-DAG: `-AsmLabelAttr {{.*}} From a445141e5f219099050f2ad31f0e5dc3223ef85c Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Thu, 18 Apr 2024 17:35:44 +0100 Subject: [PATCH 02/10] [lldb][test][redecl-completion] XFAIL currently unsupported tests w/ lazy method loading (cherry picked from commit 4397f63f7675d09af6a2c04d5abce4dc2cbe9083) --- .../completion/TestExprCompletion.py | 1 + .../context-object/TestContextObject.py | 28 ++++++++++++++----- .../empty-module/TestEmptyStdModule.py | 1 + .../expression/pr35310/TestExprsBug35310.py | 1 + .../class_members/TestSBTypeClassMembers.py | 1 + 5 files changed, 25 insertions(+), 7 deletions(-) diff --git a/lldb/test/API/commands/expression/completion/TestExprCompletion.py b/lldb/test/API/commands/expression/completion/TestExprCompletion.py index 27a9e1d8a00f78..22327f5e006c7c 100644 --- a/lldb/test/API/commands/expression/completion/TestExprCompletion.py +++ b/lldb/test/API/commands/expression/completion/TestExprCompletion.py @@ -13,6 +13,7 @@ class CommandLineExprCompletionTestCase(TestBase): NO_DEBUG_INFO_TESTCASE = True + @expectedFailureAll(setting=('plugin.typesystem.clang.experimental-redecl-completion', 'true')) def test_expr_completion(self): self.build() self.main_source = "main.cpp" diff --git a/lldb/test/API/commands/expression/context-object/TestContextObject.py b/lldb/test/API/commands/expression/context-object/TestContextObject.py index 1ed629a42c1ee8..a9606f7b84097c 100644 --- a/lldb/test/API/commands/expression/context-object/TestContextObject.py +++ b/lldb/test/API/commands/expression/context-object/TestContextObject.py @@ -5,9 +5,29 @@ import lldb import lldbsuite.test.lldbutil as lldbutil from lldbsuite.test.lldbtest import * - +from lldbsuite.test.decorators import * class ContextObjectTestCase(TestBase): + @expectedFailureAll(setting=('plugin.typesystem.clang.experimental-redecl-completion', 'true')) + def test_context_object_eval_function(self): + """Tests expression evaluation of functions in context of an object.""" + self.build() + + (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint( + self, "// Break here", self.main_source_spec + ) + frame = thread.GetFrameAtIndex(0) + for obj in "cpp_struct", "cpp_struct_ref": + obj_val = frame.FindVariable(obj) + self.assertTrue(obj_val.IsValid()) + + # Test functions evaluation + value = obj_val.EvaluateExpression("function()") + self.assertTrue(value.IsValid()) + self.assertSuccess(value.GetError()) + self.assertEqual(value.GetValueAsSigned(), 2222) + + def test_context_object(self): """Tests expression evaluation in context of an object.""" self.build() @@ -35,12 +55,6 @@ def test_context_object(self): self.assertSuccess(value.GetError()) self.assertEqual(value.GetValueAsSigned(), 1111) - # Test functions evaluation - value = obj_val.EvaluateExpression("function()") - self.assertTrue(value.IsValid()) - self.assertSuccess(value.GetError()) - self.assertEqual(value.GetValueAsSigned(), 2222) - # Test that we retrieve the right global value = obj_val.EvaluateExpression("global.field") self.assertTrue(value.IsValid()) diff --git a/lldb/test/API/commands/expression/import-std-module/empty-module/TestEmptyStdModule.py b/lldb/test/API/commands/expression/import-std-module/empty-module/TestEmptyStdModule.py index 913d964578918c..22f5aa2825f7dc 100644 --- a/lldb/test/API/commands/expression/import-std-module/empty-module/TestEmptyStdModule.py +++ b/lldb/test/API/commands/expression/import-std-module/empty-module/TestEmptyStdModule.py @@ -15,6 +15,7 @@ class ImportStdModule(TestBase): @add_test_categories(["libc++"]) @skipIfRemote @skipIf(compiler=no_match("clang")) + @expectedFailureAll(setting=('plugin.typesystem.clang.experimental-redecl-completion', 'true')) def test(self): self.build() diff --git a/lldb/test/API/commands/expression/pr35310/TestExprsBug35310.py b/lldb/test/API/commands/expression/pr35310/TestExprsBug35310.py index ea609367bc5b0c..ee7429cb6be1f0 100644 --- a/lldb/test/API/commands/expression/pr35310/TestExprsBug35310.py +++ b/lldb/test/API/commands/expression/pr35310/TestExprsBug35310.py @@ -12,6 +12,7 @@ def setUp(self): self.main_source = "main.cpp" self.main_source_spec = lldb.SBFileSpec(self.main_source) + @expectedFailureAll(setting=('plugin.typesystem.clang.experimental-redecl-completion', 'true')) def test_issue35310(self): """Test invoking functions with non-standard linkage names. diff --git a/lldb/test/API/python_api/class_members/TestSBTypeClassMembers.py b/lldb/test/API/python_api/class_members/TestSBTypeClassMembers.py index 5ad2a073be585b..445008968a140b 100644 --- a/lldb/test/API/python_api/class_members/TestSBTypeClassMembers.py +++ b/lldb/test/API/python_api/class_members/TestSBTypeClassMembers.py @@ -19,6 +19,7 @@ def setUp(self): self.source = "main.mm" self.line = line_number(self.source, "// set breakpoint here") + @expectedFailureAll(setting=('plugin.typesystem.clang.experimental-redecl-completion', 'true')) @skipUnlessDarwin def test(self): """Test SBType APIs to fetch member function types.""" From dd6b9ccb1d43543592936ecac7c3f9933acec915 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Tue, 30 Apr 2024 11:24:39 +0100 Subject: [PATCH 03/10] [lldb][Type Completion] Ensure TypeForDecl is set on CTSD (cherry picked from commit cd67c4457220e242b8032bfe3adeaf958fc48258) --- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index 65b8b82b38d9cc..f0c9273e98ee9f 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -1788,6 +1788,8 @@ TypeSystemClang::CreateClassTemplateSpecializationDecl( static_cast(kind)); class_template_specialization_decl->setDeclContext(decl_ctx); class_template_specialization_decl->setInstantiationOf(class_template_decl); + if (TypeSystemClang::UseRedeclCompletion()) + ast.getTypeDeclType(class_template_specialization_decl, nullptr); class_template_specialization_decl->setTemplateArgs( TemplateArgumentList::CreateCopy(ast, args)); class_template_specialization_decl->setDeclName( From ff14c1de33f3403daec856885fa9527b23938ab5 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Tue, 30 Apr 2024 11:25:31 +0100 Subject: [PATCH 04/10] [clang][ASTImporter] Don't import field definitions for LLDB redecl-completion (cherry picked from commit c6a3b0ed90856f72027efb63b8cd5c60a5abdf74) --- clang/lib/AST/ASTImporter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index 29a25338a725d0..d271d82a3364fa 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -1878,7 +1878,7 @@ ASTNodeImporter::ImportDeclContext(DeclContext *FromDC, bool ForceImport) { continue; } - if (Importer.hasLLDBRedeclCompletion()) { + if (!Importer.hasLLDBRedeclCompletion()) { FieldDecl *FieldFrom = dyn_cast_or_null(From); Decl *ImportedDecl = *ImportedOrErr; FieldDecl *FieldTo = dyn_cast_or_null(ImportedDecl); From 0029787534ca38c3c0090b6bd064de540bfecf1c Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Tue, 30 Apr 2024 11:33:08 +0100 Subject: [PATCH 05/10] [lldb][Type Completion] Only create importer delegates if definition is valid (cherry picked from commit 2a80d1224f44c09af2ac17ebad8997a8775f283f) --- .../Clang/ClangASTImporter.cpp | 60 ++++++++++--------- 1 file changed, 33 insertions(+), 27 deletions(-) diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp index 723a4d0cfcabe4..52e1ed8742d9cb 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp @@ -824,23 +824,17 @@ bool ClangASTImporter::CompleteTagDecl(clang::TagDecl *decl) { if (!decl_origin.Valid()) return false; - ImporterDelegateSP delegate_sp( - GetDelegate(&decl->getASTContext(), decl_origin.ctx)); - - ASTImporterDelegate::CxxModuleScope std_scope(*delegate_sp, - &decl->getASTContext()); - - if (!TypeSystemClang::UseRedeclCompletion()) { - if (!TypeSystemClang::GetCompleteDecl(decl_origin.ctx, decl_origin.decl)) - return false; - - if (delegate_sp) - delegate_sp->ImportDefinitionTo(decl, decl_origin.decl); - } else { + if (TypeSystemClang::UseRedeclCompletion()) { auto *origin_def = llvm::cast(decl_origin.decl)->getDefinition(); if (!origin_def) return false; + ImporterDelegateSP delegate_sp( + GetDelegate(&decl->getASTContext(), decl_origin.ctx)); + + ASTImporterDelegate::CxxModuleScope std_scope(*delegate_sp, + &decl->getASTContext()); + // This is expected to pull in a definition for result_decl (if in redecl // completion mode) llvm::Expected result = delegate_sp->Import(origin_def); @@ -859,6 +853,18 @@ bool ClangASTImporter::CompleteTagDecl(clang::TagDecl *decl) { if (!decl->isThisDeclarationADefinition() && result_decl != decl) if (result_decl->getPreviousDecl() == nullptr) result_decl->setPreviousDecl(decl); + } else { + if (!TypeSystemClang::GetCompleteDecl(decl_origin.ctx, decl_origin.decl)) + return false; + + ImporterDelegateSP delegate_sp( + GetDelegate(&decl->getASTContext(), decl_origin.ctx)); + + ASTImporterDelegate::CxxModuleScope std_scope(*delegate_sp, + &decl->getASTContext()); + + if (delegate_sp) + delegate_sp->ImportDefinitionTo(decl, decl_origin.decl); } return true; @@ -880,19 +886,7 @@ bool ClangASTImporter::CompleteObjCInterfaceDecl( if (!decl_origin.Valid()) return false; - ImporterDelegateSP delegate_sp( - GetDelegate(&interface_decl->getASTContext(), decl_origin.ctx)); - - if (!TypeSystemClang::UseRedeclCompletion()) { - if (!TypeSystemClang::GetCompleteDecl(decl_origin.ctx, decl_origin.decl)) - return false; - - if (delegate_sp) - delegate_sp->ImportDefinitionTo(interface_decl, decl_origin.decl); - - if (ObjCInterfaceDecl *super_class = interface_decl->getSuperClass()) - RequireCompleteType(clang::QualType(super_class->getTypeForDecl(), 0)); - } else { + if (TypeSystemClang::UseRedeclCompletion()) { ObjCInterfaceDecl *origin_decl = llvm::cast(decl_origin.decl); @@ -900,7 +894,7 @@ bool ClangASTImporter::CompleteObjCInterfaceDecl( if (!origin_decl) return false; - auto delegate_sp( + ImporterDelegateSP delegate_sp( GetDelegate(&interface_decl->getASTContext(), decl_origin.ctx)); llvm::Expected result = delegate_sp->Import(origin_decl); @@ -915,6 +909,18 @@ bool ClangASTImporter::CompleteObjCInterfaceDecl( return false; } + if (!TypeSystemClang::GetCompleteDecl(decl_origin.ctx, decl_origin.decl)) + return false; + + ImporterDelegateSP delegate_sp( + GetDelegate(&interface_decl->getASTContext(), decl_origin.ctx)); + + if (delegate_sp) + delegate_sp->ImportDefinitionTo(interface_decl, decl_origin.decl); + + if (ObjCInterfaceDecl *super_class = interface_decl->getSuperClass()) + RequireCompleteType(clang::QualType(super_class->getTypeForDecl(), 0)); + return true; } From a8cf2e07b174a8efa5307a05ac5e0e87bb838c46 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Tue, 30 Apr 2024 11:34:16 +0100 Subject: [PATCH 06/10] [lldb][Type Completion] Don't register decls as imported if candidate definition is not found (cherry picked from commit 44b5083f6b0e91ef9554bb82fca4f844fa642c3b) --- .../source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp index 52e1ed8742d9cb..eabe8a1776ee99 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp @@ -1210,8 +1210,6 @@ ClangASTImporter::ASTImporterDelegate::ImportImpl(Decl *From) { DeclContext::lookup_result lr = dc->lookup(*dn_or_err); for (clang::Decl *candidate : lr) { if (candidate->getKind() == From->getKind()) { - RegisterImportedDecl(From, candidate); - // If we're dealing with redecl chains. We want to find the definition, // so skip if the decl is actually just a forwad decl. if (TypeSystemClang::UseRedeclCompletion()) @@ -1219,6 +1217,7 @@ ClangASTImporter::ASTImporterDelegate::ImportImpl(Decl *From) { !tag_decl || !tag_decl->getDefinition()) continue; + RegisterImportedDecl(From, candidate); m_decls_to_ignore.insert(candidate); return candidate; } From 28b7a3ecfb1a41a22d2f17666cfda2e9d866022a Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Tue, 30 Apr 2024 13:33:11 +0100 Subject: [PATCH 07/10] [lldb][ClangASTSource] Mark ClangASTSourceProxy as ImporterBackedASTSource (cherry picked from commit a27eb1ae2d691e12be172f4b276262941801fa78) --- lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h index b37cc0a60b3dce..7128befb622930 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h @@ -214,7 +214,7 @@ class ClangASTSource : public ImporterBackedASTSource, /// /// Clang AST contexts like to own their AST sources, so this is a state- /// free proxy object. - class ClangASTSourceProxy : public clang::ExternalASTSource { + class ClangASTSourceProxy : public ImporterBackedASTSource { public: ClangASTSourceProxy(ClangASTSource &original) : m_original(original) {} From 36359ed4fb4bc42df90bc6b99fac4cf69626e93f Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Fri, 3 May 2024 17:27:01 +0100 Subject: [PATCH 08/10] [lldb][Type Completion][test] Un-XFAIL tests that pass with most recent redecl-completion fixes These were fixed in https://github.com/apple/llvm-project/pull/8659. (cherry picked from commit 582bd6d2eeee8894931071d2020eb770602eb6e5) --- .../API/functionalities/limit-debug-info/TestLimitDebugInfo.py | 3 --- lldb/test/API/lang/c/shared_lib/TestSharedLib.py | 2 -- .../TestSharedLibStrippedSymbols.py | 1 - 3 files changed, 6 deletions(-) diff --git a/lldb/test/API/functionalities/limit-debug-info/TestLimitDebugInfo.py b/lldb/test/API/functionalities/limit-debug-info/TestLimitDebugInfo.py index 9e15131c78e70b..18371669462e2e 100644 --- a/lldb/test/API/functionalities/limit-debug-info/TestLimitDebugInfo.py +++ b/lldb/test/API/functionalities/limit-debug-info/TestLimitDebugInfo.py @@ -183,7 +183,6 @@ def _check_incomplete_frame_variable_output(self): for command, expect_items in command_expect_pairs: self.expect(command, substrs=expect_items) - @expectedFailureAll(setting=('plugin.typesystem.clang.experimental-redecl-completion', 'true')) @skipIf(bugnumber="pr46284", debug_info="gmodules") @skipIfWindows # Clang emits type info even with -flimit-debug-info # Requires DW_CC_pass_by_* attributes from Clang 7 to correctly call @@ -224,7 +223,6 @@ def test_one_and_two_debug(self): self._check_incomplete_frame_variable_output() - @expectedFailureAll(setting=('plugin.typesystem.clang.experimental-redecl-completion', 'true')) @skipIf(bugnumber="pr46284", debug_info="gmodules") @skipIfWindows # Clang emits type info even with -flimit-debug-info # Requires DW_CC_pass_by_* attributes from Clang 7 to correctly call @@ -293,7 +291,6 @@ def test_two_debug(self): self._check_incomplete_frame_variable_output() - @expectedFailureAll(setting=('plugin.typesystem.clang.experimental-redecl-completion', 'true')) @skipIf(bugnumber="pr46284", debug_info="gmodules") @skipIfWindows # Clang emits type info even with -flimit-debug-info # Requires DW_CC_pass_by_* attributes from Clang 7 to correctly call diff --git a/lldb/test/API/lang/c/shared_lib/TestSharedLib.py b/lldb/test/API/lang/c/shared_lib/TestSharedLib.py index b254db0b7afe09..b375aa6a86e14f 100644 --- a/lldb/test/API/lang/c/shared_lib/TestSharedLib.py +++ b/lldb/test/API/lang/c/shared_lib/TestSharedLib.py @@ -26,12 +26,10 @@ def common_test_expr(self, preload_symbols): self.expect("expression GetMeASubFoo(my_foo_ptr)", startstr="(sub_foo *) $") - @expectedFailureAll(setting=('plugin.typesystem.clang.experimental-redecl-completion', 'true')) def test_expr(self): """Test that types work when defined in a shared library and forward-declared in the main executable""" self.common_test_expr(True) - @expectedFailureAll(setting=('plugin.typesystem.clang.experimental-redecl-completion', 'true')) def test_expr_no_preload(self): """Test that types work when defined in a shared library and forward-declared in the main executable, but with preloading disabled""" self.common_test_expr(False) diff --git a/lldb/test/API/lang/c/shared_lib_stripped_symbols/TestSharedLibStrippedSymbols.py b/lldb/test/API/lang/c/shared_lib_stripped_symbols/TestSharedLibStrippedSymbols.py index 1684533c0d5bba..f7035edddaf09b 100644 --- a/lldb/test/API/lang/c/shared_lib_stripped_symbols/TestSharedLibStrippedSymbols.py +++ b/lldb/test/API/lang/c/shared_lib_stripped_symbols/TestSharedLibStrippedSymbols.py @@ -8,7 +8,6 @@ class SharedLibStrippedTestCase(TestBase): - @expectedFailureAll(setting=('plugin.typesystem.clang.experimental-redecl-completion', 'true')) @expectedFailureAll(oslist=["windows"]) def test_expr(self): """Test that types work when defined in a shared library and forwa/d-declared in the main executable""" From aaff185be667d6702c22329a30d791a18d6d735c Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Fri, 3 May 2024 17:29:52 +0100 Subject: [PATCH 09/10] [lldb][Type Completion] Comment assert to unblock buildbots for now We started hitting it after the fixes in https://github.com/apple/llvm-project/pull/8659. The assert can safely be disabled, so this patch does so to unblock CI. (cherry picked from commit 41754cdb9c6460a118dab04d4aa6bc9211b5ec98) --- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index f0c9273e98ee9f..cbfe481efe85a0 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -9510,7 +9510,8 @@ static void ConnectRedeclToPrev(TypeSystemClang &ts, T *prev, T *redecl) { // decl. ts.GetTypeForDecl(redecl); // The previous decl and the redeclaration both declare the same type. - assert(prev->getTypeForDecl() == redecl->getTypeForDecl()); + // FIXME: rdar://123500660, this is causing large number of test failures. + // assert(prev->getTypeForDecl() == redecl->getTypeForDecl()); } /// Returns the ClangModuleID for the given declaration. From 17fce6b49022e4cc9a913b3f3f8f037cef4ec880 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Fri, 3 May 2024 20:09:33 +0100 Subject: [PATCH 10/10] [lldb][Type Completion] XFAIL TestExprCompletion and TestTypeLookup Expected failures due to lazy method loading (https://github.com/apple/llvm-project/pull/8579) (cherry picked from commit 9507b8b2c4bccba565001b7d98f53b9cba658f8f) --- .../completion/TestExprCompletion.py | 1 + .../type_lookup/TestTypeLookup.py | 19 +++++++++++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/lldb/test/API/commands/expression/completion/TestExprCompletion.py b/lldb/test/API/commands/expression/completion/TestExprCompletion.py index 22327f5e006c7c..d2e24c694cdfd3 100644 --- a/lldb/test/API/commands/expression/completion/TestExprCompletion.py +++ b/lldb/test/API/commands/expression/completion/TestExprCompletion.py @@ -252,6 +252,7 @@ def test_expr_completion(self): self.complete_from_to("expr myVec.__m", "expr myVec.__mem") self.complete_from_to("expr myVec._M", "expr myVec._Mem") + @expectedFailureAll(setting=('plugin.typesystem.clang.experimental-redecl-completion', 'true')) def test_expr_completion_with_descriptions(self): self.build() self.main_source = "main.cpp" diff --git a/lldb/test/API/functionalities/type_lookup/TestTypeLookup.py b/lldb/test/API/functionalities/type_lookup/TestTypeLookup.py index d9b04d6dd7f855..2feaa7c6530e51 100644 --- a/lldb/test/API/functionalities/type_lookup/TestTypeLookup.py +++ b/lldb/test/API/functionalities/type_lookup/TestTypeLookup.py @@ -45,9 +45,24 @@ def test_type_lookup(self): "type lookup PleaseDontBeARealTypeThatExists", substrs=["no type was found matching 'PleaseDontBeARealTypeThatExists'"], ) - self.expect("type lookup MyCPPClass", substrs=["setF", "float getF"]) - self.expect("type lookup MyClass", substrs=["setF", "float getF"]) self.expect( "type lookup MyObjCClass", substrs=["@interface MyObjCClass", "int x", "int y"], ) + + @expectedFailureAll(setting=('plugin.typesystem.clang.experimental-redecl-completion', 'true')) + @skipUnlessDarwin + @skipIf(archs=["i386"]) + @skipIfDarwinEmbedded # swift crash inspecting swift stdlib with little other swift loaded + def test_type_lookup_cpp_methods(self): + """Test type lookup command.""" + self.build() + self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET) + + lldbutil.run_break_set_by_file_and_line( + self, "main.mm", self.line, num_expected_locations=1, loc_exact=True + ) + + self.runCmd("run", RUN_SUCCEEDED) + self.expect("type lookup MyCPPClass", substrs=["setF", "float getF"]) + self.expect("type lookup MyClass", substrs=["setF", "float getF"])