Michael137 created this revision. Michael137 added reviewers: aprantl, martong. Herald added a subscriber: rnkovacs. Herald added a reviewer: a.sidorin. Herald added a reviewer: shafik. Herald added a project: All. Michael137 requested review of this revision. Herald added projects: clang, LLDB. Herald added subscribers: lldb-commits, cfe-commits.
The uncached lookup is mainly used in the ASTImporter/LLDB code-path where we're not allowed to load from external storage. When importing a FieldDecl with a DeclContext that had no external visible storage (but came from a Clang module or PCH) the above call to `lookup(Name)` the regular lookup fails because: 1. `DeclContext::buildLookup` doesn't set `LookupPtr` for decls that came from a module 2. LLDB doesn't use the `SharedImporterState` In such a case but we would never go on to the "slow" path of iterating through the decls in the DeclContext. In some cases this means that `ASTNodeImporter::VisitFieldDecl` ends up importing a decl into the `DeclContext` a second time. The patch removes the short-circuit in the case where we don't find any decls via the regular lookup. **Tests** - Un-skip the failing LLDB API tests Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D133945 Files: clang/lib/AST/DeclBase.cpp clang/unittests/AST/ASTImporterTest.cpp lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py Index: lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py =================================================================== --- lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py +++ lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py @@ -30,7 +30,6 @@ class TestBaseTemplateWithSameArg(TestBase): @add_test_categories(["gmodules"]) - @skipIf(bugnumber='rdar://96581048') def test_same_base_template_arg(self): self.build() Index: clang/unittests/AST/ASTImporterTest.cpp =================================================================== --- clang/unittests/AST/ASTImporterTest.cpp +++ clang/unittests/AST/ASTImporterTest.cpp @@ -4924,9 +4924,9 @@ FooDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls); EXPECT_EQ(FoundDecls.size(), 0u); - // Cannot find in the LookupTable of its LexicalDC (A). + // Finds via linear search of its LexicalDC (A). FooLexicalDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls); - EXPECT_EQ(FoundDecls.size(), 0u); + EXPECT_EQ(FoundDecls.size(), 1u); // Can't find in the list of Decls of the DC. EXPECT_EQ(findInDeclListOfDC(FooDC, FooName), nullptr); Index: clang/lib/AST/DeclBase.cpp =================================================================== --- clang/lib/AST/DeclBase.cpp +++ clang/lib/AST/DeclBase.cpp @@ -1771,7 +1771,8 @@ if (!hasExternalVisibleStorage() && !hasExternalLexicalStorage() && Name) { lookup_result LookupResults = lookup(Name); Results.insert(Results.end(), LookupResults.begin(), LookupResults.end()); - return; + if (!Results.empty()) + return; } // If we have a lookup table, check there first. Maybe we'll get lucky.
Index: lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py =================================================================== --- lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py +++ lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py @@ -30,7 +30,6 @@ class TestBaseTemplateWithSameArg(TestBase): @add_test_categories(["gmodules"]) - @skipIf(bugnumber='rdar://96581048') def test_same_base_template_arg(self): self.build() Index: clang/unittests/AST/ASTImporterTest.cpp =================================================================== --- clang/unittests/AST/ASTImporterTest.cpp +++ clang/unittests/AST/ASTImporterTest.cpp @@ -4924,9 +4924,9 @@ FooDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls); EXPECT_EQ(FoundDecls.size(), 0u); - // Cannot find in the LookupTable of its LexicalDC (A). + // Finds via linear search of its LexicalDC (A). FooLexicalDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls); - EXPECT_EQ(FoundDecls.size(), 0u); + EXPECT_EQ(FoundDecls.size(), 1u); // Can't find in the list of Decls of the DC. EXPECT_EQ(findInDeclListOfDC(FooDC, FooName), nullptr); Index: clang/lib/AST/DeclBase.cpp =================================================================== --- clang/lib/AST/DeclBase.cpp +++ clang/lib/AST/DeclBase.cpp @@ -1771,7 +1771,8 @@ if (!hasExternalVisibleStorage() && !hasExternalLexicalStorage() && Name) { lookup_result LookupResults = lookup(Name); Results.insert(Results.end(), LookupResults.begin(), LookupResults.end()); - return; + if (!Results.empty()) + return; } // If we have a lookup table, check there first. Maybe we'll get lucky.
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits