[lldb-dev] [Lldb-commits] [lldb] r360757 - Group forward declarations in one namespace lldb_private {}

[Jim Ingham via lldb-dev]

> On May 16, 2019, at 2:24 AM, Pavel Labath <pavel at labath.sk> wrote:
> 
> On 16/05/2019 01:10, Jim Ingham via lldb-dev wrote:
>> When you add to them you are often adding some larger feature which would have required a rebuild anyway, and they go long times with no change...  I have never found the rebuild required when these files are touched to be a drag on my productivity.  And I really appreciate their convenience.
>> But thanks for your friendly advice.
>> Jim
> 
> I don't want to make a big deal out of it, but I'm also not a fan of the lldb-forward header. My two main reasons are:
> - it's inconsistent with the rest of llvm, which does not have any headers of such sort (LLVM.h, which we talked about last time, is the only thing remotely similar, but that's still has a much more narrow scope)
> - it makes it easier to violate layering. Eg. right now I can type something like:
> void do_stuff_with(Target *);
> in to a "Utility" header, and it will compile just fine because it will have the forward-declaration of the Target class available even though nothing in Utility should know about that class.
> 
> Neither of these is a big problem: this is not the most important thing we differ from llvm, and also the layering violation will become obvious once you start to implement the "do_stuff_with" function (because you will hopefully need to include the right header to get the full definition).

For sure, lldb-forward.h should only have forward declarations (or SP declarations) - as is the case now.  So you can't do anything but pass an object along with only lldb-forward.  I like it because it encourages not including the actual definition header files in other header files that don't need to see implementations.  It provides the set of names you can refer to that lldb provides, so it is easy to make your interface declarations work w/o being tempted to grab too much.  The dictate of living off lldb-forward.h actually helps to remove unnecessary dependencies.  The common defines have the other benefit that if you know you should be including these you will more easily learn to look there and see LLDB_INVALID_ADDRESS rather than inventing your own local invalid address marker...

> However, for these reasons, I would prefer if we got rid of this header, or at least moved towards a world where we have one forward-declaring header for each top-level module (so "lldb/Utility/forward.h", would forward-declare only Utility stuff, etc.).
> 

I'm not opposed to the latter option, though I'm not convinced it would be worth the churn.

Jim


> pl