Skip to content

Commit a3d76b3

Browse files
authored
[clang-tidy] fix match for binaryOperator in ExprMutationAnalyzer for misc-const-correctness (#70559)
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
1 parent 447af1c commit a3d76b3

File tree

4 files changed

+32
-4
lines changed

4 files changed

+32
-4
lines changed

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,9 @@ Changes in existing checks
315315

316316
- Improved :doc:`misc-const-correctness
317317
<clang-tidy/checks/misc/const-correctness>` check to avoid false positive when
318-
using pointer to member function.
318+
using pointer to member function. Additionally, the check no longer emits
319+
a diagnostic when a variable that is not type-dependent is an operand of a
320+
type-dependent binary operator.
319321

320322
- Improved :doc:`misc-include-cleaner
321323
<clang-tidy/checks/misc/include-cleaner>` check by adding option

clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,13 @@ void instantiate_template_cases() {
2020
type_dependent_variables<int>();
2121
type_dependent_variables<float>();
2222
}
23+
24+
namespace gh57297{
25+
// The expression to check may not be the dependent operand in a dependent
26+
// operator.
27+
28+
// Explicitly not declaring a (templated) stream operator
29+
// so the `<<` is a `binaryOperator` with a dependent type.
30+
struct Stream { };
31+
template <typename T> void f() { T t; Stream x; x << t; }
32+
} // namespace gh57297

clang/lib/Analysis/ExprMutationAnalyzer.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -296,9 +296,9 @@ const Stmt *ExprMutationAnalyzer::findDirectMutation(const Expr *Exp) {
296296
// resolved and modelled as `binaryOperator` on a dependent type.
297297
// Such instances are considered a modification, because they can modify
298298
// in different instantiations of the template.
299-
binaryOperator(hasEitherOperand(
300-
allOf(ignoringImpCasts(canResolveToExpr(equalsNode(Exp))),
301-
isTypeDependent()))),
299+
binaryOperator(
300+
hasEitherOperand(ignoringImpCasts(canResolveToExpr(equalsNode(Exp)))),
301+
isTypeDependent()),
302302
// Within class templates and member functions the member expression might
303303
// not be resolved. In that case, the `callExpr` is considered to be a
304304
// modification.

clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,22 @@ TEST(ExprMutationAnalyzerTest, UnresolvedOperator) {
343343
EXPECT_TRUE(isMutated(Results, AST.get()));
344344
}
345345

346+
TEST(ExprMutationAnalyzerTest, DependentOperatorWithNonDependentOperand) {
347+
// gh57297
348+
// The expression to check may not be the dependent operand in a dependent
349+
// operator.
350+
351+
// Explicitly not declaring a (templated) stream operator
352+
// so the `<<` is a `binaryOperator` with a dependent type.
353+
const auto AST = buildASTFromCodeWithArgs(
354+
"struct Stream { };"
355+
"template <typename T> void f() { T t; Stream x; x << t; }",
356+
{"-fno-delayed-template-parsing"});
357+
const auto Results =
358+
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
359+
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x << t"));
360+
}
361+
346362
// Section: expression as call argument
347363

348364
TEST(ExprMutationAnalyzerTest, ByValueArgument) {

0 commit comments

Comments
 (0)