<div dir="ltr">Both "significant increases" commits you linked to are to an external library used by polly, as far as I can tell. They're probably not the best examples to support your point.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, Oct 19, 2019 at 6:14 PM Stephen Kelly via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</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"><br>
Hi,<br>
<br>
I presented some features at EuroLLVM in April this year relating to <br>
improvements I made to AST Matcher tooling (clang-tidy and clang-query):<br>
<br>
<a href="https://steveire.wordpress.com/2019/04/30/the-future-of-ast-matching" rel="noreferrer" target="_blank">https://steveire.wordpress.com/2019/04/30/the-future-of-ast-matching</a><br>
<br>
My aims were:<br>
<br>
* make learning to use AST Matchers easier<br>
* make the clang-query tool itself help the user more<br>
* simplify the AST Matchers required by skipping non-visible nodes in <br>
matchers and AST dumps (such as implicit conversions, but see the talk <br>
for more).<br>
<br>
I think the javascript API I demo'd for writing clang-tidy checks could <br>
be a particularly valuable addition to the tools ecosystem.<br>
<br>
I didn't manage to get all of the patches upstream because arguments <br>
about using auto in the same way it is used in other parts of LLVM <br>
demotivated me. I haven't found an alternative reviewer to help get the <br>
patches upstream, though my attendance at EuroLLVM was partially an <br>
attempt to find one.<br>
<br>
Some people are still interested in the features getting upstream, but <br>
there is still some work for me to do in getting some of the patches <br>
from presentation-quality to commit-quality. So there is still <br>
motivation needed for my part. Beyond that, I have many other features I <br>
wish to contribute to clang tooling, so I don't really want to start <br>
again if I know I can't continue with it.<br>
<br>
The rule being applied in the reviews for those patches is approximately <br>
"if auto is used on the left, the type must appear on the right in angle <br>
brackets (such as in a cast)".<br>
<br>
I started a thread on this mailing list in November 2018 attempting to <br>
get the coding standard clarified regarding what is allowed and what is <br>
not regarding `auto`. There was no outcome from the thread. However, I <br>
included some crude measurements of the existing use of auto at the time <br>
using git grep:<br>
<br>
<br>
<a href="http://llvm.1065342.n5.nabble.com/llvm-dev-RFC-Modernizing-our-use-of-auto-td123947.html#a124564" rel="noreferrer" target="_blank">http://llvm.1065342.n5.nabble.com/llvm-dev-RFC-Modernizing-our-use-of-auto-td123947.html#a124564</a><br>
<br>
Here is a plot of the use of `auto` where there are no angle brackets on <br>
the right side of the equals sign, measured each week over the last <br>
three years:<br>
<br>
<a href="https://imgur.com/a/kcHaEhk" rel="noreferrer" target="_blank">https://imgur.com/a/kcHaEhk</a><br>
<br>
It was made with this script:<br>
<br>
<a href="https://gist.github.com/steveire/c9633d243d5d3b515e3eb26aaa3ad7c8" rel="noreferrer" target="_blank">https://gist.github.com/steveire/c9633d243d5d3b515e3eb26aaa3ad7c8</a><br>
<br>
It is quite conservative as it only counts use of auto in the lib <br>
directories of llvm, clang etc, excluding tests, include/ etc.<br>
<br>
It shows that occurances of auto - in ways that my patches are rejected <br>
for - has almost doubled in the last 3 years.<br>
<br>
I think it shows that the standard that "patches must use the returned <br>
type on the right side if using auto on the left" is not being followed <br>
in the code. However, that is how the standard is being used by some <br>
reviewers.<br>
<br>
If some of the major increases (and the decrease) intrigue you, the <br>
following patches are the responsible ones in the llvm-project repo):<br>
<br>
Significant increases:<br>
6151654c00cfcdb90fd665403948aab64b83494a<br>
d169d70bbf053f636aed25b482ec63efd094a95b<br>
Reductions:<br>
ddf3db9b5eddc6c7cc75fd30eea545c22ad70dcd<br>
a4fa0b880a63dad30d9ad1ff9fe8c2358c32dd77<br>
e372710d30c66e020576a2af6e1e5e815815a65a<br>
<br>
The MLIR team announced that they are following the LLVM coding guidelines:<br>
<br>
<a href="http://lists.llvm.org/pipermail/llvm-dev/2019-September/134992.html" rel="noreferrer" target="_blank">http://lists.llvm.org/pipermail/llvm-dev/2019-September/134992.html</a><br>
<br>
but they use auto extensively in ways that some reviewers don't accept:<br>
<br>
<a href="https://github.com/tensorflow/mlir/blob/master/lib/Parser/Parser.cpp" rel="noreferrer" target="_blank">https://github.com/tensorflow/mlir/blob/master/lib/Parser/Parser.cpp</a><br>
<br>
So, clearly other interpretations of the coding standard are possible. <br>
If my reviewer would not accept the MLIR code, then someone is <br>
misinterpreting the coding guideline.<br>
<br>
I think it would make sense for<br>
<br>
* The coding guideline<br>
* The code itself<br>
* The reviews<br>
<br>
to agree somewhat on what the coding guideline is regarding auto (and <br>
preferably have it enforced somehow). Currently, the reviews appear to <br>
be out of sync.<br>
<br>
If the rule should be "auto must not be used if the type is not on the <br>
right side", I'm sure I can write a clang-tidy check to convert most of <br>
the code. Such a tool would also enforce that new code follows the same <br>
rule.<br>
<br>
That would make it possible for me to finish the AST Matcher features <br>
and get them upstream. I don't mind not using auto, but I do wish for <br>
the coding guideline, the code itself and the reviews to agree on what <br>
the guideline is and for everyone (contributors and reviewers) to follow <br>
the same rules. That is not currently the case.<br>
<br>
An alternative (which I favor) is to change the coding standard along <br>
the lines of "use of auto is not encouraged, but it is not in itself a <br>
reason to reject the contribution".<br>
<br>
I would very much like to finish and upstream the AST Matcher improvements.<br>
<br>
Does anyone share the sentiment?<br>
<br>
Thanks,<br>
<br>
Stephen<br>
<br>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
</blockquote></div>