-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[ExprMutation] fix false postives on pointer-to-member operator #166069
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@llvm/pr-subscribers-clang Author: Congcong Cai (HerrCai0907) ChangesFixed: #161913 Full diff: https://github.com/llvm/llvm-project/pull/166069.diff 3 Files Affected:
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index ab7dc87d9d5f3..7fa096a9495f2 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -369,7 +369,8 @@ Changes in existing checks
<clang-tidy/checks/misc/const-correctness>` check to avoid false
positives when pointers is transferred to non-const references
and avoid false positives of function pointer and fix false
- positives on return of non-const pointer.
+ positives on return of non-const pointer and fix false positive on
+ pointer-to-member operator.
- Improved :doc:`misc-header-include-cycle
<clang-tidy/checks/misc/header-include-cycle>` check performance.
diff --git a/clang/lib/Analysis/ExprMutationAnalyzer.cpp b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
index 75b17c545bb78..54c30c05c3e19 100644
--- a/clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -746,11 +746,14 @@ ExprMutationAnalyzer::Analyzer::findPointeeMemberMutation(const Expr *Exp) {
Stm, Context));
if (MemberCallExpr)
return MemberCallExpr;
- const auto Matches =
- match(stmt(forEachDescendant(
- memberExpr(hasObjectExpression(canResolveToExprPointee(Exp)))
- .bind(NodeID<Expr>::value))),
- Stm, Context);
+ const auto Matches = match(
+ stmt(forEachDescendant(
+ expr(anyOf(memberExpr(
+ hasObjectExpression(canResolveToExprPointee(Exp))),
+ binaryOperator(hasOperatorName("->*"),
+ hasLHS(canResolveToExprPointee(Exp)))))
+ .bind(NodeID<Expr>::value))),
+ Stm, Context);
return findExprMutation(Matches);
}
diff --git a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
index ef229606de0f0..f7b799ab58719 100644
--- a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -2076,4 +2076,20 @@ TEST(ExprMutationAnalyzerTest, PointeeMutatedByReturn) {
}
}
+TEST(ExprMutationAnalyzerTest, PointeeMutatedByPointerToMemberOperator) {
+ // GH161913
+ const std::string Code = R"(
+ struct S { int i; };
+ void f(S s) {
+ S *x = &s;
+ (x->*(&S::i))++;
+ }
+ )";
+ auto AST = buildASTFromCodeWithArgs(Code, {"-Wno-everything"});
+ auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_TRUE(isPointeeMutated(Results, AST.get()));
+}
+
+
} // namespace clang
|
|
@llvm/pr-subscribers-clang-tools-extra Author: Congcong Cai (HerrCai0907) ChangesFixed: #161913 Full diff: https://github.com/llvm/llvm-project/pull/166069.diff 3 Files Affected:
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index ab7dc87d9d5f3..7fa096a9495f2 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -369,7 +369,8 @@ Changes in existing checks
<clang-tidy/checks/misc/const-correctness>` check to avoid false
positives when pointers is transferred to non-const references
and avoid false positives of function pointer and fix false
- positives on return of non-const pointer.
+ positives on return of non-const pointer and fix false positive on
+ pointer-to-member operator.
- Improved :doc:`misc-header-include-cycle
<clang-tidy/checks/misc/header-include-cycle>` check performance.
diff --git a/clang/lib/Analysis/ExprMutationAnalyzer.cpp b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
index 75b17c545bb78..54c30c05c3e19 100644
--- a/clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -746,11 +746,14 @@ ExprMutationAnalyzer::Analyzer::findPointeeMemberMutation(const Expr *Exp) {
Stm, Context));
if (MemberCallExpr)
return MemberCallExpr;
- const auto Matches =
- match(stmt(forEachDescendant(
- memberExpr(hasObjectExpression(canResolveToExprPointee(Exp)))
- .bind(NodeID<Expr>::value))),
- Stm, Context);
+ const auto Matches = match(
+ stmt(forEachDescendant(
+ expr(anyOf(memberExpr(
+ hasObjectExpression(canResolveToExprPointee(Exp))),
+ binaryOperator(hasOperatorName("->*"),
+ hasLHS(canResolveToExprPointee(Exp)))))
+ .bind(NodeID<Expr>::value))),
+ Stm, Context);
return findExprMutation(Matches);
}
diff --git a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
index ef229606de0f0..f7b799ab58719 100644
--- a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -2076,4 +2076,20 @@ TEST(ExprMutationAnalyzerTest, PointeeMutatedByReturn) {
}
}
+TEST(ExprMutationAnalyzerTest, PointeeMutatedByPointerToMemberOperator) {
+ // GH161913
+ const std::string Code = R"(
+ struct S { int i; };
+ void f(S s) {
+ S *x = &s;
+ (x->*(&S::i))++;
+ }
+ )";
+ auto AST = buildASTFromCodeWithArgs(Code, {"-Wno-everything"});
+ auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_TRUE(isPointeeMutated(Results, AST.get()));
+}
+
+
} // namespace clang
|
|
@llvm/pr-subscribers-clang-analysis Author: Congcong Cai (HerrCai0907) ChangesFixed: #161913 Full diff: https://github.com/llvm/llvm-project/pull/166069.diff 3 Files Affected:
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index ab7dc87d9d5f3..7fa096a9495f2 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -369,7 +369,8 @@ Changes in existing checks
<clang-tidy/checks/misc/const-correctness>` check to avoid false
positives when pointers is transferred to non-const references
and avoid false positives of function pointer and fix false
- positives on return of non-const pointer.
+ positives on return of non-const pointer and fix false positive on
+ pointer-to-member operator.
- Improved :doc:`misc-header-include-cycle
<clang-tidy/checks/misc/header-include-cycle>` check performance.
diff --git a/clang/lib/Analysis/ExprMutationAnalyzer.cpp b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
index 75b17c545bb78..54c30c05c3e19 100644
--- a/clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -746,11 +746,14 @@ ExprMutationAnalyzer::Analyzer::findPointeeMemberMutation(const Expr *Exp) {
Stm, Context));
if (MemberCallExpr)
return MemberCallExpr;
- const auto Matches =
- match(stmt(forEachDescendant(
- memberExpr(hasObjectExpression(canResolveToExprPointee(Exp)))
- .bind(NodeID<Expr>::value))),
- Stm, Context);
+ const auto Matches = match(
+ stmt(forEachDescendant(
+ expr(anyOf(memberExpr(
+ hasObjectExpression(canResolveToExprPointee(Exp))),
+ binaryOperator(hasOperatorName("->*"),
+ hasLHS(canResolveToExprPointee(Exp)))))
+ .bind(NodeID<Expr>::value))),
+ Stm, Context);
return findExprMutation(Matches);
}
diff --git a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
index ef229606de0f0..f7b799ab58719 100644
--- a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -2076,4 +2076,20 @@ TEST(ExprMutationAnalyzerTest, PointeeMutatedByReturn) {
}
}
+TEST(ExprMutationAnalyzerTest, PointeeMutatedByPointerToMemberOperator) {
+ // GH161913
+ const std::string Code = R"(
+ struct S { int i; };
+ void f(S s) {
+ S *x = &s;
+ (x->*(&S::i))++;
+ }
+ )";
+ auto AST = buildASTFromCodeWithArgs(Code, {"-Wno-everything"});
+ auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_TRUE(isPointeeMutated(Results, AST.get()));
+}
+
+
} // namespace clang
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
f7b434c to
90ecfe1
Compare
vbvictor
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Co-authored-by: Baranov Victor <[email protected]>
Fixed: #161913