Skip to content

Commit 89e2d58

Browse files
authored
[clang-tidy] Fix false positives about references in misc-const-correctness (#160971)
It's not legal to cast const pointer type to it's non-const reference type implicitly, and will cause compile error. And for explicit cast, it's legal but the pointer is mutable through this reference.
1 parent cac5bfa commit 89e2d58

File tree

4 files changed

+55
-12
lines changed

4 files changed

+55
-12
lines changed

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,10 @@ Changes in existing checks
318318

319319
- ``for`` loops are supported.
320320

321+
- Improved :doc:`misc-const-correctness
322+
<clang-tidy/checks/misc/const-correctness>` check to avoid false
323+
positives when pointers is tranferred to non-const references.
324+
321325
- Improved :doc:`misc-header-include-cycle
322326
<clang-tidy/checks/misc/header-include-cycle>` check performance.
323327

clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-pointers.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,3 +48,18 @@ void ignore_const_alias() {
4848
p_local0 = &a[1];
4949
}
5050

51+
void takeNonConstRef(int *&r);
52+
53+
void ignoreNonConstRefOps() {
54+
// init with non-const ref
55+
int* p0 {nullptr};
56+
int*& r1 = p0;
57+
58+
// non-const ref param
59+
int* p1 {nullptr};
60+
takeNonConstRef(p1);
61+
62+
// cast
63+
int* p2 {nullptr};
64+
int*& r2 = (int*&)p2;
65+
}

clang/lib/Analysis/ExprMutationAnalyzer.cpp

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -755,22 +755,23 @@ ExprMutationAnalyzer::Analyzer::findPointeeMemberMutation(const Expr *Exp) {
755755

756756
const Stmt *
757757
ExprMutationAnalyzer::Analyzer::findPointeeToNonConst(const Expr *Exp) {
758-
const auto NonConstPointerOrDependentType =
759-
type(anyOf(nonConstPointerType(), isDependentType()));
758+
const auto NonConstPointerOrNonConstRefOrDependentType = type(
759+
anyOf(nonConstPointerType(), nonConstReferenceType(), isDependentType()));
760760

761761
// assign
762762
const auto InitToNonConst =
763-
varDecl(hasType(NonConstPointerOrDependentType),
763+
varDecl(hasType(NonConstPointerOrNonConstRefOrDependentType),
764764
hasInitializer(expr(canResolveToExprPointee(Exp)).bind("stmt")));
765-
const auto AssignToNonConst =
766-
binaryOperation(hasOperatorName("="),
767-
hasLHS(expr(hasType(NonConstPointerOrDependentType))),
768-
hasRHS(canResolveToExprPointee(Exp)));
765+
const auto AssignToNonConst = binaryOperation(
766+
hasOperatorName("="),
767+
hasLHS(expr(hasType(NonConstPointerOrNonConstRefOrDependentType))),
768+
hasRHS(canResolveToExprPointee(Exp)));
769769
// arguments like
770770
const auto ArgOfInstantiationDependent = allOf(
771771
hasAnyArgument(canResolveToExprPointee(Exp)), isInstantiationDependent());
772-
const auto ArgOfNonConstParameter = forEachArgumentWithParamType(
773-
canResolveToExprPointee(Exp), NonConstPointerOrDependentType);
772+
const auto ArgOfNonConstParameter =
773+
forEachArgumentWithParamType(canResolveToExprPointee(Exp),
774+
NonConstPointerOrNonConstRefOrDependentType);
774775
const auto CallLikeMatcher =
775776
anyOf(ArgOfNonConstParameter, ArgOfInstantiationDependent);
776777
const auto PassAsNonConstArg =
@@ -779,9 +780,9 @@ ExprMutationAnalyzer::Analyzer::findPointeeToNonConst(const Expr *Exp) {
779780
parenListExpr(has(canResolveToExprPointee(Exp))),
780781
initListExpr(hasAnyInit(canResolveToExprPointee(Exp)))));
781782
// cast
782-
const auto CastToNonConst =
783-
explicitCastExpr(hasSourceExpression(canResolveToExprPointee(Exp)),
784-
hasDestinationType(NonConstPointerOrDependentType));
783+
const auto CastToNonConst = explicitCastExpr(
784+
hasSourceExpression(canResolveToExprPointee(Exp)),
785+
hasDestinationType(NonConstPointerOrNonConstRefOrDependentType));
785786

786787
// capture
787788
// FIXME: false positive if the pointee does not change in lambda

clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1749,6 +1749,13 @@ TEST(ExprMutationAnalyzerTest, PointeeMutatedByInitToNonConst) {
17491749
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
17501750
EXPECT_TRUE(isPointeeMutated(Results, AST.get()));
17511751
}
1752+
{
1753+
const std::string Code = "void f() { int* x = nullptr; int*& b = x; }";
1754+
auto AST = buildASTFromCodeWithArgs(Code, {});
1755+
auto Results =
1756+
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
1757+
EXPECT_TRUE(isPointeeMutated(Results, AST.get()));
1758+
}
17521759
}
17531760

17541761
TEST(ExprMutationAnalyzerTest, PointeeMutatedByAssignToNonConst) {
@@ -1786,6 +1793,14 @@ TEST(ExprMutationAnalyzerTest, PointeeMutatedByPassAsArgument) {
17861793
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
17871794
EXPECT_TRUE(isPointeeMutated(Results, AST.get()));
17881795
}
1796+
{
1797+
const std::string Code =
1798+
"void b(int *&); void f() { int* x = nullptr; b(x); }";
1799+
auto AST = buildASTFromCodeWithArgs(Code, {});
1800+
auto Results =
1801+
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
1802+
EXPECT_TRUE(isPointeeMutated(Results, AST.get()));
1803+
}
17891804
}
17901805

17911806
TEST(ExprMutationAnalyzerTest, PointeeMutatedByPassAsArgumentInConstruct) {
@@ -1884,6 +1899,14 @@ TEST(ExprMutationAnalyzerTest, PointeeMutatedByExplicitCastToNonConst) {
18841899
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
18851900
EXPECT_TRUE(isPointeeMutated(Results, AST.get()));
18861901
}
1902+
{
1903+
const std::string Code =
1904+
"void f() { int* x = nullptr; static_cast<int*&>(x); }";
1905+
auto AST = buildASTFromCodeWithArgs(Code, {"-Wno-everything"});
1906+
auto Results =
1907+
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
1908+
EXPECT_TRUE(isPointeeMutated(Results, AST.get()));
1909+
}
18871910
}
18881911

18891912
TEST(ExprMutationAnalyzerTest, PointeeMutatedByConstCastToNonConst) {

0 commit comments

Comments
 (0)