[Target] Hoist LanguageRuntime::GetDeclVendor

[xiaobai]

Summary

It's possible that each LanguageRuntime could have its own DeclVendor,
so let's hoist that out of ObjCLanguageRuntime into LanguageRuntime.

Additionally, this gives the opportunity to remove SBTarget's dependency
on ObjCLanguageRuntime.

[jingham]

This change makes it clear that SBTarget::FindFirstType should take a language, but that is orthogonal to this change.

source/API/SBTarget.cpp

1854–1859

As soon as you start iterating over all language runtimes, I don't think you can use ClangASTContext::GetTypeForDecl, can you? Not all language runtimes will be backed by a Clang AST.

[xiaobai]

In D63622#1552717, @jingham wrote:

This change makes it clear that SBTarget::FindFirstType should take a language, but that is orthogonal to this change.

Yep, it definitely should.

source/API/SBTarget.cpp

1854–1859

In principle, this is wrong. But FindDecl's deals with clang NamedDecl's, so this still makes sense right now. In the future we will need to make DeclVendor more TypeSystem/compiler agnostic.

[labath]

Seems like a reasonable thing to do, but I don't really know what this code does...

[compnerd]

include/lldb/Target/LanguageRuntime.h

136

Can this not be const? Seems like retrieving the vendor should not mutate the runtime.

source/API/SBTarget.cpp

1850

auto should be fine, the type is spelt out.

1851

Can this not be const auto *runtime?

1912

auto should be fine, the type is spelt out in the method.

[jingham]

source/API/SBTarget.cpp

1854–1859

If that's true, then you should signal the restriction by limiting the iteration over language runtimes to C-family ones. Like:

// DeclVendor only works for C Family languages at present.
if (!Language::LanguageIsCFamily(runtime->GetLanguageType())
       continue;

Or maybe we need another "LanguageIsBackedByClangAST" check? That would be more accurate...

[jingham]

include/lldb/Target/LanguageRuntime.h

136

The vendor is computed lazily, so calling this could cause the vendor to get built. So formally this is not a const operation.

[jingham]

In D63622#1553121, @labath wrote:

Seems like a reasonable thing to do, but I don't really know what this code does...

The runtime DeclVendor gives runtimes a way to produce type information from runtime metadata. For instance, the ObjC runtime tables actually have fairly good type information for all ObjC classes - both instance variables, properties and methods. It is a little lossy, but for instance it knows the return types of methods so with this augmentation users can call ObjC methods without requiring casting... This is just the code to query the runtime's DeclVendors to see if they know anything about a given typename.

ObjC is a bit obnoxious, because you can define instance variables and methods in the implementation of a class as well as its interface. So even though you might have a debug info definition for the type - gotten from including the interface header file - that may not be as good as what you can get from the runtime. But the runtime type info is, as I said, lossy so if you DO have debug info it will be better. That means for ObjC you have to do some kind of merge operation to get the best information for a type. That complexity doesn't affect this patch, but I couldn't resist the opportunity to moan about it a bit since it's given US so much grief over the years!

[xiaobai]

source/API/SBTarget.cpp

1854–1859

I could try to limit things based on language, but that makes it seem unlikely to get cleaned up in the future if DeclVendor is generalized beyond just using clang.

Another idea could be to introduce a method DeclVendor::GetTypes, returning a std::vector<CompilerType> which will do the job that is being done here. That will remove clang from the equation entirely. Of course, because of the way DeclVendor works now, that method will end up using clang internally, but then it will be easier to clean up when support for non-clang-based languages are added.

[xiaobai]

source/API/SBTarget.cpp

1851

Not unless GetDeclVendor is marked as const as well, and if what Jim says is correct, then we cannot do this.

[xiaobai]

Add a few autos

[jingham]

source/API/SBTarget.cpp

1854–1859

The second idea is clearly what's going to have to get done to generalize this to other TypeSystems. I was thinking of limiting the language more as a signpost that this is not a truly general solution, just to decouple this patch from the work to generalize the DeclVendor interface. On second thought, a FIXME comment to that effect is probably a more explicit way to do this.

Of course, if you want to fix how the DeclVendors work to make them handle other TypeSystems as part of that patch, that would be double plus good.

[xiaobai]

source/API/SBTarget.cpp

1854–1859

Okay, so then what I think I'm going to do is this:

• Add a FIXME to this patch denoting that this is really will only work with clang at the moment. Unfortunate, but easily findable with a quick git grep FIXME.
• Submit a patch that adds a method DeclVendor::GetTypes which will take on the burden of using clang. The FIXME will be moved there until I figure out how to generalize DeclVendor further.

Sound good to you?

[jingham]

source/API/SBTarget.cpp

1854–1859

That sounds like a fine plan.

[labath]

In D63622#1553978, @jingham wrote:

The runtime DeclVendor gives runtimes a way to produce type information from runtime metadata. For instance, the ObjC runtime tables actually have fairly good type information for all ObjC classes - both instance variables, properties and methods. It is a little lossy, but for instance it knows the return types of methods so with this augmentation users can call ObjC methods without requiring casting... This is just the code to query the runtime's DeclVendors to see if they know anything about a given typename.

ObjC is a bit obnoxious, because you can define instance variables and methods in the implementation of a class as well as its interface. So even though you might have a debug info definition for the type - gotten from including the interface header file - that may not be as good as what you can get from the runtime. But the runtime type info is, as I said, lossy so if you DO have debug info it will be better. That means for ObjC you have to do some kind of merge operation to get the best information for a type. That complexity doesn't affect this patch, but I couldn't resist the opportunity to moan about it a bit since it's given US so much grief over the years!

Thanks for the explanation/background. :)

include/lldb/Target/LanguageRuntime.h

136

That depends on how you look at it. If something behaves like "const" from the outside (i.e. doesn't change the abstract state of the object), then I'd be fine with marking it as const even if it needs to modify some internal variables. Lazy evaluation/caching is one of the few valid uses for the mutable keyword.

However, that wouldn't be very consistent with how the this is done in the rest of lldb.

include/lldb/Target/LanguageRuntime.h

 #define liblldb_LanguageRuntime_h_
 
 #include "lldb/Breakpoint/BreakpointResolver.h"
 #include "lldb/Breakpoint/BreakpointResolverName.h"
 #include "lldb/Core/PluginInterface.h"
 #include "lldb/Core/Value.h"
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Expression/LLVMUserExpression.h"
+#include "lldb/Symbol/DeclVendor.h"
 #include "lldb/Target/ExecutionContextScope.h"
 #include "lldb/lldb-private.h"
 #include "lldb/lldb-public.h"
 
 #include "clang/Basic/TargetOptions.h"
 
 namespace lldb_private {
 
virtual lldb::ThreadSP GetBacktraceThreadFromException(
       lldb::ValueObjectSP thread_sp) {
     return lldb::ThreadSP();
   }
 
   Process *GetProcess() { return m_process; }
 
   Target &GetTargetRef() { return m_process->GetTarget(); }
 
+  virtual DeclVendor *GetDeclVendor() { return nullptr; }
+
   virtual lldb::BreakpointResolverSP
   CreateExceptionResolver(Breakpoint *bkpt, bool catch_bp, bool throw_bp) = 0;
 
   virtual lldb::SearchFilterSP CreateExceptionSearchFilter() {
     return m_process->GetTarget().GetSearchFilterForModule(nullptr);
   }
 
   virtual bool GetTypeBitSize(const CompilerType &compiler_type,

include/lldb/Target/ObjCLanguageRuntime.h

 #include <unordered_set>
 
 #include "llvm/Support/Casting.h"
 
 #include "lldb/Breakpoint/BreakpointPrecondition.h"
 #include "lldb/Core/PluginInterface.h"
 #include "lldb/Core/ThreadSafeDenseMap.h"
 #include "lldb/Symbol/CompilerType.h"
-#include "lldb/Symbol/DeclVendor.h"
 #include "lldb/Symbol/Type.h"
 #include "lldb/Target/LanguageRuntime.h"
 #include "lldb/lldb-private.h"
 
 class CommandObjectObjC_ClassTable_Dump;
 
 namespace lldb_private {
 
public:
   }
 
   virtual ObjCISA GetISA(ConstString name);
 
   virtual ConstString GetActualTypeName(ObjCISA isa);
 
   virtual ObjCISA GetParentClass(ObjCISA isa);
 
-  virtual DeclVendor *GetDeclVendor() { return nullptr; }
-
   // Finds the byte offset of the child_type ivar in parent_type.  If it can't
   // find the offset, returns LLDB_INVALID_IVAR_OFFSET.
 
   virtual size_t GetByteOffsetForIvar(CompilerType &parent_qual_type,
                                       const char *ivar_name);
 
   bool HasNewLiteralsAndIndexing() {
     if (m_has_new_literals_and_indexing == eLazyBoolCalculate) {

source/API/SBTarget.cpp

 #include "lldb/Symbol/DeclVendor.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/SymbolFile.h"
 #include "lldb/Symbol/SymbolVendor.h"
 #include "lldb/Symbol/VariableList.h"
 #include "lldb/Target/ABI.h"
 #include "lldb/Target/Language.h"
 #include "lldb/Target/LanguageRuntime.h"
-#include "lldb/Target/ObjCLanguageRuntime.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/StackFrame.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/TargetList.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/Args.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/ProcessInfo.h"
for (size_t idx = 0; idx < count; idx++) {
       if (module_sp) {
         TypeSP type_sp(
             module_sp->FindFirstType(sc, const_typename, exact_match));
         if (type_sp)
           return LLDB_RECORD_RESULT(SBType(type_sp));
       }
     }
 
-    // Didn't find the type in the symbols; try the Objective-C runtime if one
-    // is installed
-
-    ProcessSP process_sp(target_sp->GetProcessSP());
-
-    if (process_sp) {
-      ObjCLanguageRuntime *objc_language_runtime =
-          ObjCLanguageRuntime::Get(*process_sp);
-
-      if (objc_language_runtime) {
-        DeclVendor *objc_decl_vendor = objc_language_runtime->GetDeclVendor();
-
-        if (objc_decl_vendor) {
+    // Didn't find the type in the symbols; Try the loaded language runtimes
+    if (auto process_sp = target_sp->GetProcessSP()) {
+      for (auto *runtime : process_sp->GetLanguageRuntimes()) {
+        if (auto vendor = runtime->GetDeclVendor()) {
           std::vector<clang::NamedDecl *> decls;
-
-          if (objc_decl_vendor->FindDecls(const_typename, true, 1, decls) > 0) {
-            if (CompilerType type = ClangASTContext::GetTypeForDecl(decls[0])) {
+          if (vendor->FindDecls(const_typename, /*append*/ true,
+                                /*max_matches*/ 1, decls) > 0) {
+            if (CompilerType type =
+                    ClangASTContext::GetTypeForDecl(decls.front()))
               return LLDB_RECORD_RESULT(SBType(type));
-            }
-          }
-        }
-      }
-    }
 
     // No matches, search for basic typename matches
     ClangASTContext *clang_ast = target_sp->GetScratchClangASTContext();
     if (clang_ast)
       return LLDB_RECORD_RESULT(SBType(ClangASTContext::GetBasicType(
           clang_ast->getASTContext(), const_typename)));
   }
   return LLDB_RECORD_RESULT(SBType());
if (typename_cstr && typename_cstr[0] && target_sp) {
     if (num_matches > 0) {
       for (size_t idx = 0; idx < num_matches; idx++) {
         TypeSP type_sp(type_list.GetTypeAtIndex(idx));
         if (type_sp)
           sb_type_list.Append(SBType(type_sp));
       }
     }
 
-    // Try the Objective-C runtime if one is installed
-
-    ProcessSP process_sp(target_sp->GetProcessSP());
-
-    if (process_sp) {
-      ObjCLanguageRuntime *objc_language_runtime =
-          ObjCLanguageRuntime::Get(*process_sp);
-
-      if (objc_language_runtime) {
-        DeclVendor *objc_decl_vendor = objc_language_runtime->GetDeclVendor();
-
-        if (objc_decl_vendor) {
+    // Try the loaded language runtimes
+    if (auto process_sp = target_sp->GetProcessSP()) {
+      for (auto *runtime : process_sp->GetLanguageRuntimes()) {
+        if (auto *vendor = runtime->GetDeclVendor()) {
           std::vector<clang::NamedDecl *> decls;
-
-          if (objc_decl_vendor->FindDecls(const_typename, true, 1, decls) > 0) {
-            for (clang::NamedDecl *decl : decls) {
-              if (CompilerType type = ClangASTContext::GetTypeForDecl(decl)) {
+          if (vendor->FindDecls(const_typename, /*append*/ true,
+                                /*max_matches*/ 1, decls) > 0) {
+            for (auto *decl : decls) {
+              if (CompilerType type = ClangASTContext::GetTypeForDecl(decl))
                 sb_type_list.Append(SBType(type));
-              }
-            }
-          }
-        }
-      }
-    }
 
     if (sb_type_list.GetSize() == 0) {
       // No matches, search for basic typename matches
       ClangASTContext *clang_ast = target_sp->GetScratchClangASTContext();
       if (clang_ast)
         sb_type_list.Append(SBType(ClangASTContext::GetBasicType(
             clang_ast->getASTContext(), const_typename)));
     }