<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=Windows-1252">
</head>
<body>
<div style="color: rgb(33, 33, 33); background-color: rgb(255, 255, 255); text-align: left;" dir="auto">
Hi Mehdi, </div>
<div style="color: rgb(33, 33, 33); background-color: rgb(255, 255, 255); text-align: left;" dir="auto">
<br>
</div>
<div style="color: rgb(33, 33, 33); background-color: rgb(255, 255, 255); text-align: left;" dir="auto">
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? </div>
<div style="color: rgb(33, 33, 33); background-color: rgb(255, 255, 255); text-align: left;" dir="auto">
<br>
</div>
<div style="color: rgb(33, 33, 33); background-color: rgb(255, 255, 255); text-align: left;" dir="auto">
Thanks for the info about clang-format, that helps explain why the whole codebase isn't run through it already. </div>
<div style="color: rgb(33, 33, 33); background-color: rgb(255, 255, 255); text-align: left;" dir="auto">
<br>
</div>
<div style="color: rgb(33, 33, 33); background-color: rgb(255, 255, 255); text-align: left;" dir="auto">
Thanks </div>
<div style="color: rgb(33, 33, 33); background-color: rgb(255, 255, 255); text-align: left;" dir="auto">
David Truby </div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Mehdi AMINI <joker.eph@gmail.com><br>
<b>Sent:</b> Monday, March 2, 2020 5:35:13 PM<br>
<b>To:</b> David Truby <David.Truby@arm.com><br>
<b>Cc:</b> flang-dev@lists.llvm.org <flang-dev@lists.llvm.org>; Doerfert, Johannes <jdoerfert@anl.gov><br>
<b>Subject:</b> Re: [flang-dev] Code formatting alignment with LLVM</font>
<div> </div>
</div>
<div>
<div dir="ltr">
<div dir="ltr"><br>
</div>
<br>
<div class="x_gmail_quote">
<div dir="ltr" class="x_gmail_attr">On Mon, Mar 2, 2020 at 7:08 AM David Truby via flang-dev <<a href="mailto:flang-dev@lists.llvm.org">flang-dev@lists.llvm.org</a>> wrote:<br>
</div>
<blockquote class="x_gmail_quote" style="margin:0px 0px 0px 0.8ex; border-left-width:1px; border-left-style:solid; border-left-color:rgb(204,204,204); padding-left:1ex">
<div style="word-wrap:break-word; line-break:after-white-space">Hi all,
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>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 <span style="color:rgb(0,0,0)">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.</span></div>
<div><br>
</div>
<div>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.
</div>
</div>
</blockquote>
<div><br>
</div>
<div>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?</div>
<div> </div>
<blockquote class="x_gmail_quote" style="margin:0px 0px 0px 0.8ex; border-left-width:1px; border-left-style:solid; border-left-color:rgb(204,204,204); padding-left:1ex">
<div style="word-wrap:break-word; line-break:after-white-space">
<div>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.</div>
</div>
</blockquote>
<div><br>
</div>
<div>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.</div>
<div> </div>
<blockquote class="x_gmail_quote" style="margin:0px 0px 0px 0.8ex; border-left-width:1px; border-left-style:solid; border-left-color:rgb(204,204,204); padding-left:1ex">
<div style="word-wrap:break-word; line-break:after-white-space">
<div>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. </div>
</div>
</blockquote>
<blockquote class="x_gmail_quote" style="margin:0px 0px 0px 0.8ex; border-left-width:1px; border-left-style:solid; border-left-color:rgb(204,204,204); padding-left:1ex">
<div style="word-wrap:break-word; line-break:after-white-space">
<div>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. </div>
</div>
</blockquote>
<blockquote class="x_gmail_quote" style="margin:0px 0px 0px 0.8ex; border-left-width:1px; border-left-style:solid; border-left-color:rgb(204,204,204); padding-left:1ex">
<div style="word-wrap:break-word; line-break:after-white-space">
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>I hope this helps to move the conversation forward.</div>
<div><br>
</div>
<div>Thanks</div>
<div>David Truby<br>
<div><br>
<blockquote type="cite">
<div>On 11 Feb 2020, at 10:20, David Truby via flang-dev <<a href="mailto:flang-dev@lists.llvm.org" target="_blank">flang-dev@lists.llvm.org</a>> wrote:</div>
<br>
<div>
<div style="word-wrap:break-word; line-break:after-white-space">Hi all,
<div><br>
</div>
<div>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:
<a href="https://github.com/flang-compiler/f18/pull/945" target="_blank">https://github.com/flang-compiler/f18/pull/945</a>. 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. </div>
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>Does anyone else have any input on this?</div>
<div><br>
</div>
<div>Thanks</div>
<div>David Truby</div>
</div>
_______________________________________________<br>
flang-dev mailing list<br>
<a href="mailto:flang-dev@lists.llvm.org" target="_blank">flang-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/flang-dev" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/flang-dev</a><br>
</div>
</blockquote>
</div>
<br>
</div>
</div>
_______________________________________________<br>
flang-dev mailing list<br>
<a href="mailto:flang-dev@lists.llvm.org" target="_blank">flang-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/flang-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/flang-dev</a><br>
</blockquote>
</div>
</div>
</div>
</body>
</html>