[lldb] Don't complete ObjCInterfaceDecls in...
[lldb] Don't complete ObjCInterfaceDecls in ClangExternalASTSourceCallbacks::FindExternalVisibleDeclsByName Summary: For ObjCInterfaceDecls, LLDB iterates over the `methods` of the interface in FindExternalVisibleDeclsByName since commit ef423a3b . However, when LLDB calls `oid->methods()` in that function, Clang will pull in all declarations in the current DeclContext from the current ExternalASTSource (which is again, `ClangExternalASTSourceCallbacks`). The reason for that is that `methods()` is just a wrapper for `decls()` which is supposed to provide a list of *all* (both currently loaded and external) decls in the DeclContext. However, `ClangExternalASTSourceCallbacks::FindExternalLexicalDecls` doesn't implement support for ObjCInterfaceDecl, so we don't actually add any declarations and just mark the ObjCInterfaceDecl as having no ExternalLexicalStorage. As LLDB uses the ExternalLexicalStorage to see if it can complete a type with the ExternalASTSource, this causes that LLDB thinks our class can't be completed any further by the ExternalASTSource and will from on no longer make any CompleteType/FindExternalLexicalDecls calls to that decl. This essentially renders those types unusable in the expression parser as they will always be considered incomplete. This patch just changes the call to `methods` (which is just a `decls()` wrapper), to some ad-hoc `noload_methods` call which is wrapping `noload_decls()`. `noload_decls()` won't trigger any calls to the ExternalASTSource, so this prevents that ExternalLexicalStorage will be set to false. The test for this is just adding a method to an ObjC interface. Before this patch, this unset the ExternalLexicalStorage flag and put the interface into the state described above. In a normal user session this situation was triggered by setting a breakpoint in a method of some ObjC class. This caused LLDB to create the MethodDecl for that specific method and put it into the the ObjCInterfaceDecl. Also `ObjCLanguageRuntime::LookupInCompleteClassCache` needs to be unable to resolve the type do an actual definition when the breakpoint is set (I'm not sure how exactly this can happen, but we just found no Type instance that had the `TypePayloadClang::IsCompleteObjCClass` flag set in its payload in the situation where this happens. This however doesn't seem to be a regression as logic wasn't changed from what I can see). The module-ownership.mm test had to be changed as the only reason why the ObjC interface in that test had it's ExternalLexicalStorage flag set to false was because of this unintended side effect. What actually happens in the test is that ExternalLexicalStorage is first set to false in `DWARFASTParserClang::CompleteTypeFromDWARF` when we try to complete the `SomeClass` interface, but is then the flag is set back to true once we add the last ivar of `SomeClass` (see `SetMemberOwningModule` in `TypeSystemClang.cpp` which is called when we add the ivar). I'll fix the code for that in a follow-up patch. I think some of the code here needs some rethinking. LLDB and Clang shouldn't infer anything about the ExternalASTSource and its ability to complete the current type form the `ExternalLexicalStorage` flag. We probably should also actually provide any declarations when we get asked for the lexical decls of an ObjCInterfaceDecl. But both of those changes are bigger (and most likely would cause us to eagerly complete more types), so those will be follow up patches and this patch just brings us back to the state before commit ef423a3b . Fixes rdar://63584164 Reviewers: aprantl, friss, shafik Reviewed By: aprantl, shafik Subscribers: arphaman, abidh, JDevlieghere Differential Revision: https://reviews.llvm.org/D80556
Loading
Please sign in to comment