[lldb-dev] [RFC] Adding a clang-style LLVM.h (or, "Are you tired of typing 'llvm::' everywhere ?")

[Pavel Labath via lldb-dev]

Thanks for the replies. I was hoping to get more positive feedback for 
this, so given the current mixed-feelings replies, I think I'll just 
give up on this idea, unless a more vocal supporter appears (probably 
not the best idea to send this out just before the easter holidays).

In the mean time, here are my thoughts on what was said.


On 18/04/2019 01:54, Adrian McCarthy wrote:
> I don't have a strong opinion, but I lean against the idea for two reasons:
> 
> 1.  The `llvm::` prefixes don't really hinder readability for me.  
> They're like `std::` prefixes on all the C++ standard library types, 
> which I'm perfectly happy to type and read--moreso than using 
> declarations.  Sure, anybody who's been here a while knows which classes 
> come from LLVM, but new folks might build that knowledge by seeing the 
> prefixes.

Yeah, I was wondering why I'm bothered by typing "llvm::" and not by 
"std::". I concluded that this is down to two things:
1. we don't use that many things from the std:: namespace actually. 
Pretty much everything except std::string and std::vector is discouraged 
because llvm has better alternatives

2. llvm names are longer. This is not just due to to "llvm" prefix, 
which is just one char, but also the class names themselves tend to be 
longer. std::vector vs llvm::SmallVector, std::map vs. llvm::DenseMap, 
std::string vs. llvm::StringRef, etc.

This effect gets multiplied once you start to combine things. For 
instance if you have a function returning Expected<ArrayRef<T>> (which 
is not an unreasonable thing to do), then by the time you spell out the 
full type, more than half of your usable horizontal space is gone. 
Because of this, I've found myself using "auto" or relying on ADL more 
and more often, which I don't consider very ideal either.

I don't think using "auto" is always a good choice because it hides 
interesting details. E.g. an Optional<T> can look a lot like 
Expected<T>, but there are differences in how they are supposed used 
which should not be overlooked (I wish I was able to type 
"Expected<auto>" :P). And ADL is sometimes just too magical...


> 
> 2.  I'm not a fan of forward declaring types provided by other parts of 
> the code, as it requires intimate knowledge of implementation details.  
> In practice this may not matter much for the types we're considering.  
> If it grew more widespread, however, I'd be more concerned.  (Somewhere 
> I've written a long explanation of this opinion.  I'll go search for it 
> if anyone cares.  The Google style guide discourages forward 
> declarations, but the rationale given there isn't as persuasive.)

Yeah, I agree the forward declarations are not ideal (and the clang file 
did raise my eyebrows when I first saw it), but after a while I started 
to like it.

FWIW, I wouldn't be opposed to just #including the relevant files 
instead of forward-declaring stuff, but I think doing it the same way is 
better for consistency.


Out of interest, I took a look at what lld is doing. I've found that 
while it doesn't have a LLVM.h equivalent, it is a heavy user of "using 
namespace llvm" (about 2 out of 3 cpp files have it). This approach 
wouldn't work that well for us because of naming conflicts ("Module"), 
and I would consider it inferior for the same reason that "using 
namespace std" is discouraged -- it just brings in too much stuff into 
your scope.

regards,
pl