[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

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 rL364845: [Core] Generalize ValueObject::IsRuntimeSupportValue (authored by xiaobai, committed by ). Herald added a project:

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

jingham accepted this revision. jingham added a comment. Sure, NP. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63240/new/ https://reviews.llvm.org/D63240 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

clayborg added a comment. Jim, you ok with doing the symbol context scope refactoring as a separate diff? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63240/new/ https://reviews.llvm.org/D63240 ___ lldb-commits mailing list lldb-commits@li

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

xiaobai added a comment. Centralizing it means changing the classes that inherit from SymbolContextScope, so I will refactor that in a seaparate change and stack this change on top of that one. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63240/new/ https://reviews.llvm.org/D63240

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

clayborg added a comment. In D63240#1562527 , @jingham wrote: > This looks good to me. I wonder if the SymbolContextScope -> Language > calculation that you do in IsRuntimeSupportValue should really be done in > SymbolContextScope? If that's going to b

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

jingham added a comment. This looks good to me. I wonder if the SymbolContextScope -> Language calculation that you do in IsRuntimeSupportValue should really be done in SymbolContextScope? If that's going to be the policy for going from SymbolContextScope, maybe centralize it there? CHANGES

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

xiaobai updated this revision to Diff 206994. xiaobai added a comment. Herald added a subscriber: jdoerfert. - Implement @jingham's suggestion - Change Function::GetLanguage to first guess the language from the name of the function you're in. - Fix a bug in DWARFASTParserClang::ParseFunctionFromD

Re: [Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

> On Jun 27, 2019, at 3:28 PM, Alex Langford via Phabricator > wrote: > > xiaobai added a comment. > > In D63240#1561531 , @jingham wrote: > >> In D63240#1561488 , @xiaobai wrote: >> >>> @jingham: Okay, so I t

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

xiaobai added a comment. In D63240#1561531 , @jingham wrote: > In D63240#1561488 , @xiaobai wrote: > > > @jingham: Okay, so I tried to do what you suggested and that solution > > doesn't work because of ObjC++. Aft

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

jingham added a comment. In D63240#1561488 , @xiaobai wrote: > @jingham: Okay, so I tried to do what you suggested and that solution doesn't > work because of ObjC++. After looking into it, it looks like Variable and > Function just ask the CompileUnit f

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

xiaobai added a comment. @jingham: Okay, so I tried to do what you suggested and that solution doesn't work because of ObjC++. After looking into it, it looks like Variable and Function just ask the CompileUnit for the language type instead of determining it themselves meaning that we determini

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

xiaobai added a comment. In D63240#1560074 , @jingham wrote: > I was talking specifically about which runtime should be asked the question. > It seemed to me most straightforward to invert the order in which the > question was asked, but another way to

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

jingham added a comment. In D63240#1560038 , @xiaobai wrote: > In D63240#1559994 , @jingham wrote: > > > To be more precise, "frame" is the wrong word to use, Variables have > > "scopes"... All Variables have a sc

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

xiaobai added a comment. In D63240#1559994 , @jingham wrote: > To be more precise, "frame" is the wrong word to use, Variables have > "scopes"... All Variables have a scope, though not every scope is contained > in a Function. > > But just to back up a

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

jingham added a comment. In D63240#1559931 , @xiaobai wrote: > In D63240#1559913 , @jingham wrote: > > > Shouldn't ValueObjectVariables figure out their runtime language from their > > defining frame, not their Com

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

xiaobai added a comment. In D63240#1559913 , @jingham wrote: > Shouldn't ValueObjectVariables figure out their runtime language from their > defining frame, not their CompilerType? For a ValueObject you get from an > expression, you probably can't do th

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

jingham added a comment. Shouldn't ValueObjectVariables figure out their runtime language from their defining frame, not their CompilerType? For a ValueObject you get from an expression, you probably can't do that. But we're always talking about hiding locals or args here - i.e. they are all

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

clayborg added a comment. Jim: do you have any other solutions that could make this work? Comment at: source/Core/ValueObject.cpp:1706-1708 + for (auto *runtime : process->GetLanguageRuntimes()) { +if (runtime->IsWhitelistedRuntimeValue(GetName())) + return false; ---

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

xiaobai marked an inline comment as done. xiaobai added inline comments. Comment at: source/Core/ValueObject.cpp:1706-1708 + for (auto *runtime : process->GetLanguageRuntimes()) { +if (runtime->IsWhitelistedRuntimeValue(GetName())) + return false; apran

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

aprantl added inline comments. Comment at: source/Core/ValueObject.cpp:1706-1708 + for (auto *runtime : process->GetLanguageRuntimes()) { +if (runtime->IsWhitelistedRuntimeValue(GetName())) + return false; clayborg wrote: > aprantl wrote: > > jingham wr

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

clayborg added inline comments. Comment at: source/Core/ValueObject.cpp:1706-1708 + for (auto *runtime : process->GetLanguageRuntimes()) { +if (runtime->IsWhitelistedRuntimeValue(GetName())) + return false; aprantl wrote: > jingham wrote: > > clayborg w

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

aprantl added inline comments. Comment at: source/Core/ValueObject.cpp:1706-1708 + for (auto *runtime : process->GetLanguageRuntimes()) { +if (runtime->IsWhitelistedRuntimeValue(GetName())) + return false; jingham wrote: > clayborg wrote: > > davide wro

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

jingham requested changes to this revision. jingham added inline comments. This revision now requires changes to proceed. Comment at: source/Core/ValueObject.cpp:1706-1708 + for (auto *runtime : process->GetLanguageRuntimes()) { +if (runtime->IsWhitelistedRuntimeValue(GetNam

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

clayborg added inline comments. Comment at: source/Core/ValueObject.cpp:1706-1708 + for (auto *runtime : process->GetLanguageRuntimes()) { +if (runtime->IsWhitelistedRuntimeValue(GetName())) + return false; davide wrote: > clayborg wrote: > > Still seem

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

davide added inline comments. Comment at: source/Core/ValueObject.cpp:1706-1708 + for (auto *runtime : process->GetLanguageRuntimes()) { +if (runtime->IsWhitelistedRuntimeValue(GetName())) + return false; clayborg wrote: > Still seems weird that any lan

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

clayborg added inline comments. Comment at: source/Core/ValueObject.cpp:1706-1708 + for (auto *runtime : process->GetLanguageRuntimes()) { +if (runtime->IsWhitelistedRuntimeValue(GetName())) + return false; Still seems weird that any language can white

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

xiaobai updated this revision to Diff 206506. xiaobai added a comment. Address feedback CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63240/new/ https://reviews.llvm.org/D63240 Files: include/lldb/Target/CPPLanguageRuntime.h include/lldb/Target/LanguageRuntime.h include/lldb/Targ

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

aprantl added inline comments. Comment at: source/Core/ValueObject.cpp:1699 if (process) { -LanguageRuntime *runtime = -process->GetLanguageRuntime(GetObjectRuntimeLanguage()); -if (!runtime) - runtime = ObjCLanguageRuntime::Get(*process); -if (runtime

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

xiaobai updated this revision to Diff 206351. xiaobai added a comment. Address feedback CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63240/new/ https://reviews.llvm.org/D63240 Files: include/lldb/Target/CPPLanguageRuntime.h include/lldb/Target/LanguageRuntime.h include/lldb/Targ

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

xiaobai marked an inline comment as done. xiaobai added inline comments. Comment at: include/lldb/Target/LanguageRuntime.h:157 + virtual bool IsRuntimeSupportValue(ValueObject &valobj) { +return valobj.GetVariable() && valobj.GetVariable()->IsArtificial(); + } -

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

aprantl added inline comments. Comment at: include/lldb/Target/LanguageRuntime.h:157 + virtual bool IsRuntimeSupportValue(ValueObject &valobj) { +return valobj.GetVariable() && valobj.GetVariable()->IsArtificial(); + } I think this function should not be pa

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

clayborg added inline comments. Comment at: source/Core/ValueObject.cpp:1719 +// artificial. return GetVariable() && GetVariable()->IsArtificial(); } jingham wrote: > clayborg wrote: > > Things brings the questions: do we really need to filter these v

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

jingham added inline comments. Comment at: source/Core/ValueObject.cpp:1719 +// artificial. return GetVariable() && GetVariable()->IsArtificial(); } clayborg wrote: > Things brings the questions: do we really need to filter these variables? I > would

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

clayborg added inline comments. Comment at: source/Core/ValueObject.cpp:1719 +// artificial. return GetVariable() && GetVariable()->IsArtificial(); } Things brings the questions: do we really need to filter these variables? I wouldn't mind seeing "_c

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

labath resigned from this revision. labath added a comment. I'm not familiar with the code enough to make the calls here. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63240/new/ https://reviews.llvm.org/D63240 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

xiaobai added a comment. ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63240/new/ https://reviews.llvm.org/D63240 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

xiaobai updated this revision to Diff 204615. xiaobai added a comment. Simplify ValueObject::IsRuntimeSupportValue CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63240/new/ https://reviews.llvm.org/D63240 Files: include/lldb/Target/CPPLanguageRuntime.h include/lldb/Target/LanguageRu

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

xiaobai marked 3 inline comments as done. xiaobai added inline comments. Comment at: include/lldb/Target/LanguageRuntime.h:160-162 + /// Identify whether a name is a runtime value that should not be hidden by + /// from the user interface. + virtual bool IsWhitelistedRuntimeVa

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

clayborg requested changes to this revision. clayborg added inline comments. This revision now requires changes to proceed. Comment at: source/Core/ValueObject.cpp:1702-1724 + return runtime->IsRuntimeSupportValue(*this) && + !runtime->IsWhitelistedRuntimeValue(G

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

labath added a reviewer: aprantl. labath added a comment. + Adrian, as I believe we touched this code recently... Comment at: include/lldb/Target/LanguageRuntime.h:160-162 + /// Identify whether a name is a runtime value that should not be hidden by + /// from the user interf

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

xiaobai created this revision. xiaobai added reviewers: labath, compnerd, JDevlieghere, davide. Instead of falling back to ObjCLanguageRuntime, we should be falling back to every loaded language runtime. This makes ValueObject more language agnostic. https://reviews.llvm.org/D63240 Files: inc