[LLVMdev] lld coding style

[Reid Kleckner]

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.

---

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.

If we limit the scope of the proposed change to removing leading
underscores from fields, then that might be worth doing, but it's really up
to you folks. I just don't think we're ready for a function and variable
case unification yet.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20141009/e6e8ab47/attachment.html>