[clang-tidy] fix fp when modifying variant by ``operator[]`` with template in parameters (#128407)
`ArraySubscriptExpr` can switch base and idx. For dependent array
subscript access, we should check both base and idx conservatively.
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 07a79d6..71edb70 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -122,7 +122,8 @@
- Improved :doc:`misc-const-correctness
<clang-tidy/checks/misc/const-correctness>` check by adding the option
`AllowedTypes`, that excludes specified types from const-correctness
- checking.
+ checking and fixing false positives when modifying variant by ``operator[]``
+ with template in parameters.
- Improved :doc:`misc-redundant-expression
<clang-tidy/checks/misc/redundant-expression>` check by providing additional
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp
index 5efb64b..654deea 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp
@@ -998,3 +998,11 @@
// CHECK-MESSAGES:[[@LINE-1]]:3: warning: variable 'member_pointer_tmp' of type 'Value &' can be declared 'const'
(member_pointer_tmp.*m)();
}
+
+namespace gh127776_false_positive {
+template <class T> struct vector { T &operator[](int t); };
+template <typename T> void f() {
+ vector<int> x;
+ x[T{}] = 3;
+}
+} // namespace gh127776_false_positive
diff --git a/clang/lib/Analysis/ExprMutationAnalyzer.cpp b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
index 8944343..823d754 100644
--- a/clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -80,6 +80,17 @@
namespace {
+// `ArraySubscriptExpr` can switch base and idx, e.g. `a[4]` is the same as
+// `4[a]`. When type is dependent, we conservatively assume both sides are base.
+AST_MATCHER_P(ArraySubscriptExpr, hasBaseConservative,
+ ast_matchers::internal::Matcher<Expr>, InnerMatcher) {
+ if (Node.isTypeDependent()) {
+ return InnerMatcher.matches(*Node.getLHS(), Finder, Builder) ||
+ InnerMatcher.matches(*Node.getRHS(), Finder, Builder);
+ }
+ return InnerMatcher.matches(*Node.getBase(), Finder, Builder);
+}
+
AST_MATCHER(Type, isDependentType) { return Node.isDependentType(); }
AST_MATCHER_P(LambdaExpr, hasCaptureInit, const Expr *, E) {
@@ -513,8 +524,8 @@
// Check whether any element of an array is mutated.
const auto SubscriptExprs = match(
findAll(arraySubscriptExpr(
- anyOf(hasBase(canResolveToExpr(Exp)),
- hasBase(implicitCastExpr(allOf(
+ anyOf(hasBaseConservative(canResolveToExpr(Exp)),
+ hasBaseConservative(implicitCastExpr(allOf(
hasCastKind(CK_ArrayToPointerDecay),
hasSourceExpression(canResolveToExpr(Exp)))))))
.bind(NodeID<Expr>::value)),
@@ -716,7 +727,8 @@
unaryOperator(hasOperatorName("*"),
hasUnaryOperand(canResolveToExprPointee(Exp))),
// deref by []
- arraySubscriptExpr(hasBase(canResolveToExprPointee(Exp)))))
+ arraySubscriptExpr(
+ hasBaseConservative(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 cc277d5..7209992 100644
--- a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -870,6 +870,19 @@
EXPECT_THAT(mutatedBy(ResultsY, AST.get()), ElementsAre("y"));
}
+TEST(ExprMutationAnalyzerTest, T1) {
+ const auto AST = buildASTFromCodeWithArgs(
+ "template <class T> struct vector { T &operator[](int t); };"
+ "template <typename T> void func() {"
+ " vector<int> x;"
+ " x[T{}] = 3;"
+ "}",
+ {"-fno-delayed-template-parsing"});
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_TRUE(isMutated(Results, AST.get()));
+}
+
// section: special case: all created references are non-mutating themself
// and therefore all become 'const'/the value is not modified!
