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

[Raphael Isemann via Phabricator via lldb-commits]

teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.

So it seems there aren't any alternative solutions that are less invasive, so I think this can go in.



================
Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:348
+  if (auto *nd = llvm::dyn_cast<clang::NamedDecl>(member))
+    if (auto *dc = llvm::dyn_cast<clang::DeclContext>(parent)) {
+      // This triggers ExternalASTSource::FindExternalVisibleDeclsByName() to be
----------------
teemperor wrote:
> SetMemberOwningModule is called really often and these two conditions are always met in our code, so we are now constantly resetting setHasExternalVisibleStorage to true?
Actually this probably can stay. It's not perfect that we keep resetting, but we should usually add all members at once I think, so it's not really making this thing more complicated.


================
Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:351
+      // called when searching for members.
+      dc->setHasExternalLexicalStorage(true);
+      dc->setHasExternalVisibleStorage(true);
----------------
teemperor wrote:
> You shouldn't need this to get FindExternalVisibleDeclsByName and all this seems to work without it?
So if you move this above the comment this is already good enough. It's not needed for FindExternalVisibleDeclsByName but it makes sense that this has external lexical storage.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78333/new/

https://reviews.llvm.org/D78333