https://github.com/PiotrZSL created
https://github.com/llvm/llvm-project/pull/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
>From 026b6ea48a697282ce8d18b2efe48499016276cc Mon Sep 17 00:00:00 2001
From: Piotr Zegar <m...@piotrzegar.pl>
Date: Wed, 1 May 2024 14:44:08 +0000
Subject: [PATCH] [clang-tidy] Handle expr with side-effects in
readability-static-accessed-through-instance
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
---
.../StaticAccessedThroughInstanceCheck.cpp | 37 +++++++++++-------
clang-tools-extra/docs/ReleaseNotes.rst | 5 +++
.../static-accessed-through-instance.rst | 3 ++
.../static-accessed-through-instance.cpp | 38 +++++++++++++++----
4 files changed, 62 insertions(+), 21 deletions(-)
diff --git
a/clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
b/clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
index 65356cc3929c54..08adc7134cfea2 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 3038d2b125f20d..1f3e2c6953c2a4 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -331,6 +331,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 23d12f41836640..ffb3738bf72c92 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 81c1cecf607f66..202fe9be6d00c5 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();{{$}}
+ }
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
