[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

This revision was automatically updated to reflect the committed changes. Closed by commit rL362544: [Target] Remove Process::GetCPPLanguageRuntime (authored by xiaobai, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SINCE LAS

[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

labath added a comment. In D62755#1527679 , @xiaobai wrote: > > So, what I'd do (and what I have been doing so far, but mainly as a > > passtime effort) was to cut the dependencies way lower that this. Ideally, > > I wouldn't want to include anything fro

[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

labath accepted this revision. labath added inline comments. Comment at: include/lldb/Target/CPPLanguageRuntime.h:47 + static CPPLanguageRuntime *GetCPPLanguageRuntime(Process &process) { +return static_cast( +process.GetLanguageRuntime(lldb::eLanguageTypeC_plus_plus

[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

JDevlieghere added inline comments. Comment at: include/lldb/Target/CPPLanguageRuntime.h:47 + static CPPLanguageRuntime *GetCPPLanguageRuntime(Process &process) { +return static_cast( +process.GetLanguageRuntime(lldb::eLanguageTypeC_plus_plus)); xiao

[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

jingham added a comment. Fine by me. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62755/new/ https://reviews.llvm.org/D62755 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-com

[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

xiaobai marked an inline comment as done. xiaobai added inline comments. Comment at: include/lldb/Target/CPPLanguageRuntime.h:47 + static CPPLanguageRuntime *GetCPPLanguageRuntime(Process &process) { +return static_cast( +process.GetLanguageRuntime(lldb::eLanguageTyp

[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. This deals with my objections so I'm marking as accepted because that's the only way I can see to unblock. Seems like you were still discussing return types, and I don't want to preempt tha

[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

JDevlieghere added inline comments. Comment at: include/lldb/Target/CPPLanguageRuntime.h:47 + static CPPLanguageRuntime *GetCPPLanguageRuntime(Process &process) { +return static_cast( +process.GetLanguageRuntime(lldb::eLanguageTypeC_plus_plus)); xiao

[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

xiaobai marked an inline comment as done. xiaobai added a comment. In D62755#1527004 , @labath wrote: > However, I just want to add that from an lldb-server POV, even the fact that > we pull in the `Process` class into it's dependency graph is a bug. Ag

[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

labath added a comment. I agree with your reasoning about language runtime dependencies, and I think this is the right way to accomplish this. However, I just want to add that from an lldb-server POV, even the fact that we pull in the `Process` class into it's dependency graph is a bug. So, wha

[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

xiaobai updated this revision to Diff 202523. xiaobai added a comment. Jim's suggestion CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62755/new/ https://reviews.llvm.org/D62755 Files: include/lldb/Target/CPPLanguageRuntime.h include/lldb/Target/Process.h include/lldb/lldb-forward

[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

xiaobai added a comment. In D62755#1525937 , @jingham wrote: > Yeah, getting CPPLanguageRuntime out of the general forward declarations does > seem a worthy goal. But it would be great to do it in a way that doesn't add > so much line noise. I added th

[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

jingham added a comment. In D62755#1525919 , @xiaobai wrote: > In D62755#1525890 , @aprantl wrote: > > > I don't yet see the connection between those goals and this patch, but I > > might be missing something. Woul

[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

xiaobai added a comment. In D62755#1525890 , @aprantl wrote: > I don't yet see the connection between those goals and this patch, but I > might be missing something. Would CPPLanguageRuntime need to be anything but > a forward declaration in Process.h?

[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

xiaobai added a comment. In D62755#1525890 , @aprantl wrote: > Those are good goals. Thank you for working on this! Thank you for taking the time to review this and discuss this with me! :) > I don't yet see the connection between those goals and this p

[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

aprantl added a comment. > I disagree that it is "carrying purity a little too far". My goal is to see > LLDB's non-plugin libraries be more language agnostic. I have two motivations for this: > Better language support in LLDB, making it easier to add support for new > languages in a clean fas

[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

xiaobai marked an inline comment as done. xiaobai added a comment. In D62755#1525790 , @jingham wrote: > This seems like carrying purity a little too far. I disagree that it is "carrying purity a little too far". My goal is to see LLDB's non-plugin libr

[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

aprantl added a comment. I tend to agree with Jim's comment. > There is "GetLanguageRuntime()" which should be used instead. Can you explain why that is? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62755/new/ https://reviews.llvm.org/D62755 ___

[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. This seems like carrying purity a little too far. You haven't removed the fact that the using code is explicitly dialing up the C++ language runtime, you've just made the call-sit

[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

aprantl added inline comments. Comment at: source/Target/Process.cpp:1601 -CPPLanguageRuntime *Process::GetCPPLanguageRuntime(bool retry_if_null) { - std::lock_guard guard(m_language_runtimes_mutex); so what about this mutex at the new call sites? CHANGES SI

[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

xiaobai created this revision. xiaobai added reviewers: compnerd, davide, JDevlieghere, jingham, clayborg, labath, aprantl. I want to remove this method because I think that Process should be language agnostic, or at least, not have knowledge about specific language runtimes. There is "GetLanguag