https://github.com/vbvictor updated https://github.com/llvm/llvm-project/pull/206515
>From 7ca65d9d30ef708f61e8c5ee8daf7d1940fcc783 Mon Sep 17 00:00:00 2001 From: Victor Baranov <[email protected]> Date: Thu, 2 Jul 2026 22:54:27 +0300 Subject: [PATCH] [clang-tidy] Fix FP in misc-const-correctness --- clang-tools-extra/docs/ReleaseNotes.rst | 3 + .../const-correctness-pointer-as-pointers.cpp | 32 +++ clang/lib/Analysis/ExprMutationAnalyzer.cpp | 39 ++-- .../Analysis/ExprMutationAnalyzerTest.cpp | 216 ++++++++++++++++++ 4 files changed, 267 insertions(+), 23 deletions(-) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 2a980cc089a65..dc10bc59804ea 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -571,6 +571,9 @@ Changes in existing checks - Fixed false positives when pointers were later passed or bound through ``const``-qualified pointer references. + - Fixed false positive where a pointer returned as a non-const ``void *`` + was incorrectly diagnosed as allowing its pointee to be made ``const``. + - Improved :doc:`misc-multiple-inheritance <clang-tidy/checks/misc/multiple-inheritance>` by avoiding false positives when virtual inheritance causes concrete bases to be counted more than once. diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-pointers.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-pointers.cpp index 41df6c44f5bef..f6bfb98c548c3 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-pointers.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-pointers.cpp @@ -56,6 +56,38 @@ void *return_non_const() { return a; } +void *return_as_void_pointer() { + int i = 0; + int *p_local0 = &i; + // CHECK-NOT: warning + return p_local0; +} + +const void *return_as_const_void_pointer() { + int i = 0; + int *p_local0 = &i; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: pointee of variable 'p_local0' of type 'int *' can be declared 'const' + // CHECK-FIXES: int const*p_local0 = &i; + return p_local0; +} + +void take_void_pointer(void *); +void pass_as_void_pointer() { + int i = 0; + int *p_local0 = &i; + // CHECK-NOT: warning + take_void_pointer(p_local0); +} + +void take_const_void_pointer(const void *); +void pass_as_const_void_pointer() { + int i = 0; + int *p_local0 = &i; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: pointee of variable 'p_local0' of type 'int *' can be declared 'const' + // CHECK-FIXES: int const*p_local0 = &i; + take_const_void_pointer(p_local0); +} + void function_pointer_basic() { void (*const fp)() = nullptr; fp(); diff --git a/clang/lib/Analysis/ExprMutationAnalyzer.cpp b/clang/lib/Analysis/ExprMutationAnalyzer.cpp index 3709fd0af3486..59fad2a41675c 100644 --- a/clang/lib/Analysis/ExprMutationAnalyzer.cpp +++ b/clang/lib/Analysis/ExprMutationAnalyzer.cpp @@ -145,21 +145,9 @@ class ExprPointeeResolve { // explicit cast will be checked in `findPointeeToNonConst` const CastKind kind = ICE->getCastKind(); if (kind == CK_LValueToRValue || kind == CK_DerivedToBase || - kind == CK_UncheckedDerivedToBase) + kind == CK_UncheckedDerivedToBase || kind == CK_NoOp || + kind == CK_BitCast) return resolveExpr(ICE->getSubExpr()); - if (kind == CK_NoOp) { - // Binding `T *` to `T *const &` only adds top-level qualifiers to the - // pointer object, so this `CK_NoOp` still refers to the same pointer. - const auto GetLocallyUnqualifiedCanonicalType = [](QualType Type) { - return Type.getLocalUnqualifiedType().getCanonicalType(); - }; - const QualType CastType = - GetLocallyUnqualifiedCanonicalType(ICE->getType()); - const QualType SubExprType = - GetLocallyUnqualifiedCanonicalType(ICE->getSubExpr()->getType()); - if (CastType == SubExprType) - return resolveExpr(ICE->getSubExpr()); - } return false; } @@ -809,13 +797,17 @@ ExprMutationAnalyzer::Analyzer::findPointeeToNonConst(const Expr *Exp) { NonConstPointerOrNonConstRefOrDependentType); const auto CallLikeMatcher = anyOf(ArgOfNonConstParameter, ArgOfInstantiationDependent); - const auto PassAsNonConstArg = - expr(anyOf(cxxUnresolvedConstructExpr(ArgOfInstantiationDependent), - cxxNewExpr(hasAnyPlacementArg( - ignoringParenImpCasts(canResolveToExprPointee(Exp)))), - cxxConstructExpr(CallLikeMatcher), callExpr(CallLikeMatcher), - parenListExpr(has(canResolveToExprPointee(Exp))), - initListExpr(hasAnyInit(canResolveToExprPointee(Exp))))); + const auto PassAsNonConstArg = expr( + anyOf(cxxUnresolvedConstructExpr(ArgOfInstantiationDependent), + cxxNewExpr(hasAnyPlacementArg( + ignoringParenImpCasts(canResolveToExprPointee(Exp)))), + cxxConstructExpr(CallLikeMatcher), callExpr(CallLikeMatcher), + parenListExpr(has( + expr(canResolveToExprPointee(Exp), + hasType(NonConstPointerOrNonConstRefOrDependentType)))), + initListExpr(hasAnyInit( + expr(canResolveToExprPointee(Exp), + hasType(NonConstPointerOrNonConstRefOrDependentType)))))); // cast const auto CastToNonConst = explicitCastExpr( hasSourceExpression(canResolveToExprPointee(Exp)), @@ -825,8 +817,9 @@ ExprMutationAnalyzer::Analyzer::findPointeeToNonConst(const Expr *Exp) { // FIXME: false positive if the pointee does not change in lambda const auto CaptureNoConst = lambdaExpr(hasCaptureInit(Exp)); - const auto ReturnNoConst = - returnStmt(hasReturnValue(canResolveToExprPointee(Exp))); + const auto ReturnNoConst = returnStmt( + hasReturnValue(canResolveToExprPointee(Exp)), + forFunction(returns(NonConstPointerOrNonConstRefOrDependentType))); const auto Matches = match( stmt(anyOf(forEachDescendant( diff --git a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp index 1545310431fd8..4ccd1f554eefc 100644 --- a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp +++ b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp @@ -1788,6 +1788,21 @@ TEST(ExprMutationAnalyzerTest, PointeeMutatedByInitToNonConst) { match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); EXPECT_FALSE(isPointeeMutated(Results, AST.get())); } + { + const std::string Code = "void f() { int* x = nullptr; void* b = x; }"; + auto AST = buildASTFromCodeWithArgs(Code, {}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_TRUE(isPointeeMutated(Results, AST.get())); + } + { + const std::string Code = + "void f() { int* x = nullptr; const void* b = x; }"; + auto AST = buildASTFromCodeWithArgs(Code, {}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isPointeeMutated(Results, AST.get())); + } } TEST(ExprMutationAnalyzerTest, PointeeMutatedByAssignToNonConst) { @@ -1806,6 +1821,21 @@ TEST(ExprMutationAnalyzerTest, PointeeMutatedByAssignToNonConst) { match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); EXPECT_TRUE(isPointeeMutated(Results, AST.get())); } + { + const std::string Code = "void f() { int* x = nullptr; void* b; b = x; }"; + auto AST = buildASTFromCodeWithArgs(Code, {}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_TRUE(isPointeeMutated(Results, AST.get())); + } + { + const std::string Code = + "void f() { int* x = nullptr; const void* b; b = x; }"; + auto AST = buildASTFromCodeWithArgs(Code, {}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isPointeeMutated(Results, AST.get())); + } } TEST(ExprMutationAnalyzerTest, PointeeMutatedByPassAsArgument) { @@ -1874,6 +1904,22 @@ TEST(ExprMutationAnalyzerTest, PointeeMutatedByPassAsArgument) { match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); EXPECT_TRUE(isPointeeMutated(Results, AST.get())); } + { + const std::string Code = + "void b(void*); void f() { int* x = nullptr; b(x); }"; + auto AST = buildASTFromCodeWithArgs(Code, {}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_TRUE(isPointeeMutated(Results, AST.get())); + } + { + const std::string Code = + "void b(const void*); void f() { int* x = nullptr; b(x); }"; + auto AST = buildASTFromCodeWithArgs(Code, {}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isPointeeMutated(Results, AST.get())); + } { const std::string Code = "namespace std { typedef decltype(sizeof(int)) size_t; }" @@ -2157,6 +2203,143 @@ TEST(ExprMutationAnalyzerTest, PointeeMutatedByReturn) { match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); EXPECT_FALSE(isPointeeMutated(Results, AST.get())); } + { + const std::string Code = R"( + void * f() { + int *const x = nullptr; + return x; + })"; + auto AST = buildASTFromCodeWithArgs(Code, {"-Wno-everything"}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_TRUE(isPointeeMutated(Results, AST.get())); + } + { + const std::string Code = R"( + const void * f() { + int *const x = nullptr; + return x; + })"; + auto AST = buildASTFromCodeWithArgs(Code, {"-Wno-everything"}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isPointeeMutated(Results, AST.get())); + } + { + const std::string Code = R"( + int *& f() { + int *x = nullptr; + return x; + })"; + auto AST = buildASTFromCodeWithArgs(Code, {"-Wno-everything"}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_TRUE(isPointeeMutated(Results, AST.get())); + } + { + const std::string Code = R"( + int *const & f() { + int *x = nullptr; + return x; + })"; + auto AST = buildASTFromCodeWithArgs(Code, {"-Wno-everything"}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_TRUE(isPointeeMutated(Results, AST.get())); + } + { + const std::string Code = R"( + struct B {}; struct D : B {}; + B *f() { + D *x = nullptr; + return x; + })"; + auto AST = buildASTFromCodeWithArgs(Code, {"-Wno-everything"}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_TRUE(isPointeeMutated(Results, AST.get())); + } + { + const std::string Code = R"( + struct B {}; struct D : B {}; + const B *f() { + D *x = nullptr; + return x; + })"; + auto AST = buildASTFromCodeWithArgs(Code, {"-Wno-everything"}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isPointeeMutated(Results, AST.get())); + } + { + const std::string Code = R"( + void *f() { + int **x = nullptr; + return x; + })"; + auto AST = buildASTFromCodeWithArgs(Code, {"-Wno-everything"}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_TRUE(isPointeeMutated(Results, AST.get())); + } + { + const std::string Code = R"( + const void *f() { + int **x = nullptr; + return x; + })"; + auto AST = buildASTFromCodeWithArgs(Code, {"-Wno-everything"}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isPointeeMutated(Results, AST.get())); + } + { + const std::string Code = R"( + void *f(bool b) { + int *x = nullptr; + int *y = nullptr; + return b ? x : y; + })"; + auto AST = buildASTFromCodeWithArgs(Code, {"-Wno-everything"}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_TRUE(isPointeeMutated(Results, AST.get())); + } + { + const std::string Code = R"( + void *f() { + int z = 0; + int *x = nullptr; + return (z, x); + })"; + auto AST = buildASTFromCodeWithArgs(Code, {"-Wno-everything"}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_TRUE(isPointeeMutated(Results, AST.get())); + } + { + const std::string Code = R"( + void *f() { + int *x = nullptr; + return (x); + })"; + auto AST = buildASTFromCodeWithArgs(Code, {"-Wno-everything"}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_TRUE(isPointeeMutated(Results, AST.get())); + } + { + const std::string Code = R"( + const void *f(bool b) { + int *x = nullptr; + int *y = nullptr; + return b ? x : y; + })"; + auto AST = buildASTFromCodeWithArgs(Code, {"-Wno-everything"}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isPointeeMutated(Results, AST.get())); + } } TEST(ExprMutationAnalyzerTest, PointeeMutatedByPointerToMemberOperator) { @@ -2191,4 +2374,37 @@ TEST(ExprMutationAnalyzerTest, PointeeMutatedByPassAsPointerToPointer) { } } +TEST(ExprMutationAnalyzerTest, PointeeMutatedByInitListElement) { + { + const std::string Code = "void f() { int* x = nullptr; void* a[] = {x}; }"; + auto AST = buildASTFromCodeWithArgs(Code, {"-Wno-everything"}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_TRUE(isPointeeMutated(Results, AST.get())); + } + { + const std::string Code = "void f() { int* x = nullptr; int* a[] = {x}; }"; + auto AST = buildASTFromCodeWithArgs(Code, {"-Wno-everything"}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_TRUE(isPointeeMutated(Results, AST.get())); + } + { + const std::string Code = + "void f() { int* x = nullptr; const void* a[] = {x}; }"; + auto AST = buildASTFromCodeWithArgs(Code, {"-Wno-everything"}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isPointeeMutated(Results, AST.get())); + } + { + const std::string Code = + "void f() { int* x = nullptr; const int* a[] = {x}; }"; + auto AST = buildASTFromCodeWithArgs(Code, {"-Wno-everything"}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isPointeeMutated(Results, AST.get())); + } +} + } // namespace clang _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
