[PATCH] D73775: [clang-tidy] Cover cases like (b && c && b) in the redundant expression check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D73775#1859156 <https://reviews.llvm.org/D73775#1859156>, @alexeyr wrote:

> In D73775#1856869 <https://reviews.llvm.org/D73775#1856869>, @aaron.ballman wrote:
>
> > In D73775#1856848 <https://reviews.llvm.org/D73775#1856848>, @alexeyr wrote:
> >
> > > In D73775#1856765 <https://reviews.llvm.org/D73775#1856765>, @aaron.ballman wrote:
> > >
> > > > In D73775#1851553 <https://reviews.llvm.org/D73775#1851553>, @alexeyr wrote:
> > > >
> > > > > Also I am not sure why, but the ranges added to the diagnostic in lines 1222-1226 don't show up in the message.
> > > >
> > > >
> > > > Do you mean that there are no squiggly underlines under the range, or something else? Passing in a range to the diagnostics engine gives it something to put an underline under, as in what's under the -12 in: https://godbolt.org/z/GeQzYg
> > >
> > >
> > > Yes, exactly. I expect to see the underlines, but don't; only the `^` in the location provided to `diag` call.
> >
> >
> > The only time I've seen that happen is when the range is invalid. I'm guessing you'll have to step through the diagnostics code to see what's going on.
>
>
> After stepping through diagnostics code, I... don't understand how it is supposed to work. `ClangTidyDiagnosticConsumer` is
>
> > A diagnostic consumer that turns each Diagnostic into a SourceManager-independent ClangTidyError.
>
> And it adds the //hints// to the `ClangTidyError` here <https://github.com/llvm/llvm-project/blob/706256b6d3972dcfa2d3e888d0640e1689c4be95/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp#L78-L96>:
>
>   for (const auto &FixIt : Hints) { ... }
>
>
> But not the ranges, and I don't see any place where it does or even how they can be inserted into a `ClangTidyError` in the first place!


Huh, this does seem like it may be a bug in clang-tidy. I would have expected `ClangTidyDiagnosticConsumer::forwardDiagnostic()` to do this work.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73775/new/

https://reviews.llvm.org/D73775