<div dir="auto"><div><br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Apr 20, 2021, 7:34 PM David Blaikie via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Tue, Apr 20, 2021 at 3:39 PM Stephen Kelly via llvm-dev<br>
<<a href="mailto:llvm-dev@lists.llvm.org" target="_blank" rel="noreferrer">llvm-dev@lists.llvm.org</a>> wrote:<br>
><br>
> On 19/04/2021 20:32, David Blaikie via llvm-dev wrote:<br>
> > eh, I think that's probably the wrong direction for LLVM, actually - I<br>
> > think we've generally encouraged "const" being explicit when it's<br>
> > otherwise wrapped up in "auto" - same as for *.<br>
><br>
> Neither make sense to be to be honest. I'd very much like to see<br>
> clang-tidy in reviews not complain about it. The '*' is quite easy to<br>
> miss and<br>
><br>
>      const auto *j1 = getPointer();<br>
>      const auto j2 = getPointer();<br>
><br>
> mean very different things.<br>
<br>
They do, which to me I think means it's valuable/important to include<br>
both const and * to clarify which thing is intended. It's valuable to<br>
know that something is a pointer - cheap to copy, non-owning (not a<br>
unique_ptr, don't have to use std::move on it), etc. It doesn't mean<br>
every type that is cheap to copy and non-owning is documented in this<br>
way - but does help for some types without making the type name<br>
significantly longer/making expressions more unwieldy, etc.<br>
<br>
(I'm surprised there wasn't much more discussion around it (perhaps<br>
there was on an llvm-dev thread or the like) when this rule first went<br>
in: <a href="https://github.com/llvm-mirror/llvm/commit/fc9031cdffa3063ef747bd3a98833f164d07fc4a#diff-38d8333325264c104bb94d32db2248c0384fd39d7dbd8512fb4bb4939e3cf2a4" rel="noreferrer noreferrer" target="_blank">https://github.com/llvm-mirror/llvm/commit/fc9031cdffa3063ef747bd3a98833f164d07fc4a#diff-38d8333325264c104bb94d32db2248c0384fd39d7dbd8512fb4bb4939e3cf2a4</a><br>
or<br>
<br>
> The latter is also easier to port to a smart<br>
> (or dumb) pointer.<br>
<br>
I think it's more important that the code is a bit easier to read than<br>
easier to modify in this way.<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Strong +1. As a reviewer, I prefer explicit pointer/reference and const qualifiers. Trying to infer that information, especially outside of an IDE, makes reviews significantly harder.</div><div dir="auto"><br></div><div dir="auto">~Aaron</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
- Dave<br>
<br>
><br>
> Thanks,<br>
><br>
> Stephen.<br>
><br>
> ><br>
> > On Mon, Apr 19, 2021 at 11:12 AM Nathan James via llvm-dev<br>
> > <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank" rel="noreferrer">llvm-dev@lists.llvm.org</a> <mailto:<a href="mailto:llvm-dev@lists.llvm.org" target="_blank" rel="noreferrer">llvm-dev@lists.llvm.org</a>>> wrote:<br>
> ><br>
> >     For the record the warning about turning `auto *X` into `const auto *X`<br>
> >     shouldn't be emitted, The check was adapted so that warning should no<br>
> >     longer be emitted in llvm code.<br>
> >     <a href="https://github.com/llvm/llvm-project/commit/8a68c40a1bf256523993ee97b39f79001eaade91" rel="noreferrer noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/8a68c40a1bf256523993ee97b39f79001eaade91</a><br>
> >     <<a href="https://github.com/llvm/llvm-project/commit/8a68c40a1bf256523993ee97b39f79001eaade91" rel="noreferrer noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/8a68c40a1bf256523993ee97b39f79001eaade91</a>><br>
> ><br>
> >     I can only guess that the pre-merge build bot is using an old build of<br>
> >     clang-tidy as that commit should be in the 11.0.0 release.<br>
> ><br>
> >     ~Nathan James<br>
> ><br>
> >     On Mon, 2021-04-19 at 09:43 +0000, Maxim Kazantsev via llvm-dev wrote:<br>
> >      > Hello everyone,<br>
> >      ><br>
> >      > I started noticing that lately we’ve improved reporting from clang-<br>
> >      > tidy, pointing out at various formatting issues. However the more<br>
> >      > verbose it becomes, the more annoyed I feel about it. For example<br>
> >      > here:<br>
> >      ><br>
> >      > <a href="https://reviews.llvm.org/D100721" rel="noreferrer noreferrer" target="_blank">https://reviews.llvm.org/D100721</a> <<a href="https://reviews.llvm.org/D100721" rel="noreferrer noreferrer" target="_blank">https://reviews.llvm.org/D100721</a>><br>
> >      ><br>
> >      > It complains literally about every second line, inserting its<br>
> >      > comments straight into review. They take as much space as the actual<br>
> >      > code. Maybe it’s just me, but it’s really hard to me to understand<br>
> >      > what the patch is actually doing with so many inlined auto-generated<br>
> >      > comments. Maybe there is a button to hide them somewhere, but I<br>
> >      > failed to find it.<br>
> >      ><br>
> >      > I understand what was the intention, and clang-tidy is a cool thing<br>
> >      > in general, but it’s getting too intrusive. Does anyone else have the<br>
> >      > same problem as I do? If there’s a lot of people whom it annoys,<br>
> >      > maybe we should think how to make it less invasive. Maybe it should<br>
> >      > put these comment when the patch gets approved, or something like<br>
> >      > this.<br>
> >      ><br>
> >      > Thanks,<br>
> >      > Max<br>
> >      ><br>
> >      > _______________________________________________<br>
> >      > LLVM Developers mailing list<br>
> >      > <a href="mailto:llvm-dev@lists.llvm.org" target="_blank" rel="noreferrer">llvm-dev@lists.llvm.org</a> <mailto:<a href="mailto:llvm-dev@lists.llvm.org" target="_blank" rel="noreferrer">llvm-dev@lists.llvm.org</a>><br>
> >      > <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
> >     <<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a>><br>
> ><br>
> >     _______________________________________________<br>
> >     LLVM Developers mailing list<br>
> >     <a href="mailto:llvm-dev@lists.llvm.org" target="_blank" rel="noreferrer">llvm-dev@lists.llvm.org</a> <mailto:<a href="mailto:llvm-dev@lists.llvm.org" target="_blank" rel="noreferrer">llvm-dev@lists.llvm.org</a>><br>
> >     <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
> >     <<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a>><br>
> ><br>
> ><br>
> > _______________________________________________<br>
> > LLVM Developers mailing list<br>
> > <a href="mailto:llvm-dev@lists.llvm.org" target="_blank" rel="noreferrer">llvm-dev@lists.llvm.org</a><br>
> > <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
> ><br>
><br>
><br>
> _______________________________________________<br>
> LLVM Developers mailing list<br>
> <a href="mailto:llvm-dev@lists.llvm.org" target="_blank" rel="noreferrer">llvm-dev@lists.llvm.org</a><br>
> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank" rel="noreferrer">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div></div></div>