[Lldb-commits] [PATCH] D133906: [lldb] Generate lldb-forward with .def file

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D133906#3791352 <https://reviews.llvm.org/D133906#3791352>, @JDevlieghere wrote:

> In D133906#3791153 <https://reviews.llvm.org/D133906#3791153>, @jingham wrote:
>
>> This patch makes me a little sad because it breaks the "Jump to Definition" chain in Xcode (and I bet it does in other IDE's that support this.)  You used to be able to do "Jump to Definition" on ProcessSP, then jump to definition on the class name in the shared pointer definition to jump to the class.  Now the first jump takes you to:
>>
>> LLDB_FORWARD_CLASS(Process)
>>
>> in the lldb-forward.def file, and you can't go any further because the IDE can't tell what to do with the contents of the .def file (it has no way to know how it was imported to create real definitions).  These .def insertions almost always make things harder to find in the actual code, so they aren't free.
>
> The alternative would be to have tablegen generate the header, but I think that's overkill, and I'm not even sure Xcode would pick the generated header.

I have a feeling that the code would be simpler if you reversed the "iteration order", and it might even make the go-to definition command more useful. I'm thinking of something like

  // no .def file
  #define LLDB_FORWARD_CLASS(cls) \
    namespace lldb_private { class cls; } \
    namespace lldb {  using cls##SP = std::shared_ptr<lldb_private::cls>; } \
    ...
  
  LLDB_FORWARD_CLASS(Foo)
  LLDB_FORWARD_CLASS(Bar)
  ...

That said, my preferred solution would be something like `template<typename T> using SP = std::shared_ptr<T>`, and then replacing all `XyzSP` with `SP<Xyz>`. The two main benefits (besides simplifying the forward file) I see are:

- being able to write `SP<const Xyz>`. With the current pattern, you'd have to introduce a whole new class of typedefs, or live with the fact that `shared_ptr<const xyz>` looks very different from XyzSP
- being compatible with the llvm naming scheme. XyzSP and xyz_sp would collide if they both used camel case. With this, they won't because one of them will not exist.


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

https://reviews.llvm.org/D133906