[Lldb-commits] [PATCH] D63622: [Target] Hoist LanguageRuntime::GetDeclVendor

[Alex Langford via Phabricator via lldb-commits]

xiaobai marked an inline comment as done.
xiaobai added a comment.

In D63622#1552717 <https://reviews.llvm.org/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.



================
Comment at: source/API/SBTarget.cpp:1854-1859
+          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));
           }
----------------
jingham wrote:
> 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.
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.


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

https://reviews.llvm.org/D63622