[LLVMdev] lld coding style

[David Blaikie]

On Thu, Oct 9, 2014 at 2:58 PM, Nick Kledzik <kledzik at apple.com> wrote:

>
> On Oct 9, 2014, at 10:47 AM, Reid Kleckner <rnk at google.com> wrote:
>
> On Wed, Oct 8, 2014 at 7:20 PM, Nick Kledzik <kledzik at apple.com> wrote:
>
>> Sure, I actually have no problem with this.
>>
>> I'm going to point out that one of the naming conventions used by LLD has
>> serious problems: naming variables with a leading underscore puts them
>> *way* too close to the reserved identifier space. Folks have accidentally
>> ended up with reserved names quite a few times already.
>>
>> The lld conventions for ivars is a leading underscore followed by a
>> lowercase letter.  The reserved identifiers are a leading underscore
>> followed by an uppercase letter.  There is no conflict.
>>
>> On the other hand the LLVM conventions prevents the use of the -Wshadow
>> which catches real bugs.
>>
>
> If people think this is a valuable warning, we should just fix it so that
> it doesn't fire on this common pattern:
> struct A {
>   A(int x) : x(x) {}
>   int x;
> };
>
> It should still fire on this pattern though:
> struct A {
>   A(int x) : x(x) { x++; } // I modified the parameter 'x', not the member
> 'x'.
>   int x;
> };
>
> We shouldn't create conventions just to work around bugs in a toolchain
> that we control.
>
> Or if your convention does not work with existing tools (clang, gcc,
> VisualStudio, etc), maybe that should be a hint that the convention is
> buggy.
>

I don't really agree with this. The idiom of naming ctor parameters the
same as members isn't uncommon - one reason -Wshadow isn't on by default,
because large codebases don't conform to it so the signal/noise ratio is
low.

There are quite a few cases where we improve Clang based on usage in LLVM,
or disable VS warnings rather than working around them in LLVM code.


>
>
>
> My two cents: LLVM's conventions around method names and local variable
> names are widely ignored. For example, the IRBuilder uses leading capitals
> for methods. Clang CodeGen does as well. Clang CodeGen also uses lots of
> local variables with leading lower-case letters, which is against the
> variable naming guidelines.
>
> I think the takeaway from that is that lots of people don't actually
> understand or agree with the LLVM naming standards in this area. I don't
> think we should do a mass rename in LLD if we aren't willing to do a mass
> rename across Clang internals, and so far no one has indicated a desire to
> do that.
>
> Yes.  The top of the LLVM coding conventions, the “golden rule” is don’t
> do mass renames.  So, I’m surprised at the desire for mass renamings.
>
> But, as you said, there is lots of straying from the naming conventions.
>

So far as I know it's legacy (we didn't do a mass rename, so there's lots
of old code still using prior conventions), consistency (use the local
convention or at least change a whole file/class together - sometimes hard
to get the momentum for that), or just simple mistakes (due to the
inconsistency it's sometimes hard to remember the conventions), not
deliberately going against the style conventions.


> So, as Chris suggested, I’ll start a thread to discuss naming conventions.
>
> -Nick
>
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20141009/4ab15617/attachment.html>