[clang-tools-extra] 37b6ba9 - [clang-tidy] Handle expr with side-effects in readability-static-accessed-through-instance (#90736)
via cfe-commits
cfe-commits at lists.llvm.org
Wed May 8 11:11:14 PDT 2024
Author: Piotr Zegar
Date: 2024-05-08T20:11:10+02:00
New Revision: 37b6ba96dea54e7af3772d78c90bfb3fd61140f6
URL: https://github.com/llvm/llvm-project/commit/37b6ba96dea54e7af3772d78c90bfb3fd61140f6
DIFF: https://github.com/llvm/llvm-project/commit/37b6ba96dea54e7af3772d78c90bfb3fd61140f6.diff
LOG: [clang-tidy] Handle expr with side-effects in readability-static-accessed-through-instance (#90736)
Improved readability-static-accessed-through-instance check to
support expressions with side-effects.
Originally calls to overloaded operator were
ignored by check, in fear of possible side-effects.
This change remove that restriction, and enables
fix-its for expressions with side-effect via
--fix-notes.
Closes #75163
Added:
Modified:
clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/readability/static-accessed-through-instance.rst
clang-tools-extra/test/clang-tidy/checkers/readability/static-accessed-through-instance.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp b/clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
index 65356cc3929c5..08adc7134cfea 100644
--- a/clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
@@ -59,10 +59,6 @@ void StaticAccessedThroughInstanceCheck::check(
const Expr *BaseExpr = MemberExpression->getBase();
- // Do not warn for overloaded -> operators.
- if (isa<CXXOperatorCallExpr>(BaseExpr))
- return;
-
const QualType BaseType =
BaseExpr->getType()->isPointerType()
? BaseExpr->getType()->getPointeeType().getUnqualifiedType()
@@ -89,17 +85,30 @@ void StaticAccessedThroughInstanceCheck::check(
return;
SourceLocation MemberExprStartLoc = MemberExpression->getBeginLoc();
- auto Diag =
- diag(MemberExprStartLoc, "static member accessed through instance");
-
- if (BaseExpr->HasSideEffects(*AstContext) ||
- getNameSpecifierNestingLevel(BaseType) > NameSpecifierNestingThreshold)
- return;
+ auto CreateFix = [&] {
+ return FixItHint::CreateReplacement(
+ CharSourceRange::getCharRange(MemberExprStartLoc,
+ MemberExpression->getMemberLoc()),
+ BaseTypeName + "::");
+ };
+
+ {
+ auto Diag =
+ diag(MemberExprStartLoc, "static member accessed through instance");
+
+ if (getNameSpecifierNestingLevel(BaseType) > NameSpecifierNestingThreshold)
+ return;
+
+ if (!BaseExpr->HasSideEffects(*AstContext,
+ /* IncludePossibleEffects =*/true)) {
+ Diag << CreateFix();
+ return;
+ }
+ }
- Diag << FixItHint::CreateReplacement(
- CharSourceRange::getCharRange(MemberExprStartLoc,
- MemberExpression->getMemberLoc()),
- BaseTypeName + "::");
+ diag(MemberExprStartLoc, "member base expression may carry some side effects",
+ DiagnosticIDs::Level::Note)
+ << BaseExpr->getSourceRange() << CreateFix();
}
} // namespace clang::tidy::readability
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index c4c9df27ee1ed..5b7ea42c63d65 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -352,6 +352,11 @@ Changes in existing checks
<clang-tidy/checks/readability/redundant-inline-specifier>` check to properly
emit warnings for static data member with an in-class initializer.
+- Improved :doc:`readability-static-accessed-through-instance
+ <clang-tidy/checks/readability/static-accessed-through-instance>` check to
+ support calls to overloaded operators as base expression and provide fixes to
+ expressions with side-effects.
+
- Improved :doc:`readability-static-definition-in-anonymous-namespace
<clang-tidy/checks/readability/static-definition-in-anonymous-namespace>`
check by resolving fix-it overlaps in template code by disregarding implicit
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/static-accessed-through-instance.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/static-accessed-through-instance.rst
index 23d12f4183664..ffb3738bf72c9 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/readability/static-accessed-through-instance.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability/static-accessed-through-instance.rst
@@ -35,3 +35,6 @@ is changed to:
C::E1;
C::E2;
+The `--fix` commandline option provides default support for safe fixes, whereas
+`--fix-notes` enables fixes that may replace expressions with side effects,
+potentially altering the program's behavior.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/static-accessed-through-instance.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/static-accessed-through-instance.cpp
index 81c1cecf607f6..202fe9be6d00c 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/static-accessed-through-instance.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/static-accessed-through-instance.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s readability-static-accessed-through-instance %t -- -- -isystem %S/Inputs/static-accessed-through-instance
+// RUN: %check_clang_tidy %s readability-static-accessed-through-instance %t -- --fix-notes -- -isystem %S/Inputs/static-accessed-through-instance
#include <__clang_cuda_builtin_vars.h>
enum OutEnum {
@@ -47,7 +47,8 @@ C &f(int, int, int, int);
void g() {
f(1, 2, 3, 4).x;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member accessed through instance [readability-static-accessed-through-instance]
- // CHECK-FIXES: {{^}} f(1, 2, 3, 4).x;{{$}}
+ // CHECK-MESSAGES: :[[@LINE-2]]:3: note: member base expression may carry some side effects
+ // CHECK-FIXES: {{^}} C::x;{{$}}
}
int i(int &);
@@ -59,12 +60,14 @@ int k(bool);
void f(C c) {
j(i(h().x));
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: static member
- // CHECK-FIXES: {{^}} j(i(h().x));{{$}}
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: note: member base expression may carry some side effects
+ // CHECK-FIXES: {{^}} j(i(C::x));{{$}}
// The execution of h() depends on the return value of a().
j(k(a() && h().x));
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: static member
- // CHECK-FIXES: {{^}} j(k(a() && h().x));{{$}}
+ // CHECK-MESSAGES: :[[@LINE-2]]:14: note: member base expression may carry some side effects
+ // CHECK-FIXES: {{^}} j(k(a() && C::x));{{$}}
if ([c]() {
c.ns();
@@ -72,7 +75,8 @@ void f(C c) {
}().x == 15)
;
// CHECK-MESSAGES: :[[@LINE-5]]:7: warning: static member
- // CHECK-FIXES: {{^}} if ([c]() {{{$}}
+ // CHECK-MESSAGES: :[[@LINE-6]]:7: note: member base expression may carry some side effects
+ // CHECK-FIXES: {{^}} if (C::x == 15){{$}}
}
// Nested specifiers
@@ -261,8 +265,11 @@ struct Qptr {
};
int func(Qptr qp) {
- qp->y = 10; // OK, the overloaded operator might have side-effects.
- qp->K = 10; //
+ qp->y = 10;
+ qp->K = 10;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member accessed through instance [readability-static-accessed-through-instance]
+ // CHECK-MESSAGES: :[[@LINE-2]]:3: note: member base expression may carry some side effects
+ // CHECK-FIXES: {{^}} Q::K = 10;
}
namespace {
@@ -380,3 +387,20 @@ namespace PR51861 {
// CHECK-FIXES: {{^}} PR51861::Foo::getBar();{{$}}
}
}
+
+namespace PR75163 {
+ struct Static {
+ static void call();
+ };
+
+ struct Ptr {
+ Static* operator->();
+ };
+
+ void test(Ptr& ptr) {
+ ptr->call();
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: static member accessed through instance [readability-static-accessed-through-instance]
+ // CHECK-MESSAGES: :[[@LINE-2]]:5: note: member base expression may carry some side effects
+ // CHECK-FIXES: {{^}} PR75163::Static::call();{{$}}
+ }
+}
More information about the cfe-commits
mailing list