[clang] e456d2b - [clang][ASTImporter] DeclContext::localUncachedLookup: Continue lookup into decl chain when regular lookup fails

[Michael Buch via cfe-commits]

Author: Michael Buch
Date: 2022-09-16T12:38:50-04:00
New Revision: e456d2ba8bcadaa63386b339a4f2abcb39505502

URL: https://github.com/llvm/llvm-project/commit/e456d2ba8bcadaa63386b339a4f2abcb39505502
DIFF: https://github.com/llvm/llvm-project/commit/e456d2ba8bcadaa63386b339a4f2abcb39505502.diff

LOG: [clang][ASTImporter] DeclContext::localUncachedLookup: Continue lookup into decl chain when regular lookup fails

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 `DeclContext::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 we would never continue with the "slow" path of iterating
through the decl chain on 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

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

Added: 
    

Modified: 
    clang/lib/AST/DeclBase.cpp
    clang/unittests/AST/ASTImporterTest.cpp
    lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index f22bf6b0937ee..837ff90d6e34f 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -1771,7 +1771,8 @@ void DeclContext::localUncachedLookup(DeclarationName Name,
   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.

diff  --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp
index d9b7883119345..86a96305bed11 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -4924,9 +4924,9 @@ TEST_P(ASTImporterLookupTableTest,
   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);

diff  --git a/lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py b/lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py
index 0ad4603651a18..b4aec8ff95e4b 100644
--- a/lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py
+++ b/lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py
@@ -34,7 +34,6 @@ def setUp(self):
         self.main_source_file = lldb.SBFileSpec("main.cpp")
 
     @add_test_categories(["gmodules"])
-    @skipIf(bugnumber='rdar://96581048')
     def test_same_template_arg(self):
         lldbutil.run_to_source_breakpoint(self, "Break here", self.main_source_file)
 
@@ -51,7 +50,6 @@ def test_same_template_arg(self):
             ])
 
     @add_test_categories(["gmodules"])
-    @skipIf(bugnumber='rdar://96581048')
     def test_duplicate_decls(self):
         lldbutil.run_to_source_breakpoint(self, "Break here", self.main_source_file)