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

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

In D62755#1525937 <https://reviews.llvm.org/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 these methods because it got tiresome to write and read the version you are showing...
>
> Maybe we can add a static:
>
> CPPLanguageRuntime *GetCPPLanguageRuntime(Process &process);
>
> to CPPLanguageRuntime.h?
>
> If you are calling LanguageRuntime methods on the CPPLanguageRuntime, you shouldn't need the cast and probably shouldn't be including CPPLanguageRuntime.h.   So in that case, you'd just do:
>
> #include "Target/LanguageRuntime.h"
>  ...
>
>   LanguageRuntime *cpp_runtime = process->GetLanguageRuntime(eLanguageTypeCPlusPlus)
>   cpp_runtime->GenericMethod();
>   
>
> which is easy to type and read.
>
> You only need to get a runtime cast as a CPPLanguageRuntime if you are planning to call methods that are in CPPLanguageRuntime.h, so you would be including that header anyway, and then you could use the convenience method from there to improve readability.


What you're saying makes sense to me. Since it's true that casting requires including CPPLanguageRuntime.h in the first place, adding it as a static method to CPPLanguageRuntime itself shouldn't be a problem. I don't think this will be an issue when I try to move CPPLanguageRuntime into the Plugins directory either, so I'm perfectly fine with this solution. I will also see if I can remove CPPLanguageRuntime from the forward declarations, and will try to update this diff tonight or tomorrow.


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

https://reviews.llvm.org/D62755