[PATCH] D79912: Assignment and Inc/Dec operators wouldn't register as a mutation when Implicit Paren Casts were present
Tridacnid added a comment. Ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79912/new/ https://reviews.llvm.org/D79912 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79912: Assignment and Inc/Dec operators wouldn't register as a mutation when Implicit Paren Casts were present
Tridacnid updated this revision to Diff 268991.
Tridacnid added a comment.
Add (Limit) -= 1 test cases to bugprone-infinite-loop tests
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79912/new/
https://reviews.llvm.org/D79912
Files:
clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
clang/lib/Analysis/ExprMutationAnalyzer.cpp
clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
Index: clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
===
--- clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -111,11 +111,21 @@
class AssignmentTest : public ::testing::TestWithParam {};
TEST_P(AssignmentTest, AssignmentModifies) {
- const std::string ModExpr = "x " + GetParam() + " 10";
- const auto AST = buildASTFromCode("void f() { int x; " + ModExpr + "; }");
- const auto Results =
- match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
- EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+ {
+const std::string ModExpr = "x " + GetParam() + " 10";
+const auto AST = buildASTFromCode("void f() { int x; " + ModExpr + "; }");
+const auto Results =
+match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+ }
+
+ {
+const std::string ModExpr = "(x) " + GetParam() + " 10";
+const auto AST = buildASTFromCode("void f() { int x; " + ModExpr + "; }");
+const auto Results =
+match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+ }
}
INSTANTIATE_TEST_CASE_P(AllAssignmentOperators, AssignmentTest,
@@ -133,7 +143,8 @@
}
INSTANTIATE_TEST_CASE_P(AllIncDecOperators, IncDecTest,
-Values("++x", "--x", "x++", "x--"), );
+Values("++x", "--x", "x++", "x--", "++(x)", "--(x)",
+ "(x)++", "(x)--"), );
TEST(ExprMutationAnalyzerTest, NonConstMemberFunc) {
const auto AST = buildASTFromCode(
Index: clang/lib/Analysis/ExprMutationAnalyzer.cpp
===
--- clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -201,14 +201,15 @@
const Stmt *ExprMutationAnalyzer::findDirectMutation(const Expr *Exp) {
// LHS of any assignment operators.
- const auto AsAssignmentLhs =
- binaryOperator(isAssignmentOperator(),
- hasLHS(maybeEvalCommaExpr(equalsNode(Exp;
+ const auto AsAssignmentLhs = binaryOperator(
+ isAssignmentOperator(),
+ hasLHS(maybeEvalCommaExpr(ignoringParenImpCasts(equalsNode(Exp);
// Operand of increment/decrement operators.
const auto AsIncDecOperand =
unaryOperator(anyOf(hasOperatorName("++"), hasOperatorName("--")),
-hasUnaryOperand(maybeEvalCommaExpr(equalsNode(Exp;
+hasUnaryOperand(maybeEvalCommaExpr(
+ignoringParenImpCasts(equalsNode(Exp);
// Invoking non-const member function.
// A member function is assumed to be non-const when it is unresolved.
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
@@ -70,11 +70,25 @@
i++;
}
+ while ((Limit)--) {
+// Not an error since 'Limit' is updated.
+i++;
+ }
+
+ while ((Limit) -= 1) {
+// Not an error since 'Limit' is updated.
+ }
+
while (int k = Limit) {
// Not an error since 'Limit' is updated.
Limit--;
}
+ while (int k = Limit) {
+// Not an error since 'Limit' is updated
+(Limit)--;
+ }
+
while (int k = Limit--) {
// Not an error since 'Limit' is updated.
i++;
@@ -86,6 +100,15 @@
for (i = 0; i < Limit; Limit--) {
}
+
+ for (i = 0; i < Limit; (Limit) = Limit - 1) {
+ }
+
+ for (i = 0; i < Limit; (Limit) -= 1) {
+ }
+
+ for (i = 0; i < Limit; --(Limit)) {
+ }
}
void simple_not_infinite2() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79912: Assignment and Inc/Dec operators wouldn't register as a mutation when Implicit Paren Casts were present
Tridacnid added a comment. Awesome. I don't have commit access, https://llvm.org/docs/Phabricator.html says I just need to ask here and someone will pick it up. Let me know if there's anything else I need to do. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79912/new/ https://reviews.llvm.org/D79912 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79912: Assignment and Inc/Dec operators wouldn't register as a mutation when Implicit Paren Casts were present
Tridacnid added a comment. tridac...@gmail.com https://github.com/Tridacnid Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79912/new/ https://reviews.llvm.org/D79912 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79912: Assignment and Inc/Dec operators wouldn't register as a mutation when Implicit Paren Casts were present
Tridacnid added a comment. Bug report has been closed. I'm seeing some build failures in my inbox but eyeballing them doesn't make me think this change is related. What is the expectation for me in this scenario? Will I get an email letting me know once the build has been fixed? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79912/new/ https://reviews.llvm.org/D79912 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79912: Assignment and Inc/Dec operators wouldn't register as a mutation when Implicit Paren Casts were present
Tridacnid added a comment. I did run both of the testing targets you mentioned, but only on Linux. I'll keep an eye on the buildbot, thanks for the info! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79912/new/ https://reviews.llvm.org/D79912 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79912: Assignment and Inc/Dec operators wouldn't register as a mutation when Implicit Paren Casts were present
Tridacnid created this revision.
Tridacnid added reviewers: llvm-commits, alexfh, klimek.
Tridacnid added projects: clang-tools-extra, clang.
Herald added a subscriber: cfe-commits.
Add ignoringParenImpCasts to assignment and inc/dec mutation checks in
ExprMutationAnalyzer to fix clang-tidy bug PR45490.
https://bugs.llvm.org/show_bug.cgi?id=45490
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D79912
Files:
clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
clang/lib/Analysis/ExprMutationAnalyzer.cpp
clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
Index: clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
===
--- clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -111,11 +111,21 @@
class AssignmentTest : public ::testing::TestWithParam {};
TEST_P(AssignmentTest, AssignmentModifies) {
- const std::string ModExpr = "x " + GetParam() + " 10";
- const auto AST = buildASTFromCode("void f() { int x; " + ModExpr + "; }");
- const auto Results =
- match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
- EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+ {
+const std::string ModExpr = "x " + GetParam() + " 10";
+const auto AST = buildASTFromCode("void f() { int x; " + ModExpr + "; }");
+const auto Results =
+match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+ }
+
+ {
+const std::string ModExpr = "(x) " + GetParam() + " 10";
+const auto AST = buildASTFromCode("void f() { int x; " + ModExpr + "; }");
+const auto Results =
+match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+ }
}
INSTANTIATE_TEST_CASE_P(AllAssignmentOperators, AssignmentTest,
@@ -133,7 +143,7 @@
}
INSTANTIATE_TEST_CASE_P(AllIncDecOperators, IncDecTest,
-Values("++x", "--x", "x++", "x--"), );
+Values("++x", "--x", "x++", "x--", "++(x)", "--(x)", "(x)++", "(x)--"), );
TEST(ExprMutationAnalyzerTest, NonConstMemberFunc) {
const auto AST = buildASTFromCode(
Index: clang/lib/Analysis/ExprMutationAnalyzer.cpp
===
--- clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -201,14 +201,15 @@
const Stmt *ExprMutationAnalyzer::findDirectMutation(const Expr *Exp) {
// LHS of any assignment operators.
- const auto AsAssignmentLhs =
- binaryOperator(isAssignmentOperator(),
- hasLHS(maybeEvalCommaExpr(equalsNode(Exp;
+ const auto AsAssignmentLhs = binaryOperator(
+ isAssignmentOperator(),
+ hasLHS(maybeEvalCommaExpr(ignoringParenImpCasts(equalsNode(Exp);
// Operand of increment/decrement operators.
const auto AsIncDecOperand =
unaryOperator(anyOf(hasOperatorName("++"), hasOperatorName("--")),
-hasUnaryOperand(maybeEvalCommaExpr(equalsNode(Exp;
+hasUnaryOperand(maybeEvalCommaExpr(
+ignoringParenImpCasts(equalsNode(Exp);
// Invoking non-const member function.
// A member function is assumed to be non-const when it is unresolved.
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
@@ -70,11 +70,21 @@
i++;
}
+ while ((Limit)--) {
+// Not an error since 'Limit' is updated.
+i++;
+ }
+
while (int k = Limit) {
// Not an error since 'Limit' is updated.
Limit--;
}
+ while (int k = Limit) {
+// Not an error since 'Limit' is updated
+(Limit)--;
+ }
+
while (int k = Limit--) {
// Not an error since 'Limit' is updated.
i++;
@@ -86,6 +96,12 @@
for (i = 0; i < Limit; Limit--) {
}
+
+ for (i = 0; i < Limit; (Limit) = Limit - 1) {
+ }
+
+ for (i = 0; i < Limit; --(Limit)) {
+ }
}
void simple_not_infinite2() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79912: Assignment and Inc/Dec operators wouldn't register as a mutation when Implicit Paren Casts were present
Tridacnid updated this revision to Diff 263885.
Tridacnid added a comment.
Apply clang-format patch.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79912/new/
https://reviews.llvm.org/D79912
Files:
clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
Index: clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
===
--- clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -146 +146,2 @@
-Values("++x", "--x", "x++", "x--", "++(x)", "--(x)",
"(x)++", "(x)--"), );
+Values("++x", "--x", "x++", "x--", "++(x)", "--(x)",
+ "(x)++", "(x)--"), );
Index: clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
===
--- clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -146 +146,2 @@
-Values("++x", "--x", "x++", "x--", "++(x)", "--(x)", "(x)++", "(x)--"), );
+Values("++x", "--x", "x++", "x--", "++(x)", "--(x)",
+ "(x)++", "(x)--"), );
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79912: Assignment and Inc/Dec operators wouldn't register as a mutation when Implicit Paren Casts were present
Tridacnid updated this revision to Diff 263887.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79912/new/
https://reviews.llvm.org/D79912
Files:
clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
clang/lib/Analysis/ExprMutationAnalyzer.cpp
clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
Index: clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
===
--- clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -111,11 +111,21 @@
class AssignmentTest : public ::testing::TestWithParam {};
TEST_P(AssignmentTest, AssignmentModifies) {
- const std::string ModExpr = "x " + GetParam() + " 10";
- const auto AST = buildASTFromCode("void f() { int x; " + ModExpr + "; }");
- const auto Results =
- match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
- EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+ {
+const std::string ModExpr = "x " + GetParam() + " 10";
+const auto AST = buildASTFromCode("void f() { int x; " + ModExpr + "; }");
+const auto Results =
+match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+ }
+
+ {
+const std::string ModExpr = "(x) " + GetParam() + " 10";
+const auto AST = buildASTFromCode("void f() { int x; " + ModExpr + "; }");
+const auto Results =
+match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+ }
}
INSTANTIATE_TEST_CASE_P(AllAssignmentOperators, AssignmentTest,
@@ -133,7 +143,8 @@
}
INSTANTIATE_TEST_CASE_P(AllIncDecOperators, IncDecTest,
-Values("++x", "--x", "x++", "x--"), );
+Values("++x", "--x", "x++", "x--", "++(x)", "--(x)",
+ "(x)++", "(x)--"), );
TEST(ExprMutationAnalyzerTest, NonConstMemberFunc) {
const auto AST = buildASTFromCode(
Index: clang/lib/Analysis/ExprMutationAnalyzer.cpp
===
--- clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -201,14 +201,15 @@
const Stmt *ExprMutationAnalyzer::findDirectMutation(const Expr *Exp) {
// LHS of any assignment operators.
- const auto AsAssignmentLhs =
- binaryOperator(isAssignmentOperator(),
- hasLHS(maybeEvalCommaExpr(equalsNode(Exp;
+ const auto AsAssignmentLhs = binaryOperator(
+ isAssignmentOperator(),
+ hasLHS(maybeEvalCommaExpr(ignoringParenImpCasts(equalsNode(Exp);
// Operand of increment/decrement operators.
const auto AsIncDecOperand =
unaryOperator(anyOf(hasOperatorName("++"), hasOperatorName("--")),
-hasUnaryOperand(maybeEvalCommaExpr(equalsNode(Exp;
+hasUnaryOperand(maybeEvalCommaExpr(
+ignoringParenImpCasts(equalsNode(Exp);
// Invoking non-const member function.
// A member function is assumed to be non-const when it is unresolved.
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
@@ -70,11 +70,21 @@
i++;
}
+ while ((Limit)--) {
+// Not an error since 'Limit' is updated.
+i++;
+ }
+
while (int k = Limit) {
// Not an error since 'Limit' is updated.
Limit--;
}
+ while (int k = Limit) {
+// Not an error since 'Limit' is updated
+(Limit)--;
+ }
+
while (int k = Limit--) {
// Not an error since 'Limit' is updated.
i++;
@@ -86,6 +96,12 @@
for (i = 0; i < Limit; Limit--) {
}
+
+ for (i = 0; i < Limit; (Limit) = Limit - 1) {
+ }
+
+ for (i = 0; i < Limit; --(Limit)) {
+ }
}
void simple_not_infinite2() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79912: Assignment and Inc/Dec operators wouldn't register as a mutation when Implicit Paren Casts were present
Tridacnid added a comment. I don't think that did what I wanted. Looks like I should submit the whole change as a single patch? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79912/new/ https://reviews.llvm.org/D79912 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
