NoQ updated this revision to Diff 342567.
NoQ added a comment.
Unforget to `git add` Objective-{C,C++} tests.
Apart from testing the patch, some of these tests demonstrate that the checker
doesn't really work in Objective-C. The problem is that `forFunction()` doesn't
treat Objective-C methods as functions - a similar problem that caused false
negatives demonstrated D101787 <https://reviews.llvm.org/D101787>. I hope to
address this soon.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D101790/new/
https://reviews.llvm.org/D101790
Files:
clang-tools-extra/clang-tidy/utils/Aliasing.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.mm
clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
@@ -967,12 +967,29 @@
}
if (tryToExtinguish(onFire) && onFire) {
if (tryToExtinguishByVal(onFire) && onFire) {
- // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+ // NO-MESSAGE: technically tryToExtinguish() may launch
+ // a background thread to extinguish the fire while tryToExtinguishByVal()
+ // may be waiting for that thread to finish.
scream();
}
}
}
+bool &hidden_reference(bool &flag) {
+ return flag;
+}
+
+void test_hidden_reference() {
+ bool onFire = isBurning();
+ bool onFireRef = hidden_reference(onFire);
+ if (onFire) {
+ onFireRef = false;
+ if (onFire) {
+ // NO-MESSAGE: fire was extinguished by the above assignment
+ }
+ }
+}
+
void negative_reassigned() {
bool onFire = isBurning();
if (onFire) {
@@ -992,11 +1009,9 @@
bool onFire = isBurning();
if (onFire) {
if (onFire) {
- // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
- // CHECK-FIXES: {{^\ *$}}
+ // FIXME: This should warn.
scream();
}
- // CHECK-FIXES: {{^\ *$}}
tryToExtinguish(onFire);
}
}
@@ -1006,11 +1021,9 @@
if (onFire) {
if (someOtherCondition()) {
if (onFire) {
- // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
- // CHECK-FIXES: {{^\ *$}}
+ // FIXME: This should warn.
scream();
}
- // CHECK-FIXES: {{^\ *$}}
}
tryToExtinguish(onFire);
}
@@ -1021,11 +1034,9 @@
if (onFire) {
if (someOtherCondition()) {
if (onFire) {
- // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
- // CHECK-FIXES: {{^\ *$}}
+ // FIXME: This should warn.
scream();
}
- // CHECK-FIXES: {{^\ *$}}
tryToExtinguish(onFire);
}
}
@@ -1035,8 +1046,7 @@
bool onFire = isBurning();
if (onFire) {
if (onFire) {
- // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
- // CHECK-FIXES: {{^\ *$}}
+ // FIXME: This should warn.
tryToExtinguish(onFire);
scream();
}
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.mm
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.mm
@@ -0,0 +1,50 @@
+// RUN: %check_clang_tidy %s bugprone-infinite-loop %t -- -- \
+// RUN: -fexceptions -fblocks
+
+@interface I
+-(void) instanceMethod;
++(void) classMethod;
++(int &) hiddenReferenceTo: (int &)x;
+@end
+
+void plainCFunction() {
+ int i = 0;
+ int j = 0;
+ while (i < 10) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (i) are updated in the loop body [bugprone-infinite-loop]
+ j++;
+ }
+}
+
+void testHiddenReference() {
+ int i = 0;
+ int &j = [I hiddenReferenceTo: i];
+ while (i < 10) {
+ // No warning. 'j' is a reference to 'i'.
+ j++;
+ }
+}
+
+@implementation I
+- (void)instanceMethod {
+ int i = 0;
+ int j = 0;
+ while (i < 10) {
+ // FIXME: Should warn.
+ j++;
+ }
+}
+
++ (void)classMethod {
+ int i = 0;
+ int j = 0;
+ while (i < 10) {
+ // FIXME: Should warn.
+ j++;
+ }
+}
+
++(int &) hiddenReferenceTo: (int &) x {
+ return x;
+}
+@end
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
@@ -551,3 +551,28 @@
do {
} while (false && CondVar);
}
+
+int &hidden_reference(int &x) {
+ return x;
+}
+
+void test_hidden_reference() {
+ int x = 0;
+ int &y = hidden_reference(x);
+ for (; x < 10; ++y) {
+ // No warning. The loop is finite because 'y' is a reference to 'x'.
+ }
+}
+
+struct HiddenReference {
+ int &y;
+ HiddenReference(int &x) : y(x) {}
+};
+
+void test_HiddenReference() {
+ int x = 0;
+ int &y = HiddenReference(x).y;
+ for (; x < 10; ++y) {
+ // No warning. The loop is finite because 'y' is a reference to 'x'.
+ }
+}
Index: clang-tools-extra/clang-tidy/utils/Aliasing.cpp
===================================================================
--- clang-tools-extra/clang-tidy/utils/Aliasing.cpp
+++ clang-tools-extra/clang-tidy/utils/Aliasing.cpp
@@ -8,6 +8,7 @@
#include "Aliasing.h"
+#include "clang/Analysis/AnyCall.h"
#include "clang/AST/Expr.h"
#include "clang/AST/ExprCXX.h"
@@ -50,6 +51,10 @@
} else if (const auto *LE = dyn_cast<LambdaExpr>(S)) {
// Treat lambda capture by reference as a form of taking a reference.
return capturesByRef(LE->getLambdaClass(), Var);
+ } else if (Optional<AnyCall> Call = AnyCall::forExpr(S)) {
+ return llvm::any_of(Call->arguments(), [Var](const Expr *ArgE) {
+ return isAccessForVar(ArgE, Var);
+ });
}
return false;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
