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

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rL364229: [Target] Hoist LanguageRuntime::GetDeclVendor (authored by xiaobai, committed by ). Herald added a project: LLVM.

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

labath added a comment. 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 Obj

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

jingham 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 = +ClangASTConte

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

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

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

jingham 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 = +ClangASTConte

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

xiaobai updated this revision to Diff 206054. xiaobai added a comment. Add a few autos CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63622/new/ https://reviews.llvm.org/D63622 Files: include/lldb/Target/LanguageRuntime.h include/lldb/Target/ObjCLanguageRuntime.h source/API/SBTarg

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

xiaobai marked an inline comment as done. xiaobai added inline comments. Comment at: source/API/SBTarget.cpp:1851 +if (ProcessSP process_sp = target_sp->GetProcessSP()) { + for (auto *runtime : process_sp->GetLanguageRuntimes()) { +if (auto vendor = runtime->GetD

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

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

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

jingham added a subscriber: labath. jingham added a comment. 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 informati

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

jingham added inline comments. Comment at: include/lldb/Target/LanguageRuntime.h:137 + virtual DeclVendor *GetDeclVendor() { return nullptr; } + compnerd wrote: > Can this not be `const`? Seems like retrieving the vendor should not mutate > the runtime. The

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

jingham 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 = +ClangASTConte

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

compnerd added inline comments. Comment at: include/lldb/Target/LanguageRuntime.h:137 + virtual DeclVendor *GetDeclVendor() { return nullptr; } + Can this not be `const`? Seems like retrieving the vendor should not mutate the runtime. Comm

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

labath edited reviewers, added: jingham; removed: labath. labath added a comment. Seems like a reasonable thing to do, but I don't really know what this code does... CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63622/new/ https://reviews.llvm.org/D63622

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

xiaobai marked an inline comment as done. xiaobai added a comment. 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.

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

jingham added a comment. This change makes it clear that SBTarget::FindFirstType should take a language, but that is orthogonal to this change. Comment at: source/API/SBTarget.cpp:1854-1859 + if (vendor->FindDecls(const_typename, /*append*/ true, +

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

xiaobai created this revision. xiaobai added reviewers: JDevlieghere, labath, compnerd, davide. 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