[flang-dev] Code formatting alignment with LLVM

Mon Mar 2 09:40:13 PST 2020

Hi Mehdi,

Thanks for the reply. My understanding of the concern is that if you change a line that has trailing comments in a block for example, and the length of the line changes, all the comments on other lines will be realigned, and the diff will show every line as having changed even though only one has. Does that make sense?

Thanks for the info about clang-format, that helps explain why the whole codebase isn't run through it already.

Thanks
David Truby
________________________________
From: Mehdi AMINI <joker.eph at gmail.com>
Sent: Monday, March 2, 2020 5:35:13 PM
To: David Truby <David.Truby at arm.com>
Cc: flang-dev at lists.llvm.org <flang-dev at lists.llvm.org>; Doerfert, Johannes <jdoerfert at anl.gov>
Subject: Re: [flang-dev] Code formatting alignment with LLVM

On Mon, Mar 2, 2020 at 7:08 AM David Truby via flang-dev <flang-dev at lists.llvm.org<mailto:flang-dev at lists.llvm.org>> wrote:
Hi all,

Since we haven’t seen any movement on this in quite a while and our projected merge date is moving ever closer, I wonder if I could summarise the positions that have been given so far so we can decide on a way forward. If I misrepresent what you’ve stated at all please reply to correct me, I’m just trying to give a brief summary.

I opened the discussion with a PR to see what the projected changes would look like; I did this for two reasons, firstly that we were requested to in a discussion about merging on llvm-dev, but also because I believe we should align with the rest of the community on a single coding style. My position is that I have no preference at all in what the style is, I just think there should be a single one across the community. My reason for thinking this is that fairly likely that changes to f18, mlir and llvm will be mixed in together once we land, and working on a single change that has multiple different coding styles within it may lead to some issues for anyone working on those.

Tim Keith and Peter Klausler from Nvidia replied to say that comment alignment (and code alignment in general) cause spurious changes to a appear when submitting diffs, making it more difficult to see what has changed.

Can you clarify the concern here? If there is a one-time formatting of the codebase now (LLDB did such a one-time formatting change to align with LLVM a few years ago), then any future patches correctly formatted should not have any spurious change right?

Tim Keith also pointed out that clang and llvm are not properly clang-formatted entirely themselves, which weakens the “common code style across the board” desire.

It is worth noting that a lot of the codebase predates clang-format, when there wasn't any tool to help formatting we could only rely on code review and best effort.

Eric Schweitz replied to show MLIR’s clang-format file, which is also the one used for the lib/Lower subdirectory of f18. This style has only one small change from the LLVM style, but it is nonetheless a diversion.
Eric also pointed out that clang-formatting for new code is enforced on Phabricator by pre-commit bots. This is the case not only for mlir, but for new code heading to clang and llvm too. This is consistent with the Coding Guidelines which do not require that all code is properly clang-formatted, only all new code. Eric also suggested that we could submit diffs without running clang-format, so that the changes are clear, and then once approved run clang-format to match the code with the rest of the project. This might mitigate the code alignment issues.

Johannes Doerfert stated his preference for minimizing clang-format differences from the rest of the project, in part to ease our acceptance by the community. He also stated that he is used to the llvm settings and so anything different makes his workflow uneasy. I believe this goes hand in hand with my statement about working with different coding styles in a single change.

On the F18 community call, Hal Finkel explained the reasoning behind the LLVM coding style decisions. He stated that LLVM prefers readability of code over writability (or reviewability) as code is much more likely to be read than written or reviewed. This is why LLVM uses code and comment alignment in its coding style. On the call it was also discussed that some people do not find that the alignment makes code easier to read and that this seems to be subjective.

I think that this is the point the discussions have reached so far, if I have missed anything or misrepresented anyone’s position please reply to this to clarify.

Given the impasse we have got to, I suggest that if we cannot find a compromise here we should take the discussion to llvm-dev and see how the wider community feels about a large diversion from the coding style for a specific sub-project. It may be that this is not a hard block for merging, however it is something that was requested of us initially so it is something we need to discuss there if we decide not to do it.

I hope this helps to move the conversation forward.

Thanks
David Truby

On 11 Feb 2020, at 10:20, David Truby via flang-dev <flang-dev at lists.llvm.org<mailto:flang-dev at lists.llvm.org>> wrote:

Hi all,

We have been having a discussion on the GitHub issue tracker about code formatting (and specifically clang-format settings) and whether to align closer with the rest of the project, which you can find here: https://github.com/flang-compiler/f18/pull/945. Since the discussion there hasn’t moved much recently I’d like to start a discussion here so we can get input from a wider group of people.

My opinion is that regardless of technical preferences we shouldn’t diverge from the style of the rest of the project as much as we currently do, or at least if we want to do that then we should have a discussion with the wider community about whether that is acceptable to them.

Does anyone else have any input on this?

Thanks
David Truby
_______________________________________________
flang-dev mailing list
flang-dev at lists.llvm.org<mailto:flang-dev at lists.llvm.org>
https://lists.llvm.org/cgi-bin/mailman/listinfo/flang-dev

_______________________________________________
flang-dev mailing list
flang-dev at lists.llvm.org<mailto:flang-dev at lists.llvm.org>
https://lists.llvm.org/cgi-bin/mailman/listinfo/flang-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/flang-dev/attachments/20200302/af30fba0/attachment.html>