<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=Windows-1252">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Hi Tim,</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
We are currently under a different level of scrutiny to existing established and successful LLVM projects, and one need only look at this week's llgo removal to see why the community is more concerned about adding new frontends than it may previously have been.</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
I would personally suggest that if other LLVM projects have disalignment from the overall LLVM style here and it is undocumented why then that is a defect that we should raise diffs on Phabricator for those too. However I think that it's separate from and orthogonal
to the discussion we are having about flang.</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
With regards to Phabricator it is unfortunate that it doesn't show these diffs in a better way. Is it possible to configure it to do so?
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Thanks</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
David Truby<br>
</div>
<div id="appendonsend"></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> Timothy Keith <tkeith@nvidia.com><br>
<b>Sent:</b> 13 February 2020 23:29<br>
<b>To:</b> David Truby <David.Truby@arm.com>; Johannes Doerfert <jdoerfert@anl.gov><br>
<b>Cc:</b> David Truby via flang-dev <flang-dev@lists.llvm.org><br>
<b>Subject:</b> Re: [flang-dev] Code formatting alignment with LLVM</font>
<div> </div>
</div>
<style>
<!--
@font-face
{font-family:"Cambria Math"}
@font-face
{font-family:Calibri}
@font-face
{font-family:Verdana}
p.x_MsoNormal, li.x_MsoNormal, div.x_MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif}
a:link, span.x_MsoHyperlink
{color:blue;
text-decoration:underline}
span.x_EmailStyle19
{font-family:"Calibri",sans-serif;
color:windowtext}
.x_MsoChpDefault
{font-size:10.0pt}
@page WordSection1
{margin:1.0in 1.0in 1.0in 1.0in}
div.x_WordSection1
{}
-->
</style>
<div lang="EN-US" link="blue" vlink="purple">
<div class="x_WordSection1">
<p class="x_MsoNormal">If AlignTrailingComments had been set to false in .clang-format, lines 321 and 322 would not have changed and so Phabricator would not show them as having changed. It’s no better than in the github PR.</p>
<p class="x_MsoNormal"> </p>
<p class="x_MsoNormal">I notice that openmp/runtime/.clang-format also uses “AlignTrailingComments: false”.</p>
<p class="x_MsoNormal"> </p>
<p class="x_MsoNormal">Tim</p>
<p class="x_MsoNormal"> </p>
<div style="border:none; border-top:solid #B5C4DF 1.0pt; padding:3.0pt 0in 0in 0in">
<p class="x_MsoNormal"><b><span style="font-size:12.0pt; color:black">From: </span>
</b><span style="font-size:12.0pt; color:black">flang-dev <flang-dev-bounces@lists.llvm.org> on behalf of David Truby via flang-dev <flang-dev@lists.llvm.org><br>
<b>Reply-To: </b>David Truby <David.Truby@arm.com><br>
<b>Date: </b>Thursday, February 13, 2020 at 1:35 PM<br>
<b>To: </b>Johannes Doerfert <jdoerfert@anl.gov><br>
<b>Cc: </b>David Truby via flang-dev <flang-dev@lists.llvm.org><br>
<b>Subject: </b>Re: [flang-dev] Code formatting alignment with LLVM</span></p>
</div>
<div>
<p class="x_MsoNormal"> </p>
</div>
<table class="x_MsoNormalTable" border="1" cellpadding="0" style="background:#FFEB9C">
<tbody>
<tr>
<td style="padding:.75pt .75pt .75pt .75pt">
<p class="x_MsoNormal"><b><span style="font-size:7.5pt; font-family:"Verdana",sans-serif; color:black">External email: Use caution opening links or attachments</span></b><span style="font-size:7.5pt; font-family:"Verdana",sans-serif; color:black">
</span></p>
</td>
</tr>
</tbody>
</table>
<p class="x_MsoNormal"> </p>
<div>
<div>
<p class="x_MsoNormal"><span style="font-size:12.0pt; color:black">Hi Johannes,</span></p>
</div>
<div>
<p class="x_MsoNormal"><span style="font-size:12.0pt; color:black">I didn't realise we could upload the same diff on Phabricator to look at, that's really helpful!</span></p>
</div>
<div>
<p class="x_MsoNormal"><span style="font-size:12.0pt; color:black"> </span></p>
</div>
<div>
<p class="x_MsoNormal"><span style="font-size:12.0pt; color:black">I believe we are looking at lines 320-324 of PFTBuilder.h.</span></p>
</div>
<div>
<p class="x_MsoNormal"><span style="font-size:12.0pt; color:black">I don't see a real significant difference in the way this is displayed I suppose. Phabricator doesn't pick up that lines 323 and 324 on the right are the same as 312, 322 on the left sans formatting.</span></p>
</div>
<div>
<p class="x_MsoNormal"><span style="font-size:12.0pt; color:black">However, I think that even if the formatting was the same Phabricator would display this in the same way as that's just how it displays diffs?</span></p>
</div>
<div>
<p class="x_MsoNormal"><span style="font-size:12.0pt; color:black"> </span></p>
</div>
<div>
<p class="x_MsoNormal"><span style="font-size:12.0pt; color:black">Maybe Tim can comment on what he thinks of this?</span></p>
</div>
<div>
<p class="x_MsoNormal"><span style="font-size:12.0pt; color:black"> </span></p>
</div>
<div>
<p class="x_MsoNormal"><span style="font-size:12.0pt; color:black">Thanks</span></p>
</div>
<div>
<p class="x_MsoNormal"><span style="font-size:12.0pt; color:black">David Truby</span></p>
</div>
<div>
<p class="x_MsoNormal"><span style="font-size:12.0pt; color:black"> </span></p>
</div>
<div>
<p class="x_MsoNormal"><span style="font-size:12.0pt; color:black"> </span></p>
<div class="x_MsoNormal" align="center" style="text-align:center"><span style="font-size:12.0pt; color:black">
<hr size="2" width="98%" align="center">
</span></div>
<p class="x_MsoNormal"><b><span style="font-size:12.0pt; color:black">From:</span></b><span style="font-size:12.0pt; color:black"> Johannes Doerfert<br>
<b>Sent:</b> Thursday, 13 February 2020 21:27<br>
<b>To:</b> David Truby<br>
<b>Cc:</b> David Truby via flang-dev<br>
<b>Subject:</b> Re: [flang-dev] Code formatting alignment with LLVM </span></p>
<div>
<p class="x_MsoNormal"><span style="font-size:12.0pt; color:black"> </span></p>
</div>
</div>
<div>
<div>
<p class="x_MsoNormal">On 02/13, David Truby via flang-dev wrote:<br>
> Hi all,<br>
> <br>
> Tim Kieth from Nvidia has provided a specific example from the recent<br>
> FIR pull request where the indentation rules caused spurious diffs<br>
> (see this comment:<br>
> <a href="https://github.com/flang-compiler/f18/pull/945#issuecomment-585856610" target="_blank">
https://github.com/flang-compiler/f18/pull/945#issuecomment-585856610</a>).<br>
> <br>
> I'm curious if this is also something that shows up badly in<br>
> Phabricator or if that has a better way of showing diffs involving<br>
> whitespace. Is anyone familiar enough with Phabricator to comment on<br>
> this? I wonder if Phabricator might just be better at handling code<br>
> reviews of this ilk better, meaning that this problem is less visible<br>
> to clang/llvm developers.<br>
> <br>
<br>
Here is the diff:<br>
<a href="https://reviews.llvm.org/differential/diff/244525/" target="_blank">https://reviews.llvm.org/differential/diff/244525/</a><br>
<br>
What line numbers are we looking at again?<br>
<br>
<br>
> Thanks<br>
> David Truby<br>
> <br>
> ________________________________<br>
> From: flang-dev <flang-dev-bounces@lists.llvm.org> on behalf of David Truby via flang-dev <flang-dev@lists.llvm.org><br>
> Sent: 11 February 2020 10:20<br>
> To: flang-dev@lists.llvm.org <flang-dev@lists.llvm.org><br>
> Subject: [flang-dev] Code formatting alignment with LLVM<br>
> <br>
> Hi all,<br>
> <br>
> 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.<br>
> <br>
> 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.<br>
> <br>
> Does anyone else have any input on this?<br>
> <br>
> Thanks<br>
> David Truby<br>
<br>
> _______________________________________________<br>
> flang-dev mailing list<br>
> flang-dev@lists.llvm.org<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>
<br>
<br>
-- <br>
<br>
Johannes Doerfert<br>
Researcher<br>
<br>
Argonne National Laboratory<br>
Lemont, IL 60439, USA<br>
<br>
jdoerfert@anl.gov</p>
</div>
</div>
</div>
</div>
<div>
<hr>
</div>
<div>This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message. </div>
<div>
<hr>
</div>
</div>
</body>
</html>