Code formatting alignment with LLVM

Hi Johannes,
I didn’t realise we could upload the same diff on Phabricator to look at, that’s really helpful!

I believe we are looking at lines 320-324 of PFTBuilder.h.
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.
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?

Maybe Tim can comment on what he thinks of this?

Thanks
David Truby

Hi Johannes,
I didn't realise we could upload the same diff on Phabricator to look at, that's really helpful!

I believe we are looking at lines 320-324 of PFTBuilder.h.

I thought we talk about alignment "issues" and this seems not to be one.

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.

I notice that openmp/runtime/.clang-format also uses “AlignTrailingComments: false”.

Tim

Hi Tim,

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

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?

Thanks
David Truby

The comments, I missed that somehow.

TBH, most of the code I see (in LLVM), and all the code I write, doesn't
have trailing comments to begin with. That would be my preferred
solution to this particular problem(, not all alignment issues though).

Not all "space only" changes look so confusing in a diff [0] so there
might be some way to improve it for the ones that currently do.

[0] https://reviews.llvm.org/D71830#change-WplcvCNm0Aq7