https://github.com/5chmidti updated
https://github.com/llvm/llvm-project/pull/70559
>From b29eb35fe8597ceefc4c615817174181a16f3c4c Mon Sep 17 00:00:00 2001
From: Julian Schmidt <44101708+5chmi...@users.noreply.github.com>
Date: Sat, 28 Oct 2023 18:08:51 +0200
Subject: [PATCH 1/4] [clang-tidy] fix match for binaryOperator in
ExprMutationAnalyzer for misc-const-correctness
The `ExprMutationAnalyzer`s matcher of `binaryOperator`s
that contained the variable expr, were previously narrowing the
variable to be type dependent, when the `binaryOperator` should
have been narrowed as dependent.
The variable we are trying to find mutations for does
not need to be the dependent type, the other operand of
the `binaryOperator` could be dependent.
Fixes #57297
---
clang-tools-extra/docs/ReleaseNotes.rst | 5 +++++
.../checkers/misc/const-correctness-templates.cpp | 11 +++++++++++
clang/docs/ReleaseNotes.rst | 3 +++
clang/lib/Analysis/ExprMutationAnalyzer.cpp | 6 +++---
clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp | 11 +++++++++++
5 files changed, 33 insertions(+), 3 deletions(-)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index 5ce38ab48bf295f..bb75c9a3ce08125 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -308,6 +308,11 @@ Changes in existing checks
<clang-tidy/checks/misc/const-correctness>` check to avoid false positive
when
using pointer to member function.
+- Improved :doc:`misc-const-correctness
+ <clang-tidy/checks/misc/const-correctness>` check to not warn on uses in
+ type-dependent binary operators, when the variable that is being
+ looked at, is not the dependent operand.
+
- Improved :doc:`misc-include-cleaner
<clang-tidy/checks/misc/include-cleaner>` check by adding option
`DeduplicateFindings` to output one finding per symbol occurrence, avoid
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp
b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp
index 7b5ccabdd6ef611..415bb06020b9dc3 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp
@@ -20,3 +20,14 @@ void instantiate_template_cases() {
type_dependent_variables<int>();
type_dependent_variables<float>();
}
+
+namespace gh57297{
+struct Stream {};
+// Explicitly not declaring a (templated) stream operator
+// so the `<<` is a `binaryOperator` with a dependent type.
+template <typename T> void f() {
+ T t;
+ Stream stream;
+ stream << t;
+}
+} // namespace gh57297
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 2595737e8b3b143..fb9afc0646ac8cb 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -683,6 +683,9 @@ Bug Fixes to AST Handling
- Fixed a bug where RecursiveASTVisitor fails to visit the
initializer of a bitfield.
`Issue 64916 <https://github.com/llvm/llvm-project/issues/64916>`_
+- Fixed a bug where ``ExprMutationAnalyzer`` did not find a potential mutation
+ for uses in type-dependent binary operators, when the variable that is being
+ looked at, is not the dependent operand.
Miscellaneous Bug Fixes
^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Analysis/ExprMutationAnalyzer.cpp
b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
index f2e1beb025423cf..624a643cc60e4ba 100644
--- a/clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -296,9 +296,9 @@ const Stmt *ExprMutationAnalyzer::findDirectMutation(const
Expr *Exp) {
// resolved and modelled as `binaryOperator` on a dependent type.
// Such instances are considered a modification, because they can modify
// in different instantiations of the template.
- binaryOperator(hasEitherOperand(
- allOf(ignoringImpCasts(canResolveToExpr(equalsNode(Exp))),
- isTypeDependent()))),
+ binaryOperator(
+
hasEitherOperand(ignoringImpCasts(canResolveToExpr(equalsNode(Exp)))),
+ isTypeDependent()),
// Within class templates and member functions the member expression
might
// not be resolved. In that case, the `callExpr` is considered to be a
// modification.
diff --git a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
index c0a398394093c48..cfa3c535ce35292 100644
--- a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -343,6 +343,17 @@ TEST(ExprMutationAnalyzerTest, UnresolvedOperator) {
EXPECT_TRUE(isMutated(Results, AST.get()));
}
+TEST(ExprMutationAnalyzerTest, DependentOperatorWithNonDependentOperand) {
+ // gh57297
+ // Explicitly not declaring a (templated) stream operator
+ // so the `<<` is a `binaryOperator` with a dependent type.
+ const auto AST = buildASTFromCode("struct Stream {}; template <typename T> "
+ "void f() { T t; Stream x; x << t;}");
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x << t"));
+}
+
// Section: expression as call argument
TEST(ExprMutationAnalyzerTest, ByValueArgument) {
>From 3909609b88e1643aa13f21613123f693bae52ba1 Mon Sep 17 00:00:00 2001
From: Julian Schmidt <44101708+5chmi...@users.noreply.github.com>
Date: Sat, 28 Oct 2023 19:57:51 +0200
Subject: [PATCH 2/4] rm release note comment from clang specific docs
---
clang/docs/ReleaseNotes.rst | 3 ---
1 file changed, 3 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index fb9afc0646ac8cb..2595737e8b3b143 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -683,9 +683,6 @@ Bug Fixes to AST Handling
- Fixed a bug where RecursiveASTVisitor fails to visit the
initializer of a bitfield.
`Issue 64916 <https://github.com/llvm/llvm-project/issues/64916>`_
-- Fixed a bug where ``ExprMutationAnalyzer`` did not find a potential mutation
- for uses in type-dependent binary operators, when the variable that is being
- looked at, is not the dependent operand.
Miscellaneous Bug Fixes
^^^^^^^^^^^^^^^^^^^^^^^
>From 196eb2ac73126db4272a632d3391f0dd17703107 Mon Sep 17 00:00:00 2001
From: Julian Schmidt <44101708+5chmi...@users.noreply.github.com>
Date: Wed, 8 Nov 2023 19:33:14 +0100
Subject: [PATCH 3/4] fix windows CI failure
- explicitly instantiate template
- provide operator int() to Stream to ensure << is a BinaryOperator,
not a CXXOperatorCallExpr
---
.../misc/const-correctness-templates.cpp | 19 +++++++++++--------
.../Analysis/ExprMutationAnalyzerTest.cpp | 16 ++++++++++++----
2 files changed, 23 insertions(+), 12 deletions(-)
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp
b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp
index 415bb06020b9dc3..98063bcf47af8f1 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp
@@ -22,12 +22,15 @@ void instantiate_template_cases() {
}
namespace gh57297{
-struct Stream {};
-// Explicitly not declaring a (templated) stream operator
-// so the `<<` is a `binaryOperator` with a dependent type.
-template <typename T> void f() {
- T t;
- Stream stream;
- stream << t;
-}
+// The expression to check may not be the dependent operand in a dependent
+// operator.
+
+// Explicitly adding the conversion operator to int for `Stream` to make the
+// explicit instantiation (required due to windows' delayed template parsing
+// pre C++20) of `f` work without compile errors. Writing an `operator<<` for
+// `Stream` would make the `x << t` expression a CXXOperatorCallExpr, not a
+// BinaryOperator.
+struct Stream { operator int(); };
+template <typename T> void f() { T t; Stream x; x << t; }
+void foo() { f<int>(); }
} // namespace gh57297
diff --git a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
index cfa3c535ce35292..6d8582b48ee0a02 100644
--- a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -345,10 +345,18 @@ TEST(ExprMutationAnalyzerTest, UnresolvedOperator) {
TEST(ExprMutationAnalyzerTest, DependentOperatorWithNonDependentOperand) {
// gh57297
- // Explicitly not declaring a (templated) stream operator
- // so the `<<` is a `binaryOperator` with a dependent type.
- const auto AST = buildASTFromCode("struct Stream {}; template <typename T> "
- "void f() { T t; Stream x; x << t;}");
+ // The expression to check may not be the dependent operand in a dependent
+ // operator.
+
+ // Explicitly adding the conversion operator to int for `Stream` to make the
+ // explicit instantiation (required due to windows' delayed template parsing
+ // pre C++20) of `f` work without compile errors. Writing an `operator<<` for
+ // `Stream` would make the `x << t` expression a CXXOperatorCallExpr, not a
+ // BinaryOperator.
+ const auto AST = buildASTFromCode(
+ "struct Stream { operator int(); };"
+ "template <typename T> void f() { T t; Stream x; x << t; }"
+ "void foo() { f<int>(); }");
const auto Results =
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x << t"));
>From 1b6dead9af110a4013b74d54749c55c5005ffe04 Mon Sep 17 00:00:00 2001
From: Julian Schmidt <44101708+5chmi...@users.noreply.github.com>
Date: Sun, 12 Nov 2023 23:06:50 +0100
Subject: [PATCH 4/4] test with `-fno-delayed-template-parsing` instead
---
.../checkers/misc/const-correctness-templates.cpp | 10 +++-------
.../Analysis/ExprMutationAnalyzerTest.cpp | 15 ++++++---------
2 files changed, 9 insertions(+), 16 deletions(-)
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp
b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp
index 98063bcf47af8f1..794578ceeeba8fc 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp
@@ -25,12 +25,8 @@ namespace gh57297{
// The expression to check may not be the dependent operand in a dependent
// operator.
-// Explicitly adding the conversion operator to int for `Stream` to make the
-// explicit instantiation (required due to windows' delayed template parsing
-// pre C++20) of `f` work without compile errors. Writing an `operator<<` for
-// `Stream` would make the `x << t` expression a CXXOperatorCallExpr, not a
-// BinaryOperator.
-struct Stream { operator int(); };
+// Explicitly not declaring a (templated) stream operator
+// so the `<<` is a `binaryOperator` with a dependent type.
+struct Stream { };
template <typename T> void f() { T t; Stream x; x << t; }
-void foo() { f<int>(); }
} // namespace gh57297
diff --git a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
index 6d8582b48ee0a02..f1a1f857a0ee5be 100644
--- a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -348,15 +348,12 @@ TEST(ExprMutationAnalyzerTest,
DependentOperatorWithNonDependentOperand) {
// The expression to check may not be the dependent operand in a dependent
// operator.
- // Explicitly adding the conversion operator to int for `Stream` to make the
- // explicit instantiation (required due to windows' delayed template parsing
- // pre C++20) of `f` work without compile errors. Writing an `operator<<` for
- // `Stream` would make the `x << t` expression a CXXOperatorCallExpr, not a
- // BinaryOperator.
- const auto AST = buildASTFromCode(
- "struct Stream { operator int(); };"
- "template <typename T> void f() { T t; Stream x; x << t; }"
- "void foo() { f<int>(); }");
+ // Explicitly not declaring a (templated) stream operator
+ // so the `<<` is a `binaryOperator` with a dependent type.
+ const auto AST = buildASTFromCodeWithArgs(
+ "struct Stream { };"
+ "template <typename T> void f() { T t; Stream x; x << t; }",
+ {"-fno-delayed-template-parsing"});
const auto Results =
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x << t"));
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
