[Lldb-commits] [PATCH] D78333: Add Objective-C property accessors loaded from Clang module DWARF to lookup

[Adrian Prantl via Phabricator via lldb-commits]

aprantl created this revision.
aprantl added reviewers: teemperor, shafik.
aprantl added a parent revision: D78329: Allow lldb-test to combine -find with -dump-clang-ast.

This patch fixes a bug when synthesizing an ObjC property from -gmodules debug info. Because the method declaration that is injected via the non-modular property implementation is not added to the ObjCInterfaceDecl's lookup pointer, a second copy of the accessor would be generated when processing the ObjCPropertyDecl. This can be avoided by finding the existing method decl in ClangExternalASTSourceCallbacks::FindExternalVisibleDeclsByName() and adding it to the LookupPtr.


https://reviews.llvm.org/D78333

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm


Index: lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm
===================================================================
--- lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm
+++ lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm
@@ -27,14 +27,21 @@
 // CHECK-DAG: EnumDecl {{.*}} imported in A {{.*}} Enum_e
 // FIXME: -EnumConstantDecl {{.*}} imported in A a
 
+ at implementation SomeClass {
+  int private_ivar;
+}
+ at synthesize number = private_ivar;
+ at end
+
 SomeClass *obj1;
 // RUN: lldb-test symbols -dump-clang-ast -find type --language=ObjC++ \
 // RUN:   -compiler-context 'Module:A,Struct:SomeClass' %t.o \
 // RUN:   | FileCheck %s --check-prefix=CHECK-OBJC
 // CHECK-OBJC: ObjCInterfaceDecl {{.*}} imported in A SomeClass
-// CHECK-OBJC: |-ObjCPropertyDecl {{.*}} imported in A number 'int' readonly
-// CHECK-OBJC: | `-getter ObjCMethod {{.*}} 'number'
-// CHECK-OBJC: `-ObjCMethodDecl {{.*}} imported in A implicit - number 'int'
+// CHECK-OBJC-NEXT: |-ObjCIvarDecl
+// CHECK-OBJC-NEXT: |-ObjCMethodDecl 0x[[NUMBER:[0-9a-f]+]]{{.*}} imported in A
+// CHECK-OBJC-NEXT: `-ObjCPropertyDecl {{.*}} imported in A number 'int' readonly
+// CHECK-OBJC-NEXT:   `-getter ObjCMethod 0x[[NUMBER]] 'number'
 
 // Template specializations are not yet supported, so they lack the ownership info:
 Template<int> t2;
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===================================================================
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -344,6 +344,13 @@
   member->setFromASTFile();
   member->setOwningModuleID(id.GetValue());
   member->setModuleOwnershipKind(clang::Decl::ModuleOwnershipKind::Visible);
+  if (auto *nd = llvm::dyn_cast<clang::NamedDecl>(member))
+    if (auto *dc = llvm::dyn_cast<clang::DeclContext>(parent)) {
+      // This triggers ExternalASTSource::FindExternalVisibleDeclsByName() to be
+      // called when searching for members.
+      dc->setHasExternalLexicalStorage(true);
+      dc->setHasExternalVisibleStorage(true);
+    }
 }
 
 char TypeSystemClang::ID;
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h
@@ -30,6 +30,9 @@
       llvm::function_ref<bool(clang::Decl::Kind)> IsKindWeWant,
       llvm::SmallVectorImpl<clang::Decl *> &Result) override;
 
+  bool FindExternalVisibleDeclsByName(const clang::DeclContext *DC,
+                                      clang::DeclarationName Name) override;
+
   void CompleteType(clang::TagDecl *tag_decl) override;
 
   void CompleteType(clang::ObjCInterfaceDecl *objc_decl) override;
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.cpp
@@ -10,6 +10,7 @@
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclObjC.h"
 
 using namespace lldb_private;
 
@@ -46,6 +47,19 @@
   }
 }
 
+bool ClangExternalASTSourceCallbacks::FindExternalVisibleDeclsByName(
+    const clang::DeclContext *DC, clang::DeclarationName Name) {
+  llvm::SmallVector<clang::NamedDecl *, 4> decls;
+  // Objective-C methods are not added into the LookupPtr when they originate
+  // from an external source. SetExternalVisibleDeclsForName() adds them.
+  if (auto *oid = llvm::dyn_cast<clang::ObjCInterfaceDecl>(DC)) {
+    for (auto *omd : oid->methods())
+      if (omd->getDeclName() == Name)
+        decls.push_back(omd);
+  }
+  return !SetExternalVisibleDeclsForName(DC, Name, decls).empty();
+}
+
 OptionalClangModuleID
 ClangExternalASTSourceCallbacks::RegisterModule(clang::Module *module) {
   m_modules.push_back(module);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D78333.258177.patch
Type: text/x-patch
Size: 4122 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20200416/e0eb574b/attachment.bin>