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. 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.
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.