xiaobai marked an inline comment as done.
xiaobai added inline comments.
================
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:
> xiaobai wrote:
> > 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.
> 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...
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.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63622/new/
https://reviews.llvm.org/D63622
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
