aaron.ballman added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp:46
+ CheckFactories.registerCheck<RedundantConditionCheck>(
+ "misc-redundant-condition");
CheckFactories.registerCheck<RedundantExpressionCheck>(
----------------
I think this check should probably live in the `bugprone` module, WDYT?
================
Comment at: clang-tools-extra/clang-tidy/misc/RedundantConditionCheck.cpp:32
+ const auto &SM = Context->getSourceManager();
+ if (const Stmt *MutS = MutAn.findMutation(Var)) {
+ if (SM.isBeforeInTranslationUnit(MutS->getEndLoc(), NextS->getBeginLoc()))
----------------
This code can be simplified into:
```
const Stmt *MutS = MutAn.findMutation(Var);
return MutS && SM.isBeforeInTranslationUnit(MutS->getEndLoc(),
NextS->getBeginLoc());
```
================
Comment at: clang-tools-extra/clang-tidy/misc/RedundantConditionCheck.cpp:73
+ // If the variable has an alias then it can be changed by that alias as well.
+ // FIXME: Track pointers and references.
+ if (hasPtrOrReferenceInFunc(Func, CondVar))
----------------
This FIXME makes me worried that what we really want is a clang static analyzer
check, since the check is already flow sensitive and needs to be data sensitive
as well. Have you considered implementing this as a static analyzer check?
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/misc-redundant-condition.cpp:1097
+ }
+}
----------------
Can you add some tests that exercise GNU expression statements, just to make
sure we get the behavior correct?
```
if (({foo;})) {
} else if (({foo;})) {
}
if (foo) ({bar;});
else if (foo) ({bar;});
```
Another thing that's interesting to test is whether the redundant expression is
in a subexpression which doesn't contribute to the value of the control
expression:
```
if (foo(), val) {
} else if (foo(), other_val) {
}
```
(Similar can be tested with GNU statement expressions.)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81272/new/
https://reviews.llvm.org/D81272
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
