David Blaikie via llvm-dev
2021-Apr-19 19:32 UTC
[llvm-dev] clang-tidy makes review a pain
eh, I think that's probably the wrong direction for LLVM, actually - I think we've generally encouraged "const" being explicit when it's otherwise wrapped up in "auto" - same as for *. On Mon, Apr 19, 2021 at 11:12 AM Nathan James via llvm-dev < llvm-dev at lists.llvm.org> wrote:> For the record the warning about turning `auto *X` into `const auto *X` > shouldn't be emitted, The check was adapted so that warning should no > longer be emitted in llvm code. > > https://github.com/llvm/llvm-project/commit/8a68c40a1bf256523993ee97b39f79001eaade91 > > I can only guess that the pre-merge build bot is using an old build of > clang-tidy as that commit should be in the 11.0.0 release. > > ~Nathan James > > On Mon, 2021-04-19 at 09:43 +0000, Maxim Kazantsev via llvm-dev wrote: > > Hello everyone, > > > > I started noticing that lately we’ve improved reporting from clang- > > tidy, pointing out at various formatting issues. However the more > > verbose it becomes, the more annoyed I feel about it. For example > > here: > > > > https://reviews.llvm.org/D100721 > > > > It complains literally about every second line, inserting its > > comments straight into review. They take as much space as the actual > > code. Maybe it’s just me, but it’s really hard to me to understand > > what the patch is actually doing with so many inlined auto-generated > > comments. Maybe there is a button to hide them somewhere, but I > > failed to find it. > > > > I understand what was the intention, and clang-tidy is a cool thing > > in general, but it’s getting too intrusive. Does anyone else have the > > same problem as I do? If there’s a lot of people whom it annoys, > > maybe we should think how to make it less invasive. Maybe it should > > put these comment when the patch gets approved, or something like > > this. > > > > Thanks, > > Max > > > > _______________________________________________ > > LLVM Developers mailing list > > llvm-dev at lists.llvm.org > > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210419/e7526426/attachment.html>
Stephen Kelly via llvm-dev
2021-Apr-20 22:38 UTC
[llvm-dev] clang-tidy makes review a pain
On 19/04/2021 20:32, David Blaikie via llvm-dev wrote:> eh, I think that's probably the wrong direction for LLVM, actually - I > think we've generally encouraged "const" being explicit when it's > otherwise wrapped up in "auto" - same as for *.Neither make sense to be to be honest. I'd very much like to see clang-tidy in reviews not complain about it. The '*' is quite easy to miss and const auto *j1 = getPointer(); const auto j2 = getPointer(); mean very different things. The latter is also easier to port to a smart (or dumb) pointer. Thanks, Stephen.> > On Mon, Apr 19, 2021 at 11:12 AM Nathan James via llvm-dev > <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote: > > For the record the warning about turning `auto *X` into `const auto *X` > shouldn't be emitted, The check was adapted so that warning should no > longer be emitted in llvm code. > https://github.com/llvm/llvm-project/commit/8a68c40a1bf256523993ee97b39f79001eaade91 > <https://github.com/llvm/llvm-project/commit/8a68c40a1bf256523993ee97b39f79001eaade91> > > I can only guess that the pre-merge build bot is using an old build of > clang-tidy as that commit should be in the 11.0.0 release. > > ~Nathan James > > On Mon, 2021-04-19 at 09:43 +0000, Maxim Kazantsev via llvm-dev wrote: > > Hello everyone, > > > > I started noticing that lately we’ve improved reporting from clang- > > tidy, pointing out at various formatting issues. However the more > > verbose it becomes, the more annoyed I feel about it. For example > > here: > > > > https://reviews.llvm.org/D100721 <https://reviews.llvm.org/D100721> > > > > It complains literally about every second line, inserting its > > comments straight into review. They take as much space as the actual > > code. Maybe it’s just me, but it’s really hard to me to understand > > what the patch is actually doing with so many inlined auto-generated > > comments. Maybe there is a button to hide them somewhere, but I > > failed to find it. > > > > I understand what was the intention, and clang-tidy is a cool thing > > in general, but it’s getting too intrusive. Does anyone else have the > > same problem as I do? If there’s a lot of people whom it annoys, > > maybe we should think how to make it less invasive. Maybe it should > > put these comment when the patch gets approved, or something like > > this. > > > > Thanks, > > Max > > > > _______________________________________________ > > LLVM Developers mailing list > > llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org> > > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > <https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev> > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > <https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev> > > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >