fwolff created this revision.
fwolff added reviewers: alexfh, salman-javed-nz.
fwolff added a project: clang-tools-extra.
Herald added subscribers: carlosgalvezp, xazax.hun.
fwolff requested review of this revision.
Herald added a subscriber: cfe-commits.
Fixes part of PR#45815. Overriding methods should not get a
`readability-identifier-naming` warning because the issue can only be fixed in
the base class; but the current check for whether a method is overriding does
not take the `override` attribute into account, which makes a difference for
dependent base classes.
The other issue mentioned in PR#45815 is not solved by this patch: Applying the
fix provided by `readability-identifier-naming` only changes the name in the
base class, not in the derived class(es) or at any call sites. This is
difficult to fix, because in addition to the template base class, there could
be specializations of the base class that also contain the overridden method,
which is why applying the fix from the base class in the derived class in
general would not lead to correct code.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D113830
Files:
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.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
@@ -305,9 +305,29 @@
}
};
-void VirtualCall(AOverridden &a_vItem) {
+template<typename some_t>
+class ATOverridden {
+public:
+ 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;
+};
+
+template<typename some_t>
+class CTOverriding : public ATOverridden<some_t> {
+ // Overriding a badly-named base isn't a new violation.
+ // FIXME: The fixes from the base class should be propagated to the derived
class here
+ // (note that there could be specializations of the template base
class, though)
+ void BadBaseMethod() override{}
+};
+
+template<typename some_t>
+void VirtualCall(AOverridden &a_vItem, ATOverridden<some_t> &a_vTitem) {
a_vItem.BadBaseMethod();
// CHECK-FIXES: {{^}} a_vItem.v_Bad_Base_Method();
+
+ // FIXME: The fixes from ATOverridden should be propagated to the following
call
+ a_vTitem.BadBaseMethod();
}
template <typename derived_t>
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -1257,7 +1257,7 @@
if (const auto *Decl = dyn_cast<CXXMethodDecl>(D)) {
if (Decl->isMain() || !Decl->isUserProvided() ||
- Decl->size_overridden_methods() > 0)
+ Decl->size_overridden_methods() > 0 || Decl->hasAttr<OverrideAttr>())
return SK_Invalid;
// If this method has the same name as any base method, this is likely
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
@@ -305,9 +305,29 @@
}
};
-void VirtualCall(AOverridden &a_vItem) {
+template<typename some_t>
+class ATOverridden {
+public:
+ 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;
+};
+
+template<typename some_t>
+class CTOverriding : public ATOverridden<some_t> {
+ // Overriding a badly-named base isn't a new violation.
+ // FIXME: The fixes from the base class should be propagated to the derived class here
+ // (note that there could be specializations of the template base class, though)
+ void BadBaseMethod() override{}
+};
+
+template<typename some_t>
+void VirtualCall(AOverridden &a_vItem, ATOverridden<some_t> &a_vTitem) {
a_vItem.BadBaseMethod();
// CHECK-FIXES: {{^}} a_vItem.v_Bad_Base_Method();
+
+ // FIXME: The fixes from ATOverridden should be propagated to the following call
+ a_vTitem.BadBaseMethod();
}
template <typename derived_t>
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -1257,7 +1257,7 @@
if (const auto *Decl = dyn_cast<CXXMethodDecl>(D)) {
if (Decl->isMain() || !Decl->isUserProvided() ||
- Decl->size_overridden_methods() > 0)
+ Decl->size_overridden_methods() > 0 || Decl->hasAttr<OverrideAttr>())
return SK_Invalid;
// If this method has the same name as any base method, this is likely
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
