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

alexeyr added a comment. Ping. Are any other changes needed? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73775/new/ https://reviews.llvm.org/D73775 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mai

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

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a minor nit. Comment at: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp:355 + collectOperands(Operands.second, AllOper

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

alexeyr updated this revision to Diff 244369. alexeyr added a comment. @aaron.ballman Updated. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73775/new/ https://reviews.llvm.org/D73775 Files: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp clang-tools-extra/test/clang

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

alexeyr updated this revision to Diff 244370. alexeyr added a comment. Actually updated. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73775/new/ https://reviews.llvm.org/D73775 Files: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp clang-tools-extra/test/clang-tidy/

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

aaron.ballman accepted this revision. aaron.ballman added a comment. Still LGTM, thank you! If you need me to commit this on your behalf, I'm happy to do so, just let me know. It'll have to wait until next week, though (unless someone else does it first). CHANGES SINCE LAST ACTION https://re

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

alexeyr added a comment. Yes, I do. Thank you (and @Eugene.Zelenko) for your help making this better! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73775/new/ https://reviews.llvm.org/D73775 ___ cfe-commits mailing list cfe-commits@lists.ll

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

alexeyr added a comment. @aaron.ballman I just noticed this in https://mlir.llvm.org/getting_started/Contributing/ > Note: if you haven’t used Arcanist to send your patch for review, committers > don’t have access to your preferred identity for commit messages. Make sure > to communicate it to

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

aaron.ballman closed this revision. aaron.ballman added a comment. In D73775#1875921 , @alexeyr wrote: > @aaron.ballman I just noticed this in > https://mlir.llvm.org/getting_started/Contributing/ > > > Note: if you haven’t used Arcanist to send your patc

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

alexeyr created this revision. alexeyr added reviewers: alexfh, etienneb, hgabii. alexeyr added a project: clang-tools-extra. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. readability-redundant-expression could detect expressions where a logical or bitwise opera

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

alexeyr added a comment. Note: during testing I found that `++X && ++X` is a false positive, my current implementation also introduces `X && ++X && X` which I'll try to fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73775/new/ https://reviews.llvm.org/D73775

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

alexeyr updated this revision to Diff 241696. alexeyr added a comment. Fixed test broken by git clang-format CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73775/new/ https://reviews.llvm.org/D73775 Files: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp clang-tools-ex

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

alexeyr added a comment. Also I am not sure why, but the ranges added to the diagnostic in lines 1222-1226 don't show up in the message. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73775/new/ https://reviews.llvm.org/D73775 ___ cfe-commi

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

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp:335 +return AsTExpr; + else +return nullptr; Please don;e use else after return. Comment at: clang-tools-extra/clang-tidy/

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

alexeyr added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp:363 + ASTContext &Context) { + const auto OpKind = getOp(TheExpr); + // if there are no nested operators of the same kind, it's handled by

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

alexeyr updated this revision to Diff 242000. alexeyr added a comment. Followed Eugene Zelenko's comments and fixed a false positive. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73775/new/ https://reviews.llvm.org/D73775 Files: clang-tools-ext

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

alexeyr marked 11 inline comments as done. alexeyr added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp:363 + ASTContext &Context) { + const auto OpKind = getOp(TheExpr); + // if there are no nested operators of

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

aaron.ballman added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp:114 if (P.a[X++] != P.a[X++]) return 1; + if (X && X++ && X) return 1; What do you think about the following? ``` bool foo(int&); bool

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

alexeyr marked an inline comment as done. alexeyr added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp:114 if (P.a[X++] != P.a[X++]) return 1; + if (X && X++ && X) return 1; aaron.ballman wrote: > What

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

aaron.ballman added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp:114 if (P.a[X++] != P.a[X++]) return 1; + if (X && X++ && X) return 1; alexeyr wrote: > aaron.ballman wrote: > > What do you think abo

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

alexeyr updated this revision to Diff 242266. alexeyr added a comment. Added tests for overloaded operators and fixed parent check logic. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73775/new/ https://reviews.llvm.org/D73775 Files: clang-tools-extra/clang-tidy/misc/RedundantExpress

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

alexeyr marked an inline comment as done. alexeyr added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp:114 if (P.a[X++] != P.a[X++]) return 1; + if (X && X++ && X) return 1; alexeyr wrote: > aaron.ball

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

alexeyr marked an inline comment as done. alexeyr added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp:114 if (P.a[X++] != P.a[X++]) return 1; + if (X && X++ && X) return 1; aaron.ballman wrote: > alex

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

aaron.ballman added a comment. In 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

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

alexeyr added a comment. In D73775#1856765 , @aaron.ballman wrote: > In 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

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

aaron.ballman added a comment. In D73775#1856848 , @alexeyr wrote: > In D73775#1856765 , @aaron.ballman > wrote: > > > In D73775#1851553 , @alexeyr wrote: > > > > > Also I a

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

alexeyr added a comment. In D73775#1856869 , @aaron.ballman wrote: > In D73775#1856848 , @alexeyr wrote: > > > In D73775#1856765 , @aaron.ballman > > wrote: > > > > > In D7

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

aaron.ballman added a comment. In D73775#1859156 , @alexeyr wrote: > In D73775#1856869 , @aaron.ballman > wrote: > > > In D73775#1856848 , @alexeyr wrote: > > > > > In D7377

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

alexeyr updated this revision to Diff 242572. alexeyr added a comment. Lower-case warning messages CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73775/new/ https://reviews.llvm.org/D73775 Files: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp clang-tools-extra/test/c

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

alexeyr added a comment. In D73775#1859167 , @aaron.ballman wrote: > Huh, this does seem like it may be a bug in clang-tidy. I would have expected > `ClangTidyDiagnosticConsumer::forwardDiagnostic()` to do this work. From my debugging attempt it seems