[clang-tools-extra] [clang-tidy] Handle expr with side-effects in readability-static-accessed-through-instance (PR #90736)
https://github.com/PiotrZSL closed https://github.com/llvm/llvm-project/pull/90736 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Handle expr with side-effects in readability-static-accessed-through-instance (PR #90736)
@@ -380,3 +387,20 @@ namespace PR51861 {
// CHECK-FIXES: {{^}}PR51861::Foo::getBar();{{$}}
}
}
+
+namespace PR75163 {
PiotrZSL wrote:
PR = Problem Report (in this context)
https://github.com/llvm/llvm-project/pull/90736
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Handle expr with side-effects in readability-static-accessed-through-instance (PR #90736)
@@ -380,3 +387,20 @@ namespace PR51861 {
// CHECK-FIXES: {{^}}PR51861::Foo::getBar();{{$}}
}
}
+
+namespace PR75163 {
5chmidti wrote:
Nit: `GH` instead of `PR`?
https://github.com/llvm/llvm-project/pull/90736
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Handle expr with side-effects in readability-static-accessed-through-instance (PR #90736)
https://github.com/5chmidti approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/90736 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Handle expr with side-effects in readability-static-accessed-through-instance (PR #90736)
https://github.com/5chmidti edited https://github.com/llvm/llvm-project/pull/90736 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Handle expr with side-effects in readability-static-accessed-through-instance (PR #90736)
llvmbot wrote:
@llvm/pr-subscribers-clang-tidy
Author: Piotr Zegar (PiotrZSL)
Changes
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
---
Full diff: https://github.com/llvm/llvm-project/pull/90736.diff
4 Files Affected:
- (modified)
clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
(+23-14)
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5)
- (modified)
clang-tools-extra/docs/clang-tidy/checks/readability/static-accessed-through-instance.rst
(+3)
- (modified)
clang-tools-extra/test/clang-tidy/checkers/readability/static-accessed-through-instance.cpp
(+31-7)
``diff
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(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
` check to properly
emit warnings for static data member with an in-class initializer.
+- Improved :doc:`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
`
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
[clang-tools-extra] [clang-tidy] Handle expr with side-effects in readability-static-accessed-through-instance (PR #90736)
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
Date: Wed, 1 May 2024 14:44:08 +
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(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
` check to properly
emit warnings for static data member with an in-class initializer.
+- Improved :doc:`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
`
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
