<html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<p><br>
</p>
<div class="moz-cite-prefix">On 6/30/20 1:19 PM, MyDeveloper Day via
llvm-dev wrote:<br>
</div>
<blockquote type="cite" cite="mid:CAD7AeN730wi-+BdNkCyWEZGRpRVrt6r2k=6eiEpA9UUc8FGB_w@mail.gmail.com">
<div dir="ltr">I 100% get that we might not like the decisions
clang-format is making, but how does one overcome this when
adding new code? The pre-merge checks enforce clang-formatting
before commit and that's a common review comment anyway for
those who didn't join the pre-merge checking group.</div>
</blockquote>
<p><br>
</p>
<p>Those checks are advisory. I do not recall a case where
thoughtfully-hand-formatted code was proposed, adhering to the
coding guidelines, and a reviewer asked for the file to be run
through clang-format. When the thoughtfully-formatted code does
not match what clang-format would produce in such cases, it's
generally obvious why. Normally, this request is made where the
file is inconsistently formatted, or formatted in a way that
obviously does not comply with our guidelines.</p>
<p>I've observed that there is a wide spectrum of developer
preferences: some thoughtfully and deliberately format all of
their code, others deal with formatting only when they feel like
they absolutely must and are happy to have a tool that does
anything consistent. Most people are probably somewhere in
between.<br>
</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:CAD7AeN730wi-+BdNkCyWEZGRpRVrt6r2k=6eiEpA9UUc8FGB_w@mail.gmail.com">
<div dir="ltr"> I'm just wondering are we not all following the
same guidelines?</div>
</blockquote>
<p><br>
</p>
<p>We have specific guidelines that everyone should follow.
clang-format implements a superset of those.</p>
<p>Here's a counter-proposal: You should hook up a tool that
automatically opens Phabricator reviews to clang-format the source
code. It should do this at a very low rate. It may take years to
get through everything. But, humans will look at the formatting
changes, and where clang-format should be improved instead of
changing the code, we'll discover and classify those things.
Formatted files can be noted, however, to incrementally build the
clang-format test suite.<br>
</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:CAD7AeN730wi-+BdNkCyWEZGRpRVrt6r2k=6eiEpA9UUc8FGB_w@mail.gmail.com">
<div dir="ltr">
<div><br>
</div>
<div>Concerns of clang-format not being good enough are fair
enough, but that's the area I'm focusing my development
efforts on (as that's where I've been trying to make a small
contribution). This effort was driven out of a need in my view
to have an extensive test suite to validate new changes to
clang-format don't break the formatting of pre-formatted code.
This isn't that possible with LLVM at the moment because large
parts are not formatted.</div>
<div><br>
</div>
<div>However checking a code base which is in high flux but one
that maintains a 100% clang-format clean status, is a near
perfect test-suite. Especially one that I assume will have
unit tests for the latest C++ features.</div>
<div><br>
</div>
<div>I'm not bored ;-) and whilst I understand this feels
somewhat periphery to the seemingly much more pressing task of
developing the next best compiler, I think we have to remember
that clang-format is extensively used. (In my view by many
more people than those actually using clang). GitHub reports
32,000 YAML files with the BasedOnStyle: LLVM alone that is
present in the .clang-format file</div>
</div>
</blockquote>
<p><br>
</p>
<p>Independent of everything else, this is a great success. Maybe we
should have a website with a tracker on it or something.</p>
<p>Thanks again,</p>
<p>Hal<br>
</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:CAD7AeN730wi-+BdNkCyWEZGRpRVrt6r2k=6eiEpA9UUc8FGB_w@mail.gmail.com">
<div dir="ltr">
<div><br>
</div>
<div>I realize it feels like unnecessary churn which is why I
suggested this be in code which hasn't been touched in some
time (I'd be fine if that time is 1+ years), but more often
than not this is quite small basic style issues, similar to
the ones below.</div>
<div><br>
</div>
<div>MyDeveloperDay</div>
<div><br>
</div>
<div>- void HandleTranslationUnit(ASTContext& context)
override {<br>
+ void HandleTranslationUnit(ASTContext &context)
override {<br>
if (!Instance.getLangOpts().DelayedTemplateParsing)<br>
return;<br>
<br>
- std::set<FunctionDecl*> LateParsedDecls;<br>
+ std::set<FunctionDecl *> LateParsedDecls;<br>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Tue, Jun 30, 2020 at 6:55
PM Matt Arsenault <<a href="mailto:arsenm2@gmail.com" moz-do-not-send="true">arsenm2@gmail.com</a>> wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div style="overflow-wrap: break-word;"><br>
<div><br>
<blockquote type="cite">
<div>On Jun 28, 2020, at 11:30, MyDeveloper Day via
llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank" moz-do-not-send="true">llvm-dev@lists.llvm.org</a>>
wrote:</div>
<br>
<div>
<p style="font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:15.008px">I’m
a contributor to clang-format and would like to see
LLVM 100% clang formatted so we can use LLVM as a
massive test-suite for clang-format when we make
changes.</p>
<br>
</div>
</blockquote>
</div>
<br>
<div>My main issue with this would be that clang-format does
things that I don’t believe are stated in the LLVM style
guide and I also disagree with. There’s a whole set of
cases where it makes unwelcome changes to put things that
were on separate lines on a single line. Whenever I run
clang-format, I typically git checkout -p to discard all
of these.</div>
<div><br>
</div>
<div>For example, it likes to take member initializer lists
and pack them into as few lines as possible. This is just
asking for merge conflicts (e.g. AMDGPUSubtarget has a ton
of fields in it, and out of tree code is constantly adding
new fields for unreleased subtargets). It also mangles
BuildMI calls, where I believe every .add() should be on
its own line, and stringing this into
BuildMI().addFoo().addBar() is way less readable.</div>
<div><br>
</div>
<div>I also believe it’s 4 space indent on line wraps
differs from the stated 2 space indent level (and this
also disagrees with emacs llvm-style)</div>
<div><br>
</div>
<div>-Matt</div>
</div>
</blockquote>
</div>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<pre class="moz-quote-pre" wrap="">_______________________________________________
LLVM Developers mailing list
<a class="moz-txt-link-abbreviated" href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>
<a class="moz-txt-link-freetext" href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a>
</pre>
</blockquote>
<pre class="moz-signature" cols="72">--
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory</pre>
</body>
</html>