[PATCH] D79674: [clang-tidy] Better support for Override function in RenamerClangTidy based checks
This revision was automatically updated to reflect the committed changes.
Closed by commit rG866dc0978449: [clang-tidy] Better support for Override
function in RenamerClangTidy based… (authored by njames93).
Changed prior to commit:
https://reviews.llvm.org/D79674?vs=298616=299043#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79674/new/
https://reviews.llvm.org/D79674
Files:
clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
@@ -266,14 +266,32 @@
virtual ~AOverridden() = default;
virtual void BadBaseMethod() = 0;
// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for virtual method 'BadBaseMethod'
+ // CHECK-FIXES: {{^}} virtual void v_Bad_Base_Method() = 0;
};
class COverriding : public AOverridden {
public:
// Overriding a badly-named base isn't a new violation.
void BadBaseMethod() override {}
+ // CHECK-FIXES: {{^}} void v_Bad_Base_Method() override {}
+
+ void foo() {
+BadBaseMethod();
+// CHECK-FIXES: {{^}}v_Bad_Base_Method();
+this->BadBaseMethod();
+// CHECK-FIXES: {{^}}this->v_Bad_Base_Method();
+AOverridden::BadBaseMethod();
+// CHECK-FIXES: {{^}}AOverridden::v_Bad_Base_Method();
+COverriding::BadBaseMethod();
+// CHECK-FIXES: {{^}}COverriding::v_Bad_Base_Method();
+ }
};
+void VirtualCall(AOverridden _vItem) {
+ a_vItem.BadBaseMethod();
+ // CHECK-FIXES: {{^}} a_vItem.v_Bad_Base_Method();
+}
+
template
class CRTPBase {
public:
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -125,6 +125,9 @@
Added an option `GetConfigPerFile` to support including files which use
different naming styles.
+ Now renames overridden virtual methods if the method they override has a
+ style violation.
+
- Removed `google-runtime-references` check because the rule it checks does
not exist in the Google Style Guide anymore.
Index: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
@@ -135,6 +135,23 @@
this));
}
+/// Returns the function that \p Method is overridding. If There are none or
+/// multiple overrides it returns nullptr. If the overridden function itself is
+/// overridding then it will recurse up to find the first decl of the function.
+static const CXXMethodDecl *getOverrideMethod(const CXXMethodDecl *Method) {
+ if (Method->size_overridden_methods() != 1)
+return nullptr;
+ while (true) {
+Method = *Method->begin_overridden_methods();
+assert(Method && "Overridden method shouldn't be null");
+unsigned NumOverrides = Method->size_overridden_methods();
+if (NumOverrides == 0)
+ return Method;
+if (NumOverrides > 1)
+ return nullptr;
+ }
+}
+
void RenamerClangTidyCheck::addUsage(
const RenamerClangTidyCheck::NamingCheckId , SourceRange Range,
SourceManager *SourceMgr) {
@@ -172,6 +189,10 @@
void RenamerClangTidyCheck::addUsage(const NamedDecl *Decl, SourceRange Range,
SourceManager *SourceMgr) {
+ if (const auto *Method = dyn_cast(Decl)) {
+if (const CXXMethodDecl *Overridden = getOverrideMethod(Method))
+ Decl = Overridden;
+ }
Decl = cast(Decl->getCanonicalDecl());
return addUsage(RenamerClangTidyCheck::NamingCheckId(Decl->getLocation(),
Decl->getNameAsString()),
@@ -412,6 +433,14 @@
}
}
+// Fix overridden methods
+if (const auto *Method = Result.Nodes.getNodeAs("decl")) {
+ if (const CXXMethodDecl *Overridden = getOverrideMethod(Method)) {
+addUsage(Overridden, Method->getLocation());
+return; // Don't try to add the actual decl as a Failure.
+ }
+}
+
// Ignore ClassTemplateSpecializationDecl which are creating duplicate
// replacements with CXXRecordDecl.
if (isa(Decl))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79674: [clang-tidy] Better support for Override function in RenamerClangTidy based checks
njames93 updated this revision to Diff 298616.
njames93 added a comment.
Updated release notes to reflect change
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79674/new/
https://reviews.llvm.org/D79674
Files:
clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
@@ -266,14 +266,32 @@
virtual ~AOverridden() = default;
virtual void BadBaseMethod() = 0;
// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for virtual method 'BadBaseMethod'
+ // CHECK-FIXES: {{^}} virtual void v_Bad_Base_Method() = 0;
};
class COverriding : public AOverridden {
public:
// Overriding a badly-named base isn't a new violation.
void BadBaseMethod() override {}
+ // CHECK-FIXES: {{^}} void v_Bad_Base_Method() override {}
+
+ void foo() {
+BadBaseMethod();
+// CHECK-FIXES: {{^}}v_Bad_Base_Method();
+this->BadBaseMethod();
+// CHECK-FIXES: {{^}}this->v_Bad_Base_Method();
+AOverridden::BadBaseMethod();
+// CHECK-FIXES: {{^}}AOverridden::v_Bad_Base_Method();
+COverriding::BadBaseMethod();
+// CHECK-FIXES: {{^}}COverriding::v_Bad_Base_Method();
+ }
};
+void VirtualCall(AOverridden _vItem) {
+ a_vItem.BadBaseMethod();
+ // CHECK-FIXES: {{^}} a_vItem.v_Bad_Base_Method();
+}
+
template
class CRTPBase {
public:
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -125,6 +125,9 @@
Added an option `GetConfigPerFile` to support including files which use
different naming styles.
+ Now renames overridden virtual methods if the method they override has a
+ style violation.
+
- Removed `google-runtime-references` check because the rule it checks does
not exist in the Google Style Guide anymore.
Index: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
@@ -135,6 +135,26 @@
this));
}
+/// Returns the function that \p Method is overridding. If There are none or
+/// multiple overrides it returns nullptr. If the overridden function itself is
+/// overridding then it will recurse up to find the first decl of the function.
+static const CXXMethodDecl *getOverrideMethod(const CXXMethodDecl *Method) {
+ if (Method->size_overridden_methods() != 1)
+return nullptr; // no overrides
+ while (true) {
+const CXXMethodDecl *Next = *Method->begin_overridden_methods();
+assert(Next && "Overridden method shouldn't be null");
+if (Next->size_overridden_methods() == 0) {
+ return Next;
+}
+if (Next->size_overridden_methods() == 1) {
+ Method = Next;
+ continue;
+}
+return nullptr; // Multiple overrides
+ }
+}
+
void RenamerClangTidyCheck::addUsage(
const RenamerClangTidyCheck::NamingCheckId , SourceRange Range,
SourceManager *SourceMgr) {
@@ -173,6 +193,10 @@
void RenamerClangTidyCheck::addUsage(const NamedDecl *Decl, SourceRange Range,
SourceManager *SourceMgr) {
Decl = cast(Decl->getCanonicalDecl());
+ if (const auto *Method = dyn_cast(Decl)) {
+if (const CXXMethodDecl *Overridden = getOverrideMethod(Method))
+ Decl = Overridden;
+ }
return addUsage(RenamerClangTidyCheck::NamingCheckId(Decl->getLocation(),
Decl->getNameAsString()),
Range, SourceMgr);
@@ -412,6 +436,14 @@
}
}
+// Fix overridden methods
+if (const auto *Method = Result.Nodes.getNodeAs("decl")) {
+ if (const CXXMethodDecl *Overridden = getOverrideMethod(Method)) {
+addUsage(Overridden, Method->getLocation());
+return; // Don't try to add the actual decl as a Failure.
+ }
+}
+
// Ignore ClassTemplateSpecializationDecl which are creating duplicate
// replacements with CXXRecordDecl.
if (isa(Decl))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79674: [clang-tidy] Better support for Override function in RenamerClangTidy based checks
alexfh added a comment. Feel free to ping patches every week or so. It looks like in this case all the reviewers were swamped with something else at the time. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79674/new/ https://reviews.llvm.org/D79674 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79674: [clang-tidy] Better support for Override function in RenamerClangTidy based checks
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good! Thanks for the fix! IIUC, this is related to https://bugs.llvm.org/show_bug.cgi?id=34879? Makes sense to specify this in the patch description. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79674/new/ https://reviews.llvm.org/D79674 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79674: [clang-tidy] Better support for Override function in RenamerClangTidy based checks
njames93 created this revision.
njames93 added reviewers: aaron.ballman, gribozavr2, alexfh.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.
Methods that override virtual methods will now get renamed if the initial
virtual method has a name violation.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D79674
Files:
clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
Index:
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
@@ -267,14 +267,32 @@
virtual ~AOverridden() = default;
virtual void BadBaseMethod() = 0;
// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for virtual
method 'BadBaseMethod'
+ // CHECK-FIXES: {{^}} virtual void v_Bad_Base_Method() = 0;
};
class COverriding : public AOverridden {
public:
// Overriding a badly-named base isn't a new violation.
void BadBaseMethod() override {}
+ // CHECK-FIXES: {{^}} void v_Bad_Base_Method() override {}
+
+ void foo() {
+BadBaseMethod();
+// CHECK-FIXES: {{^}}v_Bad_Base_Method();
+this->BadBaseMethod();
+// CHECK-FIXES: {{^}}this->v_Bad_Base_Method();
+AOverridden::BadBaseMethod();
+// CHECK-FIXES: {{^}}AOverridden::v_Bad_Base_Method();
+COverriding::BadBaseMethod();
+// CHECK-FIXES: {{^}}COverriding::v_Bad_Base_Method();
+ }
};
+void VirtualCall(AOverridden _vItem) {
+ a_vItem.BadBaseMethod();
+ // CHECK-FIXES: {{^}} a_vItem.v_Bad_Base_Method();
+}
+
template
class CRTPBase {
public:
Index: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
@@ -127,6 +127,26 @@
this));
}
+/// Returns the function that \p Method is overridding. If There are none or
+/// multiple overrides it returns nullptr. If the overridden function itself is
+/// overridding then it will recurse up to find the first decl of the function.
+static const CXXMethodDecl *getOverrideMethod(const CXXMethodDecl *Method) {
+ if (Method->size_overridden_methods() != 1)
+return nullptr; // no overrides
+ while (true) {
+const CXXMethodDecl *Next = *Method->begin_overridden_methods();
+assert(Next && "Overridden method shouldn't be null");
+if (Next->size_overridden_methods() == 0) {
+ return Next;
+}
+if (Next->size_overridden_methods() == 1) {
+ Method = Next;
+ continue;
+}
+return nullptr; // Multiple overrides
+ }
+}
+
void RenamerClangTidyCheck::addUsage(
const RenamerClangTidyCheck::NamingCheckId , SourceRange Range,
SourceManager *SourceMgr) {
@@ -160,7 +180,10 @@
void RenamerClangTidyCheck::addUsage(const NamedDecl *Decl, SourceRange Range,
SourceManager *SourceMgr) {
-
+ if (const auto *Method = dyn_cast(Decl)) {
+if (const CXXMethodDecl *Overridden = getOverrideMethod(Method))
+ Decl = Overridden;
+ }
return addUsage(RenamerClangTidyCheck::NamingCheckId(Decl->getLocation(),
Decl->getNameAsString()),
Range, SourceMgr);
@@ -390,6 +413,14 @@
}
}
+// Fix overridden methods
+if (const auto *Method = Result.Nodes.getNodeAs("decl")) {
+ if (const CXXMethodDecl *Overridden = getOverrideMethod(Method)) {
+addUsage(Overridden, Method->getLocation());
+return; // Don't try to add the actual decl as a Failure.
+ }
+}
+
// Ignore ClassTemplateSpecializationDecl which are creating duplicate
// replacements with CXXRecordDecl.
if (isa(Decl))
Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
@@ -267,14 +267,32 @@
virtual ~AOverridden() = default;
virtual void BadBaseMethod() = 0;
// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for virtual method 'BadBaseMethod'
+ // CHECK-FIXES: {{^}} virtual void v_Bad_Base_Method() = 0;
};
class COverriding : public AOverridden {
public:
// Overriding a badly-named base isn't a new violation.
void BadBaseMethod() override {}
+ // CHECK-FIXES: {{^}} void v_Bad_Base_Method() override {}
+
+ void foo() {
+BadBaseMethod();
+// CHECK-FIXES: {{^}}
