[llvm-dev] clang-tidy makes review a pain

[David Blaikie via llvm-dev]

Generally, if clang-tidy feedback is correct/actionable I think it's fine -
if it's muddying up the review, then asking the author to address the
clang-tidy feedback first, before you review the rest seems like a good
approach to me.

On Mon, Apr 19, 2021 at 9:31 AM Chris Tetreault via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

> I agree that the clang-tidy reporting on your linked patch is quite
> egregious. I think that clang-tidy should only post feedback out of line.
> Ideally, it should only complain about actual violations of the llvm coding
> standards. I see nothing in the coding standards about requiring or
> preferring that stack locals being declared const if they can be.
>

If that were the policy, I'd object to it (top level const certainly isn't
done pervasively in LLVM and I'd find it "quirky" (& I do push back on it
where it's not already in use as a micro-convention in some file/area of
the code)), but the policy clang-tidy seems to be enforcing is to use
"const" on "auto&" and "auto*" where possible, which while not quite
written in the LLVM style guide, I think has been discussed/generally
encouraged in review - the style guide sort of alludes to it in the example
here:
https://llvm.org/docs/CodingStandards.html#beware-unnecessary-copies-with-auto



>
>
> Thanks,
>
>                 Christopher Tetreault
>
>
>
> *From:* llvm-dev <llvm-dev-bounces at lists.llvm.org> *On Behalf Of *Maxim
> Kazantsev via llvm-dev
> *Sent:* Monday, April 19, 2021 2:43 AM
> *To:* llvm-dev at lists.llvm.org
> *Subject:* [EXT] [llvm-dev] clang-tidy makes review a pain
>
>
>
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210419/f727fb4c/attachment.html>