baloghadamsoftware created this revision.
baloghadamsoftware added reviewers: aaron.ballman, gribozavr2.
baloghadamsoftware added a project: clang-tools-extra.
Herald added subscribers: martong, steakhal, gamesh411, Szelethus, dkrupp,
rnkovacs, xazax.hun, whisperity, mgorny.
Herald added a project: clang.
baloghadamsoftware marked an inline comment as done.
baloghadamsoftware added a comment.
This check was made upon user request. I think it is a good base that can later
be extended also for the negated cases (where the code inside the inner `if`
never executes). That case is even more suspicious. I put it into `misc`
because it is not such a bug that it should go into `bugprone` but rather a
suspicious thing. The type of the bug is somewhat similar to the bugs detected
by `misc-redundant-expression` so I think `misc` is the best place also for
this one.
================
Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:135
`cppcoreguidelines-avoid-goto <cppcoreguidelines-avoid-goto.html>`_,
+ `cppcoreguidelines-avoid-non-const-global-variables
<cppcoreguidelines-avoid-non-const-global-variables.html>`_,
`cppcoreguidelines-init-variables
<cppcoreguidelines-init-variables.html>`_, "Yes"
----------------
What are these changes? I surely did not make them? Maybe the check adder
Python script?
Checking the same condition again in a nested if usually make no sense, except
if the value of the expression could have been changed between the two checks.
Although compilers may optimize this out, such code is suspicious: the
programmer may have meant to check something else. Therefore it is worth to
find such places in the code and notify the user about the problem.
This patch implements a basic check for this problem. Currently it only detects
redundant conditions where the condition is a variable of integral type. It
also detects the possible bug if the variable is in an "or" or "and" logical
expression in the inner if and/or the variable is in an "and" logical
expression in the outer if statement. Negated cases are not handled yet.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D81272
Files:
clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
clang-tools-extra/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
clang-tools-extra/clang-tidy/misc/RedundantConditionCheck.cpp
clang-tools-extra/clang-tidy/misc/RedundantConditionCheck.h
clang-tools-extra/clang-tidy/utils/Aliasing.cpp
clang-tools-extra/clang-tidy/utils/Aliasing.h
clang-tools-extra/clang-tidy/utils/CMakeLists.txt
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/misc-redundant-condition.rst
clang-tools-extra/test/clang-tidy/checkers/misc-redundant-condition.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/misc-redundant-condition.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-redundant-condition.cpp
@@ -0,0 +1,1058 @@
+// RUN: %check_clang_tidy %s misc-redundant-condition %t
+
+extern unsigned peopleInTheBuilding;
+extern unsigned fireFighters;
+
+bool isBurning();
+bool isReallyBurning();
+bool isCollapsing();
+void tryToExtinguish(bool&);
+void scream();
+
+bool someOtherCondition();
+
+//===--- Basic Positives --------------------------------------------------===//
+
+void positive_direct() {
+ bool onFire = isBurning();
+ if (onFire) {
+ if (onFire) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: {{^\ *$}}
+ scream();
+ }
+ // CHECK-FIXES: {{^\ *$}}
+ }
+}
+
+void positive_indirect() {
+ bool onFire = isBurning();
+ if (onFire) {
+ if (someOtherCondition()) {
+ if (onFire)
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: {{^\ *$}}
+ scream();
+ }
+ }
+}
+
+void positive_direct_inner_and_lhs() {
+ bool onFire = isBurning();
+ if (onFire) {
+ if (onFire && peopleInTheBuilding > 0) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: if (peopleInTheBuilding > 0) {
+ scream();
+ }
+ }
+}
+
+void positive_indirect_inner_and_lhs() {
+ bool onFire = isBurning();
+ if (onFire) {
+ if (someOtherCondition()) {
+ if (onFire && peopleInTheBuilding > 0) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: if (peopleInTheBuilding > 0) {
+ scream();
+ }
+ }
+ }
+}
+
+void positive_direct_inner_and_rhs() {
+ bool onFire = isBurning();
+ if (onFire) {
+ if (peopleInTheBuilding > 0 && onFire) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: if (peopleInTheBuilding > 0) {
+ scream();
+ }
+ }
+}
+
+void positive_indirect_inner_and_rhs() {
+ bool onFire = isBurning();
+ if (onFire) {
+ if (someOtherCondition()) {
+ if (peopleInTheBuilding > 0 && onFire) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: if (peopleInTheBuilding > 0) {
+ scream();
+ }
+ }
+ }
+}
+
+void positive_direct_inner_or_lhs() {
+ bool onFire = isBurning();
+ if (onFire) {
+ if (onFire || isCollapsing()) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: {{^\ *$}}
+ scream();
+ }
+ // CHECK-FIXES: {{^\ *$}}
+ }
+}
+
+void positive_indirect_inner_or_lhs() {
+ bool onFire = isBurning();
+ if (onFire) {
+ if (someOtherCondition()) {
+ if (onFire || isCollapsing()) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: {{^\ *$}}
+ scream();
+ }
+ // CHECK-FIXES: {{^\ *$}}
+ }
+ }
+}
+
+void positive_direct_inner_or_rhs() {
+ bool onFire = isBurning();
+ if (onFire) {
+ if (isCollapsing() || onFire) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: {{^\ *$}}
+ scream();
+ }
+ // CHECK-FIXES: {{^\ *$}}
+ }
+}
+
+void positive_indirect_inner_or_rhs() {
+ bool onFire = isBurning();
+ if (onFire) {
+ if (someOtherCondition()) {
+ if (isCollapsing() || onFire) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: {{^\ *$}}
+ scream();
+ }
+ // CHECK-FIXES: {{^\ *$}}
+ }
+ }
+}
+
+void positive_direct_outer_and_lhs() {
+ bool onFire = isBurning();
+ if (onFire && fireFighters < 10) {
+ if (onFire) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: {{^\ *$}}
+ scream();
+ }
+ // CHECK-FIXES: {{^\ *$}}
+ }
+}
+
+void positive_indirect_outer_and_lhs() {
+ bool onFire = isBurning();
+ if (onFire && fireFighters < 10) {
+ if (someOtherCondition()) {
+ if (onFire) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: {{^\ *$}}
+ scream();
+ }
+ // CHECK-FIXES: {{^\ *$}}
+ }
+ }
+}
+
+void positive_direct_outer_and_lhs_inner_and_lhs() {
+ bool onFire = isBurning();
+ if (onFire && fireFighters < 10) {
+ if (onFire && peopleInTheBuilding > 0) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: if (peopleInTheBuilding > 0) {
+ scream();
+ }
+ }
+}
+
+void positive_indirect_outer_and_lhs_inner_and_lhs() {
+ bool onFire = isBurning();
+ if (onFire && fireFighters < 10) {
+ if (someOtherCondition()) {
+ if (onFire && peopleInTheBuilding > 0) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: if (peopleInTheBuilding > 0) {
+ scream();
+ }
+ }
+ }
+}
+
+void positive_direct_outer_and_lhs_inner_and_rhs() {
+ bool onFire = isBurning();
+ if (onFire && fireFighters < 10) {
+ if (peopleInTheBuilding > 0 && onFire) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: if (peopleInTheBuilding > 0) {
+ scream();
+ }
+ }
+}
+
+void positive_indirect_outer_and_lhs_inner_and_rhs() {
+ bool onFire = isBurning();
+ if (onFire && fireFighters < 10) {
+ if (someOtherCondition()) {
+ if (peopleInTheBuilding > 0 && onFire) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: if (peopleInTheBuilding > 0) {
+ scream();
+ }
+ }
+ }
+}
+
+void positive_direct_outer_and_lhs_inner_or_lhs() {
+ bool onFire = isBurning();
+ if (onFire && fireFighters < 10) {
+ if (onFire || isCollapsing()) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: {{^\ *$}}
+ scream();
+ }
+ // CHECK-FIXES: {{^\ *$}}
+ }
+}
+
+void positive_indirect_outer_and_lhs_inner_or_lhs() {
+ bool onFire = isBurning();
+ if (onFire && fireFighters < 10) {
+ if (someOtherCondition()) {
+ if (onFire || isCollapsing()) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: {{^\ *$}}
+ scream();
+ }
+ // CHECK-FIXES: {{^\ *$}}
+ }
+ }
+}
+
+void positive_direct_outer_and_lhs_inner_or_rhs() {
+ bool onFire = isBurning();
+ if (onFire && fireFighters < 10) {
+ if (isCollapsing() || onFire) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: {{^\ *$}}
+ scream();
+ }
+ // CHECK-FIXES: {{^\ *$}}
+ }
+}
+
+void positive_indirect_outer_and_lhs_inner_or_rhs() {
+ bool onFire = isBurning();
+ if (onFire && fireFighters < 10) {
+ if (someOtherCondition()) {
+ if (isCollapsing() || onFire) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: {{^\ *$}}
+ scream();
+ }
+ // CHECK-FIXES: {{^\ *$}}
+ }
+ }
+}
+
+void positive_direct_outer_and_rhs() {
+ bool onFire = isBurning();
+ if (fireFighters < 10 && onFire) {
+ if (onFire) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: {{^\ *$}}
+ scream();
+ }
+ // CHECK-FIXES: {{^\ *$}}
+ }
+}
+
+void positive_indirect_outer_and_rhs() {
+ bool onFire = isBurning();
+ if (fireFighters < 10 && onFire) {
+ if (someOtherCondition()) {
+ if (onFire) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: {{^\ *$}}
+ scream();
+ }
+ // CHECK-FIXES: {{^\ *$}}
+ }
+ }
+}
+
+void positive_direct_outer_and_rhs_inner_and_lhs() {
+ bool onFire = isBurning();
+ if (fireFighters < 10 && onFire) {
+ if (onFire && peopleInTheBuilding > 0) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: if (peopleInTheBuilding > 0) {
+ scream();
+ }
+ }
+}
+
+void positive_indirect_outer_and_rhs_inner_and_lhs() {
+ bool onFire = isBurning();
+ if (fireFighters < 10 && onFire) {
+ if (someOtherCondition()) {
+ if (onFire && peopleInTheBuilding > 0) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: if (peopleInTheBuilding > 0) {
+ scream();
+ }
+ }
+ }
+}
+
+void positive_direct_inner_outer_and_rhs_and_rhs() {
+ bool onFire = isBurning();
+ if (fireFighters < 10 && onFire) {
+ if (peopleInTheBuilding > 0 && onFire) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: if (peopleInTheBuilding > 0) {
+ scream();
+ }
+ }
+}
+
+void positive_indirect_outer_and_rhs_inner_and_rhs() {
+ bool onFire = isBurning();
+ if (fireFighters < 10 && onFire) {
+ if (someOtherCondition()) {
+ if (peopleInTheBuilding > 0 && onFire) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: if (peopleInTheBuilding > 0) {
+ scream();
+ }
+ }
+ }
+}
+
+void positive_direct_outer_and_rhs_inner_or_lhs() {
+ bool onFire = isBurning();
+ if (fireFighters < 10 && onFire) {
+ if (onFire || isCollapsing()) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: {{^\ *$}}
+ scream();
+ }
+ // CHECK-FIXES: {{^\ *$}}
+ }
+}
+
+void positive_indirect_outer_and_rhs_inner_or_lhs() {
+ bool onFire = isBurning();
+ if (fireFighters < 10 && onFire) {
+ if (someOtherCondition()) {
+ if (onFire || isCollapsing()) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: {{^\ *$}}
+ scream();
+ }
+ // CHECK-FIXES: {{^\ *$}}
+ }
+ }
+}
+
+void positive_direct_outer_and_rhs_inner_or_rhs() {
+ bool onFire = isBurning();
+ if (fireFighters < 10 && onFire) {
+ if (isCollapsing() || onFire) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: {{^\ *$}}
+ scream();
+ }
+ // CHECK-FIXES: {{^\ *$}}
+ }
+}
+
+void positive_indirect_outer_and_rhs_inner_or_rhs() {
+ bool onFire = isBurning();
+ if (fireFighters < 10 && onFire) {
+ if (someOtherCondition()) {
+ if (isCollapsing() || onFire) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: {{^\ *$}}
+ scream();
+ }
+ // CHECK-FIXES: {{^\ *$}}
+ }
+ }
+}
+
+//===--- Basic Negatives --------------------------------------------------===//
+
+void negative_direct() {
+ bool onFire = isBurning();
+ if (onFire) {
+ tryToExtinguish(onFire);
+ if (onFire) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+}
+
+void negative_indirect() {
+ bool onFire = isBurning();
+ if (onFire) {
+ tryToExtinguish(onFire);
+ if (someOtherCondition()) {
+ if (onFire) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+ }
+}
+
+void negative_indirect2() {
+ bool onFire = isBurning();
+ if (onFire) {
+ if (someOtherCondition()) {
+ tryToExtinguish(onFire);
+ if (onFire) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+ }
+}
+
+void negative_direct_inner_and_lhs() {
+ bool onFire = isBurning();
+ if (onFire) {
+ tryToExtinguish(onFire);
+ if (onFire && peopleInTheBuilding > 0) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+}
+
+void negative_indirect_inner_and_lhs() {
+ bool onFire = isBurning();
+ if (onFire) {
+ tryToExtinguish(onFire);
+ if (someOtherCondition()) {
+ if (onFire && peopleInTheBuilding > 0) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+ }
+}
+
+void negative_indirect2_inner_and_lhs() {
+ bool onFire = isBurning();
+ if (onFire) {
+ if (someOtherCondition()) {
+ tryToExtinguish(onFire);
+ if (onFire && peopleInTheBuilding > 0) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+ }
+}
+
+void negative_direct_inner_and_rhs() {
+ bool onFire = isBurning();
+ if (onFire) {
+ tryToExtinguish(onFire);
+ if (peopleInTheBuilding > 0 && onFire) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+}
+
+void negative_indirect_inner_and_rhs() {
+ bool onFire = isBurning();
+ if (onFire) {
+ tryToExtinguish(onFire);
+ if (someOtherCondition()) {
+ if (peopleInTheBuilding > 0 && onFire) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+ }
+}
+
+void negative_indirect2_inner_and_rhs() {
+ bool onFire = isBurning();
+ if (onFire) {
+ if (someOtherCondition()) {
+ tryToExtinguish(onFire);
+ if (peopleInTheBuilding > 0 && onFire) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+ }
+}
+
+void negative_direct_inner_or_lhs() {
+ bool onFire = isBurning();
+ if (onFire) {
+ tryToExtinguish(onFire);
+ if (onFire || isCollapsing()) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+}
+
+void negative_indirect_inner_or_lhs() {
+ bool onFire = isBurning();
+ if (onFire) {
+ tryToExtinguish(onFire);
+ if (someOtherCondition()) {
+ if (onFire || isCollapsing()) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+ }
+}
+
+void negative_indirect2_inner_or_lhs() {
+ bool onFire = isBurning();
+ if (onFire) {
+ if (someOtherCondition()) {
+ tryToExtinguish(onFire);
+ if (onFire || isCollapsing()) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+ }
+}
+
+void negative_direct_inner_or_rhs() {
+ bool onFire = isBurning();
+ if (onFire) {
+ tryToExtinguish(onFire);
+ if (isCollapsing() || onFire) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+}
+
+void negative_indirect_inner_or_rhs() {
+ bool onFire = isBurning();
+ if (onFire) {
+ tryToExtinguish(onFire);
+ if (someOtherCondition()) {
+ if (isCollapsing() || onFire) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+ }
+}
+
+void negative_indirect2_inner_or_rhs() {
+ bool onFire = isBurning();
+ if (onFire) {
+ if (someOtherCondition()) {
+ tryToExtinguish(onFire);
+ if (isCollapsing() || onFire) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+ }
+}
+
+void negative_direct_outer_and_lhs() {
+ bool onFire = isBurning();
+ if (onFire && fireFighters < 10) {
+ tryToExtinguish(onFire);
+ if (onFire) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+}
+
+void negative_indirect_outer_and_lhs() {
+ bool onFire = isBurning();
+ if (onFire && fireFighters < 10) {
+ tryToExtinguish(onFire);
+ if (someOtherCondition()) {
+ if (onFire) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+ }
+}
+
+void negative_indirect2_outer_and_lhs() {
+ bool onFire = isBurning();
+ if (onFire && fireFighters < 10) {
+ if (someOtherCondition()) {
+ tryToExtinguish(onFire);
+ if (onFire) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+ }
+}
+
+void negative_direct_outer_and_lhs_inner_and_lhs() {
+ bool onFire = isBurning();
+ if (onFire && fireFighters < 10) {
+ tryToExtinguish(onFire);
+ if (onFire && peopleInTheBuilding > 0) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+}
+
+void negative_indirect_outer_and_lhs_inner_and_lhs() {
+ bool onFire = isBurning();
+ if (onFire && fireFighters < 10) {
+ tryToExtinguish(onFire);
+ if (someOtherCondition()) {
+ if (onFire && peopleInTheBuilding > 0) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+ }
+}
+
+void negative_indirect2_outer_and_lhs_inner_and_lhs() {
+ bool onFire = isBurning();
+ if (onFire && fireFighters < 10) {
+ if (someOtherCondition()) {
+ tryToExtinguish(onFire);
+ if (onFire && peopleInTheBuilding > 0) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+ }
+}
+
+void negative_direct_outer_and_lhs_inner_and_rhs() {
+ bool onFire = isBurning();
+ if (onFire && fireFighters < 10) {
+ tryToExtinguish(onFire);
+ if (peopleInTheBuilding > 0 && onFire) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+}
+
+void negative_indirect_outer_and_lhs_inner_and_rhs() {
+ bool onFire = isBurning();
+ if (onFire && fireFighters < 10) {
+ tryToExtinguish(onFire);
+ if (someOtherCondition()) {
+ if (peopleInTheBuilding > 0 && onFire) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+ }
+}
+
+void negative_indirect2_outer_and_lhs_inner_and_rhs() {
+ bool onFire = isBurning();
+ if (onFire && fireFighters < 10) {
+ if (someOtherCondition()) {
+ tryToExtinguish(onFire);
+ if (peopleInTheBuilding > 0 && onFire) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+ }
+}
+
+void negative_direct_outer_and_lhs_inner_or_lhs() {
+ bool onFire = isBurning();
+ if (onFire && fireFighters < 10) {
+ tryToExtinguish(onFire);
+ if (onFire || isCollapsing()) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+}
+
+void negative_indirect_outer_and_lhs_inner_or_lhs() {
+ bool onFire = isBurning();
+ if (onFire && fireFighters < 10) {
+ tryToExtinguish(onFire);
+ if (someOtherCondition()) {
+ if (onFire || isCollapsing()) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+ }
+}
+
+void negative_indirect2_outer_and_lhs_inner_or_lhs() {
+ bool onFire = isBurning();
+ if (onFire && fireFighters < 10) {
+ if (someOtherCondition()) {
+ tryToExtinguish(onFire);
+ if (onFire || isCollapsing()) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+ }
+}
+
+void negative_direct_outer_and_lhs_inner_or_rhs() {
+ bool onFire = isBurning();
+ if (onFire && fireFighters < 10) {
+ tryToExtinguish(onFire);
+ if (isCollapsing() || onFire) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+}
+
+void negative_indirect_outer_and_lhs_inner_or_rhs() {
+ bool onFire = isBurning();
+ if (onFire && fireFighters < 10) {
+ tryToExtinguish(onFire);
+ if (someOtherCondition()) {
+ if (isCollapsing() || onFire) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+ }
+}
+
+void negative_indirect2_outer_and_lhs_inner_or_rhs() {
+ bool onFire = isBurning();
+ if (onFire && fireFighters < 10) {
+ if (someOtherCondition()) {
+ tryToExtinguish(onFire);
+ if (isCollapsing() || onFire) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+ }
+}
+
+void negative_direct_outer_and_rhs() {
+ bool onFire = isBurning();
+ if (fireFighters < 10 && onFire) {
+ tryToExtinguish(onFire);
+ if (onFire) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+}
+
+void negative_indirect_outer_and_rhs() {
+ bool onFire = isBurning();
+ if (fireFighters < 10 && onFire) {
+ tryToExtinguish(onFire);
+ if (someOtherCondition()) {
+ if (onFire) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+ }
+}
+
+void negative_indirect2_outer_and_rhs() {
+ bool onFire = isBurning();
+ if (fireFighters < 10 && onFire) {
+ if (someOtherCondition()) {
+ tryToExtinguish(onFire);
+ if (onFire) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+ }
+}
+
+void negative_direct_outer_and_rhs_inner_and_lhs() {
+ bool onFire = isBurning();
+ if (fireFighters < 10 && onFire) {
+ tryToExtinguish(onFire);
+ if (onFire && peopleInTheBuilding > 0) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+}
+
+void negative_indirect_outer_and_rhs_inner_and_lhs() {
+ bool onFire = isBurning();
+ if (fireFighters < 10 && onFire) {
+ tryToExtinguish(onFire);
+ if (someOtherCondition()) {
+ if (onFire && peopleInTheBuilding > 0) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+ }
+}
+
+void negative_indirect2_outer_and_rhs_inner_and_lhs() {
+ bool onFire = isBurning();
+ if (fireFighters < 10 && onFire) {
+ if (someOtherCondition()) {
+ tryToExtinguish(onFire);
+ if (onFire && peopleInTheBuilding > 0) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+ }
+}
+
+void negative_direct_inner_outer_and_rhs_and_rhs() {
+ bool onFire = isBurning();
+ if (fireFighters < 10 && onFire) {
+ tryToExtinguish(onFire);
+ if (peopleInTheBuilding > 0 && onFire) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+}
+
+void negative_indirect_outer_and_rhs_inner_and_rhs() {
+ bool onFire = isBurning();
+ if (fireFighters < 10 && onFire) {
+ tryToExtinguish(onFire);
+ if (someOtherCondition()) {
+ if (peopleInTheBuilding > 0 && onFire) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+ }
+}
+
+void negative_indirect2_outer_and_rhs_inner_and_rhs() {
+ bool onFire = isBurning();
+ if (fireFighters < 10 && onFire) {
+ if (someOtherCondition()) {
+ tryToExtinguish(onFire);
+ if (peopleInTheBuilding > 0 && onFire) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+ }
+}
+
+void negative_direct_outer_and_rhs_inner_or_lhs() {
+ bool onFire = isBurning();
+ if (fireFighters < 10 && onFire) {
+ tryToExtinguish(onFire);
+ if (onFire || isCollapsing()) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+}
+
+void negative_indirect_outer_and_rhs_inner_or_lhs() {
+ bool onFire = isBurning();
+ if (fireFighters < 10 && onFire) {
+ tryToExtinguish(onFire);
+ if (someOtherCondition()) {
+ if (onFire || isCollapsing()) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+ }
+}
+
+void negative_indirect2_outer_and_rhs_inner_or_lhs() {
+ bool onFire = isBurning();
+ if (fireFighters < 10 && onFire) {
+ if (someOtherCondition()) {
+ tryToExtinguish(onFire);
+ if (onFire || isCollapsing()) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+ }
+}
+
+void negative_direct_outer_and_rhs_inner_or_rhs() {
+ bool onFire = isBurning();
+ if (fireFighters < 10 && onFire) {
+ tryToExtinguish(onFire);
+ if (isCollapsing() || onFire) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+}
+
+void negative_indirect_outer_and_rhs_inner_or_rhs() {
+ bool onFire = isBurning();
+ if (fireFighters < 10 && onFire) {
+ tryToExtinguish(onFire);
+ if (someOtherCondition()) {
+ if (isCollapsing() || onFire) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+ }
+}
+
+void negative_indirect2_outer_and_rhs_inner_or_rhs() {
+ bool onFire = isBurning();
+ if (fireFighters < 10 && onFire) {
+ if (someOtherCondition()) {
+ tryToExtinguish(onFire);
+ if (isCollapsing() || onFire) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+ }
+}
+
+void negative_reassigned() {
+ bool onFire = isBurning();
+ if (onFire) {
+ onFire = isReallyBurning();
+ if (onFire) {
+ // NO-MESSAGE: it was a false alarm then
+ scream();
+ }
+ }
+}
+
+//===--- Special Positives ------------------------------------------------===//
+
+// Condition variable mutated in or after the inner loop
+
+void positive_direct_mutated_after_inner() {
+ bool onFire = isBurning();
+ if (onFire) {
+ if (onFire) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: {{^\ *$}}
+ scream();
+ }
+ // CHECK-FIXES: {{^\ *$}}
+ tryToExtinguish(onFire);
+ }
+}
+
+void positive_indirect_mutated_after_inner() {
+ bool onFire = isBurning();
+ if (onFire) {
+ if (someOtherCondition()) {
+ if (onFire) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: {{^\ *$}}
+ scream();
+ }
+ // CHECK-FIXES: {{^\ *$}}
+ }
+ tryToExtinguish(onFire);
+ }
+}
+
+void positive_indirect2_mutated_after_inner() {
+ bool onFire = isBurning();
+ if (onFire) {
+ if (someOtherCondition()) {
+ if (onFire) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: {{^\ *$}}
+ scream();
+ }
+ // CHECK-FIXES: {{^\ *$}}
+ tryToExtinguish(onFire);
+ }
+ }
+}
+
+void positive_mutated_in_inner() {
+ bool onFire = isBurning();
+ if (onFire) {
+ if (onFire) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+ // CHECK-FIXES: {{^\ *$}}
+ tryToExtinguish(onFire);
+ scream();
+ }
+ // CHECK-FIXES: {{^\ *$}}
+ }
+}
+
+//===--- Special Negatives ------------------------------------------------===//
+
+// Aliasing
+
+void negative_mutated_by_ptr() {
+ bool onFire = isBurning();
+ bool *firePtr = &onFire;
+ if (onFire) {
+ tryToExtinguish(*firePtr);
+ if (onFire) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+}
+
+void negative_mutated_by_ref() {
+ bool onFire = isBurning();
+ bool &fireRef = onFire;
+ if (onFire) {
+ tryToExtinguish(fireRef);
+ if (onFire) {
+ // NO-MESSAGE: fire may have been extinguished
+ scream();
+ }
+ }
+}
+
+// Volatile
+
+void negatvie_volatile() {
+ volatile bool onFire = isBurning();
+ if (onFire) {
+ if (onFire) {
+ // NO-MESSAGE: maybe some other thread extinguished the fire
+ scream();
+ }
+ }
+}
Index: clang-tools-extra/docs/clang-tidy/checks/misc-redundant-condition.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-redundant-condition.rst
@@ -0,0 +1,80 @@
+.. title:: clang-tidy - misc-redundant-condition
+
+misc-redundant-condition
+========================
+
+Finds condition variables in nested `if` statements that were also checked in
+the outer `if` statement and were not changed.
+
+Simple example:
+
+.. code-block:: c
+
+ bool onFire = isBurning();
+ if (onFire) {
+ if (onFire)
+ scream();
+ }
+
+Here `onFire` is checked both in the outer ``if`` and the inner ``if`` statement
+without a possible change between the two checks. The check warns for this code
+and suggests removal of the second checking of variable `onFire`.
+
+The checker also detects redundant condition checks if the condition variable
+is an operand of a logical "and" (``&&``) or a logical "or" (``||``) operator:
+
+.. code-block:: c
+
+ bool onFire = isBurning();
+ if (onFire) {
+ if (onFire && peopleInTheBuilding > 0)
+ scream();
+ }
+
+.. code-block:: c
+
+ bool onFire = isBurning();
+ if (onFire) {
+ if (onFire || isCollapsing())
+ scream();
+ }
+
+In the first case (logical "and") the suggested fix is to remove the redundant
+condition variable and keep the other side of the ``&&``. In the second case
+(logical "or") the whole ``if`` is removed similarily to the simple case on the
+top.
+
+The condition of the outer ``if`` statement may also be a logical "and" (``&&``)
+expression:
+
+.. code-block:: c
+
+ bool onFire = isBurning();
+ if (onFire && fireFighters < 10) {
+ if (someOtherCondition()) {
+ if (onFire)
+ scream();
+ }
+ }
+
+The error is also detected if both the outer statement is a logical "and"
+(``&&``) and the inner statement is a logical "and" (``&&``) or "or" (``||``).
+The inner ``if`` statement does not have to be a direct descendant of the outer
+one.
+
+No error is detected if the condition variable may have been changed between the
+two checks:
+
+.. code-block:: c
+
+ bool onFire = isBurning();
+ if (onFire) {
+ tryToExtinguish(onFire);
+ if (onFire && peopleInTheBuilding > 0)
+ scream();
+ }
+
+Every possible change is considered, thus if the condition variable is not
+a local variable of the function, it is a volatile or it has an alias (pointer
+or reference) then no warning is issued.
+
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -132,6 +132,7 @@
`clang-analyzer-valist.Uninitialized <clang-analyzer-valist.Uninitialized.html>`_,
`clang-analyzer-valist.Unterminated <clang-analyzer-valist.Unterminated.html>`_,
`cppcoreguidelines-avoid-goto <cppcoreguidelines-avoid-goto.html>`_,
+ `cppcoreguidelines-avoid-non-const-global-variables <cppcoreguidelines-avoid-non-const-global-variables.html>`_,
`cppcoreguidelines-init-variables <cppcoreguidelines-init-variables.html>`_, "Yes"
`cppcoreguidelines-interfaces-global-init <cppcoreguidelines-interfaces-global-init.html>`_,
`cppcoreguidelines-macro-usage <cppcoreguidelines-macro-usage.html>`_,
@@ -188,15 +189,16 @@
`llvm-prefer-isa-or-dyn-cast-in-conditionals <llvm-prefer-isa-or-dyn-cast-in-conditionals.html>`_, "Yes"
`llvm-prefer-register-over-unsigned <llvm-prefer-register-over-unsigned.html>`_, "Yes"
`llvm-twine-local <llvm-twine-local.html>`_, "Yes"
- `llvmlibc-callee-namespace <llvmlibc-calle-namespace.html>`_,
+ `llvmlibc-callee-namespace <llvmlibc-callee-namespace.html>`_,
`llvmlibc-implementation-in-namespace <llvmlibc-implementation-in-namespace.html>`_,
- `llvmlibc-restrict-system-libc-headers <llvmlibc-restrict-system-libc-headers.html>`_, "Yes"
+ `llvmlibc-restrict-system-libc-headers <llvmlibc-restrict-system-libc-headers.html>`_,
`misc-definitions-in-headers <misc-definitions-in-headers.html>`_, "Yes"
`misc-misplaced-const <misc-misplaced-const.html>`_,
`misc-new-delete-overloads <misc-new-delete-overloads.html>`_,
`misc-no-recursion <misc-no-recursion.html>`_,
`misc-non-copyable-objects <misc-non-copyable-objects.html>`_,
`misc-non-private-member-variables-in-classes <misc-non-private-member-variables-in-classes.html>`_,
+ `misc-redundant-condition <misc-redundant-condition.html>`_, "Yes"
`misc-redundant-expression <misc-redundant-expression.html>`_, "Yes"
`misc-static-assert <misc-static-assert.html>`_, "Yes"
`misc-throw-by-value-catch-by-reference <misc-throw-by-value-catch-by-reference.html>`_,
@@ -211,7 +213,7 @@
`modernize-deprecated-headers <modernize-deprecated-headers.html>`_, "Yes"
`modernize-deprecated-ios-base-aliases <modernize-deprecated-ios-base-aliases.html>`_, "Yes"
`modernize-loop-convert <modernize-loop-convert.html>`_, "Yes"
- `modernize-make-shared <modernize-make-shared.html>`_, "Yes"
+ `modernize-make-shared <modernize-make-shared.html>`_,
`modernize-make-unique <modernize-make-unique.html>`_, "Yes"
`modernize-pass-by-value <modernize-pass-by-value.html>`_, "Yes"
`modernize-raw-string-literal <modernize-raw-string-literal.html>`_, "Yes"
@@ -241,7 +243,7 @@
`objc-dealloc-in-category <objc-dealloc-in-category.html>`_,
`objc-forbidden-subclassing <objc-forbidden-subclassing.html>`_,
`objc-missing-hash <objc-missing-hash.html>`_,
- `objc-nsinvocation-argument-lifetime <objc-nsinvocation-argument-lifetime.html>`_, "Yes"
+ `objc-nsinvocation-argument-lifetime <objc-nsinvocation-argument-lifetime.html>`_,
`objc-property-declaration <objc-property-declaration.html>`_, "Yes"
`objc-super-self <objc-super-self.html>`_, "Yes"
`openmp-exception-escape <openmp-exception-escape.html>`_,
@@ -387,7 +389,6 @@
`clang-analyzer-unix.cstring.NullArg <clang-analyzer-unix.cstring.NullArg.html>`_, `Clang Static Analyzer <https://clang.llvm.org/docs/analyzer/checkers.html>`_,
`cppcoreguidelines-avoid-c-arrays <cppcoreguidelines-avoid-c-arrays.html>`_, `modernize-avoid-c-arrays <modernize-avoid-c-arrays.html>`_,
`cppcoreguidelines-avoid-magic-numbers <cppcoreguidelines-avoid-magic-numbers.html>`_, `readability-magic-numbers <readability-magic-numbers.html>`_,
- `cppcoreguidelines-avoid-non-const-global-variables <cppcoreguidelines-avoid-non-const-global-variables.html>`_, , , ""
`cppcoreguidelines-c-copy-assignment-signature <cppcoreguidelines-c-copy-assignment-signature.html>`_, `misc-unconventional-assign-operator <misc-unconventional-assign-operator.html>`_,
`cppcoreguidelines-explicit-virtual-functions <cppcoreguidelines-explicit-virtual-functions.html>`_, `modernize-use-override <modernize-use-override.html>`_, "Yes"
`cppcoreguidelines-non-private-member-variables-in-classes <cppcoreguidelines-non-private-member-variables-in-classes.html>`_, `misc-non-private-member-variables-in-classes <misc-non-private-member-variables-in-classes.html>`_,
@@ -407,7 +408,7 @@
`hicpp-new-delete-operators <hicpp-new-delete-operators.html>`_, `misc-new-delete-overloads <misc-new-delete-overloads.html>`_,
`hicpp-no-array-decay <hicpp-no-array-decay.html>`_, `cppcoreguidelines-pro-bounds-array-to-pointer-decay <cppcoreguidelines-pro-bounds-array-to-pointer-decay.html>`_,
`hicpp-no-malloc <hicpp-no-malloc.html>`_, `cppcoreguidelines-no-malloc <cppcoreguidelines-no-malloc.html>`_,
- `hicpp-noexcept-move <hicpp-noexcept-move.html>`_, `performance-noexcept-move-constructor <performance-noexcept-move-constructor.html>`_,
+ `hicpp-noexcept-move <hicpp-noexcept-move.html>`_, `performance-noexcept-move-constructor <performance-noexcept-move-constructor.html>`_, "Yes"
`hicpp-special-member-functions <hicpp-special-member-functions.html>`_, `cppcoreguidelines-special-member-functions <cppcoreguidelines-special-member-functions.html>`_,
`hicpp-static-assert <hicpp-static-assert.html>`_, `misc-static-assert <misc-static-assert.html>`_, "Yes"
`hicpp-undelegated-constructor <hicpp-undelegated-constructor.html>`_, `bugprone-undelegated-constructor <bugprone-undelegated-constructor.html>`_,
@@ -421,4 +422,3 @@
`hicpp-use-override <hicpp-use-override.html>`_, `modernize-use-override <modernize-use-override.html>`_, "Yes"
`hicpp-vararg <hicpp-vararg.html>`_, `cppcoreguidelines-pro-type-vararg <cppcoreguidelines-pro-type-vararg.html>`_,
`llvm-qualified-auto <llvm-qualified-auto.html>`_, `readability-qualified-auto <readability-qualified-auto.html>`_, "Yes"
-
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -75,6 +75,12 @@
New checks
^^^^^^^^^^
+- New :doc:`misc-redundant-condition
+ <clang-tidy/checks/misc-redundant-condition>` check.
+
+ Finds condition variables in nested `if` statements that were also checked
+ in the outer `if` statement and were not changed.
+
- New :doc:`cppcoreguidelines-avoid-non-const-global-variables
<clang-tidy/checks/cppcoreguidelines-avoid-non-const-global-variables>` check.
Finds non-const global variables as described in check I.2 of C++ Core
Index: clang-tools-extra/clang-tidy/utils/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/utils/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/utils/CMakeLists.txt
@@ -4,6 +4,7 @@
)
add_clang_library(clangTidyUtils
+ Aliasing.cpp
ASTUtils.cpp
DeclRefExprUtils.cpp
ExceptionAnalyzer.cpp
Index: clang-tools-extra/clang-tidy/utils/Aliasing.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/utils/Aliasing.h
@@ -0,0 +1,25 @@
+//===------------- Aliasing.h - clang-tidy --------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_ALIASING_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_ALIASING_H
+
+#include "clang/AST/Decl.h"
+
+namespace clang {
+namespace tidy {
+namespace utils {
+
+/// Returns whether a ``Var`` has a pointer or reference in ``Func``
+bool hasPtrOrReferenceInFunc(const FunctionDecl *Func, const VarDecl *Var);
+
+} // namespace utils
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_ALIASING_H
Index: clang-tools-extra/clang-tidy/utils/Aliasing.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/utils/Aliasing.cpp
@@ -0,0 +1,66 @@
+//===------------- Aliasing.cpp - clang-tidy ------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "Aliasing.h"
+
+#include "clang/AST/Expr.h"
+
+namespace clang {
+namespace tidy {
+namespace utils {
+
+/// Return whether `S` is a reference to the declaration of `Var`.
+static bool isAccessForVar(const Stmt *S, const VarDecl *Var) {
+ if (const auto *DRE = dyn_cast<DeclRefExpr>(S))
+ return DRE->getDecl() == Var;
+
+ return false;
+}
+
+/// Return whether `Var` has a pointer or reference in `S`.
+static bool isPtrOrReferenceForVar(const Stmt *S, const VarDecl *Var) {
+ if (const auto *DS = dyn_cast<DeclStmt>(S)) {
+ for (const Decl *D : DS->getDeclGroup()) {
+ if (const auto *LeftVar = dyn_cast<VarDecl>(D)) {
+ if (LeftVar->hasInit() && LeftVar->getType()->isReferenceType()) {
+ return isAccessForVar(LeftVar->getInit(), Var);
+ }
+ }
+ }
+ } else if (const auto *UnOp = dyn_cast<UnaryOperator>(S)) {
+ if (UnOp->getOpcode() == UO_AddrOf)
+ return isAccessForVar(UnOp->getSubExpr(), Var);
+ }
+
+ return false;
+}
+
+/// Return whether `Var` has a pointer or reference in `S`.
+static bool hasPtrOrReferenceInStmt(const Stmt *S, const VarDecl *Var) {
+ if (isPtrOrReferenceForVar(S, Var))
+ return true;
+
+ for (const Stmt *Child : S->children()) {
+ if (!Child)
+ continue;
+
+ if (hasPtrOrReferenceInStmt(Child, Var))
+ return true;
+ }
+
+ return false;
+}
+
+/// Return whether `Var` has a pointer or reference in `Func`.
+bool hasPtrOrReferenceInFunc(const FunctionDecl *Func, const VarDecl *Var) {
+ return hasPtrOrReferenceInStmt(Func->getBody(), Var);
+}
+
+} // namespace utils
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/misc/RedundantConditionCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/RedundantConditionCheck.h
@@ -0,0 +1,35 @@
+//===--- RedundantConditionCheck.h - clang-tidy -----------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_REDUNDANTCONDITIONCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_REDUNDANTCONDITIONCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Finds condition variables in nested `if` statements that were also checked
+/// in the outer `if` statement and were not changed.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-redundant-condition.html
+class RedundantConditionCheck : public ClangTidyCheck {
+public:
+ RedundantConditionCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_REDUNDANTCONDITIONCHECK_H
Index: clang-tools-extra/clang-tidy/misc/RedundantConditionCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/RedundantConditionCheck.cpp
@@ -0,0 +1,127 @@
+//===--- RedundantConditionCheck.cpp - clang-tidy -------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "RedundantConditionCheck.h"
+#include "../utils/Aliasing.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
+
+using namespace clang::ast_matchers;
+using clang::tidy::utils::hasPtrOrReferenceInFunc;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Returns whether `Var` is changed in `S` before `NextS`.
+static bool isChangedBefore(const Stmt *S, const Stmt *NextS,
+ const VarDecl *Var, ASTContext *Context) {
+ ExprMutationAnalyzer MutAn(*S, *Context);
+ const auto &SM = Context->getSourceManager();
+ if (const Stmt *MutS = MutAn.findMutation(Var)) {
+ if (SM.isBeforeInTranslationUnit(MutS->getEndLoc(), NextS->getBeginLoc()))
+ return true;
+ }
+
+ return false;
+}
+
+void RedundantConditionCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ ifStmt(hasCondition(anyOf(
+ declRefExpr(hasDeclaration(varDecl().bind("cond_var"))),
+ binaryOperator(hasOperatorName("&&"),
+ hasEitherOperand(declRefExpr(hasDeclaration(
+ varDecl().bind("cond_var"))))))),
+ hasDescendant(
+ ifStmt(hasCondition(anyOf(
+ declRefExpr(hasDeclaration(
+ varDecl(equalsBoundNode("cond_var")))),
+ binaryOperator(
+ hasAnyOperatorName("&&", "||"),
+ hasEitherOperand(declRefExpr(hasDeclaration(
+ varDecl(equalsBoundNode("cond_var")))))))))
+ .bind("inner_if")),
+ forFunction(functionDecl().bind("func")))
+ .bind("outer_if"),
+ this);
+}
+
+void RedundantConditionCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *OuterIf = Result.Nodes.getNodeAs<IfStmt>("outer_if");
+ const auto *InnerIf = Result.Nodes.getNodeAs<IfStmt>("inner_if");
+ const auto *CondVar = Result.Nodes.getNodeAs<VarDecl>("cond_var");
+ const auto *Func = Result.Nodes.getNodeAs<FunctionDecl>("func");
+
+ // Non-local variables may be changed by any call.
+ if (!CondVar->isLocalVarDeclOrParm())
+ return;
+
+ // Volatile variables may be changed by another thread.
+ if (CondVar->getType().isVolatileQualified())
+ return;
+
+ // Class instances may be tricky, so restrict ourselves to integers.
+ if (!CondVar->getType().getTypePtr()->isIntegerType())
+ return;
+
+ // 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))
+ return;
+
+ if (isChangedBefore(OuterIf, InnerIf, CondVar, Result.Context))
+ return;
+
+ auto Diag = diag(InnerIf->getBeginLoc(), "redundant condition %0") << CondVar;
+
+ // For standalone condition variables and for "or" binary operations we simply
+ // remove the inner `if`.
+ if (isa<DeclRefExpr>(InnerIf->getCond()->IgnoreParenImpCasts()) ||
+ (isa<BinaryOperator>(InnerIf->getCond()) &&
+ cast<BinaryOperator>(InnerIf->getCond())->getOpcode() == BO_LOr)) {
+ SourceLocation IfBegin = InnerIf->getBeginLoc();
+ const Stmt *Body = InnerIf->getThen();
+
+ // For comound statements also remove the right brace at the end.
+ if (isa<CompoundStmt>(Body)) {
+ SourceLocation IfEnd = Body->getBeginLoc().getLocWithOffset(1);
+ Diag << FixItHint::CreateRemoval(
+ CharSourceRange::getCharRange(IfBegin, IfEnd))
+ << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
+ Body->getEndLoc(), Body->getEndLoc().getLocWithOffset(1)));
+ } else {
+ SourceLocation IfEnd = Body->getBeginLoc();
+ Diag << FixItHint::CreateRemoval(
+ CharSourceRange::getCharRange(IfBegin, IfEnd));
+ }
+
+ // For "and" binary operations we remove the "and" operation with the
+ // condition variable from the inner if.
+ } else {
+ const auto *CondOp = cast<BinaryOperator>(InnerIf->getCond());
+ if (isa<DeclRefExpr>(CondOp->getLHS()->IgnoreParenImpCasts()) &&
+ cast<DeclRefExpr>(CondOp->getLHS()->IgnoreParenImpCasts())->getDecl() ==
+ CondVar) {
+ Diag << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
+ CondOp->getLHS()->getBeginLoc(), CondOp->getRHS()->getBeginLoc()));
+ } else {
+ // For some reason the end location of identifiers is identical to their
+ // begin location so add their length instead.
+ Diag << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
+ CondOp->getLHS()->getEndLoc().getLocWithOffset(1),
+ CondOp->getRHS()->getBeginLoc().getLocWithOffset(
+ CondVar->getIdentifier()->getLength())));
+ }
+ }
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
@@ -15,6 +15,7 @@
#include "NoRecursionCheck.h"
#include "NonCopyableObjects.h"
#include "NonPrivateMemberVariablesInClassesCheck.h"
+#include "RedundantConditionCheck.h"
#include "RedundantExpressionCheck.h"
#include "StaticAssertCheck.h"
#include "ThrowByValueCatchByReferenceCheck.h"
@@ -41,6 +42,8 @@
"misc-non-copyable-objects");
CheckFactories.registerCheck<NonPrivateMemberVariablesInClassesCheck>(
"misc-non-private-member-variables-in-classes");
+ CheckFactories.registerCheck<RedundantConditionCheck>(
+ "misc-redundant-condition");
CheckFactories.registerCheck<RedundantExpressionCheck>(
"misc-redundant-expression");
CheckFactories.registerCheck<StaticAssertCheck>("misc-static-assert");
Index: clang-tools-extra/clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/misc/CMakeLists.txt
@@ -11,6 +11,7 @@
NoRecursionCheck.cpp
NonCopyableObjects.cpp
NonPrivateMemberVariablesInClassesCheck.cpp
+ RedundantConditionCheck.cpp
RedundantExpressionCheck.cpp
StaticAssertCheck.cpp
ThrowByValueCatchByReferenceCheck.cpp
Index: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
@@ -10,8 +10,10 @@
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
+#include "../utils/Aliasing.h"
using namespace clang::ast_matchers;
+using clang::tidy::utils::hasPtrOrReferenceInFunc;
namespace clang {
namespace tidy {
@@ -24,54 +26,6 @@
callExpr(Internal, callee(functionDecl(isNoReturn())))));
}
-/// Return whether `S` is a reference to the declaration of `Var`.
-static bool isAccessForVar(const Stmt *S, const VarDecl *Var) {
- if (const auto *DRE = dyn_cast<DeclRefExpr>(S))
- return DRE->getDecl() == Var;
-
- return false;
-}
-
-/// Return whether `Var` has a pointer or reference in `S`.
-static bool isPtrOrReferenceForVar(const Stmt *S, const VarDecl *Var) {
- if (const auto *DS = dyn_cast<DeclStmt>(S)) {
- for (const Decl *D : DS->getDeclGroup()) {
- if (const auto *LeftVar = dyn_cast<VarDecl>(D)) {
- if (LeftVar->hasInit() && LeftVar->getType()->isReferenceType()) {
- return isAccessForVar(LeftVar->getInit(), Var);
- }
- }
- }
- } else if (const auto *UnOp = dyn_cast<UnaryOperator>(S)) {
- if (UnOp->getOpcode() == UO_AddrOf)
- return isAccessForVar(UnOp->getSubExpr(), Var);
- }
-
- return false;
-}
-
-/// Return whether `Var` has a pointer or reference in `S`.
-static bool hasPtrOrReferenceInStmt(const Stmt *S, const VarDecl *Var) {
- if (isPtrOrReferenceForVar(S, Var))
- return true;
-
- for (const Stmt *Child : S->children()) {
- if (!Child)
- continue;
-
- if (hasPtrOrReferenceInStmt(Child, Var))
- return true;
- }
-
- return false;
-}
-
-/// Return whether `Var` has a pointer or reference in `Func`.
-static bool hasPtrOrReferenceInFunc(const FunctionDecl *Func,
- const VarDecl *Var) {
- return hasPtrOrReferenceInStmt(Func->getBody(), Var);
-}
-
/// Return whether `Var` was changed in `LoopStmt`.
static bool isChanged(const Stmt *LoopStmt, const VarDecl *Var,
ASTContext *Context) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
