[clang-tidy] Fix false positives in readability-redundant-inline-specifier
#135391
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…cifier Signed-off-by: Björn Svensson <bjorn.a.svensson@est.tech>
…ifier Only warn on explicitly defaulted functions which are inlined by default, i.e. dont warn on out-of-line explicitly defaulted functions. Signed-off-by: Björn Svensson <bjorn.a.svensson@est.tech>
|
@llvm/pr-subscribers-clang-tools-extra Author: Björn Svensson (bjosv) ChangesThe out-of-line explicitly-defaulted definition is not the first declaration, so it is not implicitly inline. Alt. reference: Fixes #130745 Full diff: https://github.com/llvm/llvm-project/pull/135391.diff 3 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp
index 1693e5c5e9cd4..33effb3dab977 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp
@@ -72,11 +72,13 @@ static SourceLocation getInlineTokenLocation(SourceRange RangeLocation,
}
void RedundantInlineSpecifierCheck::registerMatchers(MatchFinder *Finder) {
+ const auto IsPartOfRecordDecl = hasAncestor(recordDecl());
Finder->addMatcher(
functionDecl(isInlineSpecified(),
- anyOf(isConstexpr(), isDeleted(), isDefaulted(),
+ anyOf(isConstexpr(), isDeleted(),
+ allOf(isDefaulted(), IsPartOfRecordDecl),
isInternalLinkage(StrictMode),
- allOf(isDefinition(), hasAncestor(recordDecl()))))
+ allOf(isDefinition(), IsPartOfRecordDecl)))
.bind("fun_decl"),
this);
@@ -88,7 +90,6 @@ void RedundantInlineSpecifierCheck::registerMatchers(MatchFinder *Finder) {
this);
if (getLangOpts().CPlusPlus17) {
- const auto IsPartOfRecordDecl = hasAncestor(recordDecl());
Finder->addMatcher(
varDecl(
isInlineSpecified(),
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 761c1d3a80359..8fed96ceef647 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -204,6 +204,10 @@ Changes in existing checks
tolerating fix-it breaking compilation when functions is used as pointers
to avoid matching usage of functions within the current compilation unit.
+- Improved :doc:`readability-redundant-inline-specifier
+ <clang-tidy/checks/readability/redundant-inline-specifier>` check by fixing
+ false positives on out-of-line explicitly defaulted functions.
+
Removed checks
^^^^^^^^^^^^^^
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp
index 14f9e88f7e721..882ce640b13c1 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp
@@ -149,3 +149,13 @@ class A
// CHECK-FIXES-STRICT: static float test4;
};
}
+
+namespace ns {
+class B
+{
+public:
+ ~B();
+};
+
+inline B::~B() = default;
+}
|
|
@llvm/pr-subscribers-clang-tidy Author: Björn Svensson (bjosv) ChangesThe out-of-line explicitly-defaulted definition is not the first declaration, so it is not implicitly inline. Alt. reference: Fixes #130745 Full diff: https://github.com/llvm/llvm-project/pull/135391.diff 3 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp
index 1693e5c5e9cd4..33effb3dab977 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp
@@ -72,11 +72,13 @@ static SourceLocation getInlineTokenLocation(SourceRange RangeLocation,
}
void RedundantInlineSpecifierCheck::registerMatchers(MatchFinder *Finder) {
+ const auto IsPartOfRecordDecl = hasAncestor(recordDecl());
Finder->addMatcher(
functionDecl(isInlineSpecified(),
- anyOf(isConstexpr(), isDeleted(), isDefaulted(),
+ anyOf(isConstexpr(), isDeleted(),
+ allOf(isDefaulted(), IsPartOfRecordDecl),
isInternalLinkage(StrictMode),
- allOf(isDefinition(), hasAncestor(recordDecl()))))
+ allOf(isDefinition(), IsPartOfRecordDecl)))
.bind("fun_decl"),
this);
@@ -88,7 +90,6 @@ void RedundantInlineSpecifierCheck::registerMatchers(MatchFinder *Finder) {
this);
if (getLangOpts().CPlusPlus17) {
- const auto IsPartOfRecordDecl = hasAncestor(recordDecl());
Finder->addMatcher(
varDecl(
isInlineSpecified(),
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 761c1d3a80359..8fed96ceef647 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -204,6 +204,10 @@ Changes in existing checks
tolerating fix-it breaking compilation when functions is used as pointers
to avoid matching usage of functions within the current compilation unit.
+- Improved :doc:`readability-redundant-inline-specifier
+ <clang-tidy/checks/readability/redundant-inline-specifier>` check by fixing
+ false positives on out-of-line explicitly defaulted functions.
+
Removed checks
^^^^^^^^^^^^^^
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp
index 14f9e88f7e721..882ce640b13c1 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp
@@ -149,3 +149,13 @@ class A
// CHECK-FIXES-STRICT: static float test4;
};
}
+
+namespace ns {
+class B
+{
+public:
+ ~B();
+};
+
+inline B::~B() = default;
+}
|
PiotrZSL
approved these changes
Apr 11, 2025
| @@ -72,11 +72,13 @@ static SourceLocation getInlineTokenLocation(SourceRange RangeLocation, | |||
| } | |||
|
|
|||
| void RedundantInlineSpecifierCheck::registerMatchers(MatchFinder *Finder) { | |||
| const auto IsPartOfRecordDecl = hasAncestor(recordDecl()); | |||
There was a problem hiding this comment.
Note: I wonder, maybe hasDeclContext should be used here instead of hasAncestor or just match cxxMethodDecl directly instead of functionDecl.
HerrCai0907
approved these changes
Apr 28, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The out-of-line explicitly-defaulted definition is not the first declaration, so it is not implicitly inline.
Alt. reference:
9.5.2 (3) Explicitly-defaulted functions in N4950.
or https://timsong-cpp.github.io/cppwp/n4861/dcl.fct.def.default#3
Fixes #130745