lld coding style

Project Infrastructure LLVM Dev List Archives
#1

In spite of this possibly raising a holy war, I thought I would bring up this question again.

When switching between a couple of LLVM repositories, I find having to switch between coding styles a bit of an annoyance. It always takes me a while to get adjusted when switching between LLVM (or clang) and lld. While not an absolute show stopper, not having such friction would be nicer.

I realize that lld has had a fair amount of development and that means that it has a somewhat established codebase. However, having a single uniform style across the projects reduces the barrier to entry, encouraging more developers to work on various projects across the set.

So with that in mind, I would like to ask, would it be possible to consider switching to LLVM style for lld?

#2

We don't usually enforce code styles on side projects because it
doesn't make sense to do so. There are too many conflicts between
developers, external source code, style-oriented commits that break
the time-line of the project, that it's just not worth worrying about
it. Not to mention that style is fashion, some like it this way, some
like it that way.

Code style changes with time. If the majority of lld developers agree
with you that the current style is bad and needs to be changed (to a
more LLVM-ish style), than future commits should move lld's style
towards that. Not by adding new styles to one line inside an old
function, but by deprecating style when you deprecate code, and
creating style when you create code.

It's also very likely that, by the time you have converted all old
code with new style, the preferred style will have changed, and you'll
want to do it all over again. To avoid that pointless race against
nothing, we tend to keep the style that was in the original
file/function and not mind much.

Code style is like keyboard layout. Your first reaction is "what the
f**?", then you get used to it, than you can switch between all
keyboard styles (except AZERTY) without a problem.

cheers,
--renato

#3

> So with that in mind, I would like to ask, would it be possible to
consider
> switching to LLVM style for lld?

We don't usually enforce code styles on side projects because it
doesn't make sense to do so.

This is not completely accurate. Both LLD and LLDB were given specific
exemptions from the coding standards, but Clang wasn't and I wouldn't
expect a new subproject to *necessarily* get such an exemption. It might,
it might not.

I consider the current state with both LLD and LLDB a bug rather than a
feature because it creates pointless and non-trivial disruption for
developers to move between LLVM, Clang, LLD, and LLDB. You can argue that
coding standards are like fashion, but *changing* coding standards is a
much more pragmatic and real concern. At least one important point of
LLVM's standards (in my mind and I suspect other developers' minds) is to
drive consistency both within projects and across projects.

Code style changes with time. If the majority of lld developers agree
with you that the current style is bad and needs to be changed (to a
more LLVM-ish style),

Figuring out whether most LLD devs want to switch seems like the point of
the email?

than future commits should move lld's style
towards that. Not by adding new styles to one line inside an old
function, but by deprecating style when you deprecate code, and
creating style when you create code.

That is one option. But the developers of LLD may be willing to more
aggressively convert. We should let them speak for themselves rather than
hypothesizing here.

It's also very likely that, by the time you have converted all old
code with new style, the preferred style will have changed, and you'll
want to do it all over again. To avoid that pointless race against
nothing, we tend to keep the style that was in the original
file/function and not mind much.

This has not been my experience in any part of the LLVM project. The coding
standards are extremely stable these days.

#4

I think we should format code using LLVM coding conventions, much easier to follow and maintain. I think lldb also could consider moving towards one coding convention style.

#5

This is not completely accurate. Both LLD and LLDB were given specific
exemptions from the coding standards, but Clang wasn't and I wouldn't expect
a new subproject to *necessarily* get such an exemption. It might, it might
not.

Why exemptions are made is beyond the point, once it's made, there are
mainly two ways to move to a common point: one big clang-format in the
tree, or a slow moving change on new functions or small refactorings
when moving things around. My point is that the former breaks the
history of the project, while the latter is hard to do without making
it a little worse during the move, which can cause even more pain.

I consider the current state with both LLD and LLDB a bug rather than a
feature because it creates pointless and non-trivial disruption for
developers to move between LLVM, Clang, LLD, and LLDB.

Meh. I have seen so many different coding standards that for me
they're all alike.

You can argue that
coding standards are like fashion, but *changing* coding standards is a much
more pragmatic and real concern. At least one important point of LLVM's
standards (in my mind and I suspect other developers' minds) is to drive
consistency both within projects and across projects.

A fine goal, but IMO, not at the cost of pointlessly tainting the
commit history. Again, my *humble* opinion, which is the only thing I
can offer.

Figuring out whether most LLD devs want to switch seems like the point of
the email?

Indeed. To which I gave my personal opinion. If people felt setback, I
apologise, it wasn't my intent.

I have no stake in lld, and I trusted lld developers would easily
trump and disregard my comments if they weren't relevant to them.

That is one option. But the developers of LLD may be willing to more
aggressively convert. We should let them speak for themselves rather than
hypothesizing here.

Again, they're absolutely free to do so, and I don't believe I have
any power in stopping them to. My opinions are as good as they want it
to be.

This has not been my experience in any part of the LLVM project. The coding
standards are extremely stable these days.

That is a good point. I take that back.

cheers,
--renato

#6

    than future commits should move lld's style
    towards that. Not by adding new styles to one line inside an old
    function, but by deprecating style when you deprecate code, and
    creating style when you create code.

That is one option. But the developers of LLD may be willing to more
aggressively convert. We should let them speak for themselves rather
than hypothesizing here.

Polly did a bulk convert using clang-format if I remember correctly, that solved the coding style discrepancies in just a few commits. They now run clang-format on any commit and that keeps things nice and tidy. I'm not familiar with LLD though and its codebase could be much larger than that of polly at the time...

- Roel

#7

I'm interested in learning how you guys cope with trying to find out
changes past the big move. Is this a problem at all? How long ago was
that?

It might be just my own pre-conceptions and past experiences talking,
but I remember having to checkout a pre-move tree on another project I
was working on to keep following the changes. Though that wasn't at
all common.

All in all, I'd be very happy to prove myself wrong about that once
and for all. :slight_smile:

cheers,
--renato

#8

First, I don't know what to do with LLD but I like the way Polly is maintained
now.

If other projects want to go the same way we can provide the arcanist linter [1]
together with the clang format script [2]. This helps a lot keeping the tree
formated and minimizes the number of "[Format]" commits after something got
accidentally messed up. (We also run the format check on the buildbots to get
immediate feedback.)

[1] http://reviews.llvm.org/D4916
[2] https://github.com/llvm-mirror/polly/blob/master/utils/check_format.sh

#9

So with that in mind, I would like to ask, would it be possible to consider
switching to LLVM style for lld?

One particular feature of lld's current style is particularly dodgy:
starting member variables with '_' makes undefined behaviour very easy
to introduce (if the first real char is upper case; there's already
plenty of examples).

It's one of the more innocuous forms of UB, but still bad form for an
LLVM project. If even we can't get it right...

Cheers.

Tim.

#10

It is unfortunate that we are using a different coding scheme for LLD than LLVM, but I’m leaning toward the view that switching to LLVM style will cost too much if it means we are going to lose virtually all commit history. A patch to switch to LLVM style would rename all local and member variables, so it would touch all the lines. Diff is not powerful enough to trace the history beyond variable renaming. svn blame would become useless.

I have no strong opinion on this. If many other LLD developers really want to make this happen, I can bear with that. It doesn’t feel very productive thing to do to me, though.

#11

It is unfortunate that we are using a different coding scheme for LLD than
LLVM, but I'm leaning toward the view that switching to LLVM style will
cost too much if it means we are going to lose virtually all commit
history. A patch to switch to LLVM style would rename all local and member
variables, so it would touch all the lines. Diff is not powerful enough to
trace the history beyond variable renaming. svn blame would become useless.

You can have svn/git ignore whitespace changes for blame FWIW. I don't know
how this will work for general reformatting though :\

Do you have a feel?

-eric

#12

It is unfortunate that we are using a different coding scheme for LLD
than LLVM, but I'm leaning toward the view that switching to LLVM style
will cost too much if it means we are going to lose virtually all commit
history. A patch to switch to LLVM style would rename all local and member
variables, so it would touch all the lines. Diff is not powerful enough to
trace the history beyond variable renaming. svn blame would become useless.

You can have svn/git ignore whitespace changes for blame FWIW. I don't
know how this will work for general reformatting though :\

Do you have a feel?

I don't think there's an option to make git/svn-diff to ignore case change
and leading underscore removal. Is there?

Ideally maybe we should write a better diff tool that can ignore variable
renaming. Because we can make clang-rename, it's doable at least.

-eric

#13

The only case where this makes sense (that I’m aware of) is libc++, given that it is a very different (and highly constrained) domain. I think that lld and lldb following different coding standards is a serious bug, which increases friction within the community. I’d support progressively moving them to a style more similar to LLVM.

-Chris

#14

It is unfortunate that we are using a different coding scheme for LLD
than LLVM, but I'm leaning toward the view that switching to LLVM style
will cost too much if it means we are going to lose virtually all commit
history. A patch to switch to LLVM style would rename all local and member
variables, so it would touch all the lines. Diff is not powerful enough to
trace the history beyond variable renaming. svn blame would become useless.

You can have svn/git ignore whitespace changes for blame FWIW. I don't
know how this will work for general reformatting though :\

Do you have a feel?

I don't think there's an option to make git/svn-diff to ignore case change
and leading underscore removal. Is there?

Nope. :\

-eric

Ideally maybe we should write a better diff tool that can ignore variable

#15

"useless" is much too strong a description.

If svn blame points to the reformat commit, then simply re-run the blame on
the commit before the reformat.

If total reformats are frequent then this is impractical. If they happen
once in five years then it's not really a big deal. Wrapper scripts can
even be pretty easily written to do this automatically.

#16

The only case where this makes sense (that I’m aware of) is libc++, given that it is a very different (and highly constrained) domain. I think that lld and lldb following different coding standards is a serious bug, which increases friction within the community.

That seems to be the general feeling, yes.

I’d support progressively moving them to a style more similar to LLVM.

What I suggested on my second paragraph. :slight_smile:

I personally think that complete code refactoring is a bit pointless.
Then again, this is a decision to lld developers, I think.

cheers,
--renato

#17

Looks like most people in this thread support using LLVM style in LLD. I also had an offline discussion and many people wanted to have one coding style in all LLVM projects. So I’m convinced that we should do that.

I’m going to create a patch to rename all variables if no one objects.

#18

> The only case where this makes sense (that I’m aware of) is libc++,
given that it is a very different (and highly constrained) domain. I think
that lld and lldb following different coding standards is a serious bug,
which increases friction within the community.

That seems to be the general feeling, yes.

> I’d support progressively moving them to a style more similar to LLVM.

What I suggested on my second paragraph. :slight_smile:

I personally think that complete code refactoring is a bit pointless.
Then again, this is a decision to lld developers, I think.

Well, this may be my personal preference, but I don't like to keep the old
code in old style. Old code would be there untouched for years. LLD is
fortunately not a large project so it shouldn't be hard to convert it
entirely in one patch.

#19

Looks like most people in this thread support using LLVM style in LLD. I also had an offline discussion and many people wanted to have one coding style in all LLVM projects. So I’m convinced that we should do that.

I’m going to create a patch to rename all variables if no one objects.

I object!

-Nick

#20

Looks like most people in this thread support using LLVM style in LLD. I
also had an offline discussion and many people wanted to have one coding
style in all LLVM projects. So I'm convinced that we should do that.

I'm going to create a patch to rename all variables if no one objects.

I object!

I'd like to hear the reason. :slight_smile: