[clang-tools-extra] 7a55021 - [clang-tidy] Fix confusable identifiers interaction with DeclContext
via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 4 23:56:44 PDT 2022
Author: serge-sans-paille
Date: 2022-07-05T08:56:26+02:00
New Revision: 7a550212e8ff5552b0bd10d0c1af459a55c36234
URL: https://github.com/llvm/llvm-project/commit/7a550212e8ff5552b0bd10d0c1af459a55c36234
DIFF: https://github.com/llvm/llvm-project/commit/7a550212e8ff5552b0bd10d0c1af459a55c36234.diff
LOG: [clang-tidy] Fix confusable identifiers interaction with DeclContext
Properly checks enclosing DeclContext, and add the related test case.
It would be great to be able to use Sema to check conflicting scopes, but that's
not something clang-tidy seems to be able to do :-/
Fix #56221
Differential Revision: https://reviews.llvm.org/D128715
Added:
Modified:
clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/misc/confusable-identifiers.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.cpp b/clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.cpp
index 5783f85a79031..ce83a7778d80d 100644
--- a/clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.cpp
@@ -93,22 +93,70 @@ std::string ConfusableIdentifierCheck::skeleton(StringRef Name) {
return Skeleton;
}
+static bool mayShadowImpl(const NamedDecl *ND0, const NamedDecl *ND1) {
+ const DeclContext *DC0 = ND0->getDeclContext()->getPrimaryContext();
+ const DeclContext *DC1 = ND1->getDeclContext()->getPrimaryContext();
+
+ if (isa<TemplateTypeParmDecl>(ND0) || isa<TemplateTypeParmDecl>(ND0))
+ return true;
+
+ while (DC0->isTransparentContext())
+ DC0 = DC0->getParent();
+ while (DC1->isTransparentContext())
+ DC1 = DC1->getParent();
+
+ if (DC0->Equals(DC1))
+ return true;
+
+ return false;
+}
+
+static bool isMemberOf(const NamedDecl *ND, const CXXRecordDecl *RD) {
+ const DeclContext *NDParent = ND->getDeclContext();
+ if (!NDParent || !isa<CXXRecordDecl>(NDParent))
+ return false;
+ if (NDParent == RD)
+ return true;
+ return !RD->forallBases(
+ [NDParent](const CXXRecordDecl *Base) { return NDParent != Base; });
+}
+
+static bool mayShadow(const NamedDecl *ND0, const NamedDecl *ND1) {
+
+ const DeclContext *DC0 = ND0->getDeclContext()->getPrimaryContext();
+ const DeclContext *DC1 = ND1->getDeclContext()->getPrimaryContext();
+
+ if (const CXXRecordDecl *RD0 = dyn_cast<CXXRecordDecl>(DC0)) {
+ if (ND1->getAccess() != AS_private && isMemberOf(ND1, RD0))
+ return true;
+ }
+ if (const CXXRecordDecl *RD1 = dyn_cast<CXXRecordDecl>(DC1)) {
+ if (ND0->getAccess() != AS_private && isMemberOf(ND0, RD1))
+ return true;
+ }
+
+ if (DC0->Encloses(DC1))
+ return mayShadowImpl(ND0, ND1);
+ if (DC1->Encloses(DC0))
+ return mayShadowImpl(ND1, ND0);
+ return false;
+}
+
void ConfusableIdentifierCheck::check(
const ast_matchers::MatchFinder::MatchResult &Result) {
if (const auto *ND = Result.Nodes.getNodeAs<NamedDecl>("nameddecl")) {
- if (IdentifierInfo *II = ND->getIdentifier()) {
- StringRef NDName = II->getName();
+ if (IdentifierInfo *NDII = ND->getIdentifier()) {
+ StringRef NDName = NDII->getName();
llvm::SmallVector<const NamedDecl *> &Mapped = Mapper[skeleton(NDName)];
- const DeclContext *NDDecl = ND->getDeclContext();
for (const NamedDecl *OND : Mapped) {
- if (!NDDecl->isDeclInLexicalTraversal(OND) &&
- !OND->getDeclContext()->isDeclInLexicalTraversal(ND))
- continue;
- if (OND->getIdentifier()->getName() != NDName) {
- diag(OND->getLocation(), "%0 is confusable with %1")
- << OND->getName() << NDName;
- diag(ND->getLocation(), "other declaration found here",
- DiagnosticIDs::Note);
+ const IdentifierInfo *ONDII = OND->getIdentifier();
+ if (mayShadow(ND, OND)) {
+ StringRef ONDName = ONDII->getName();
+ if (ONDName != NDName) {
+ diag(ND->getLocation(), "%0 is confusable with %1") << ND << OND;
+ diag(OND->getLocation(), "other declaration found here",
+ DiagnosticIDs::Note);
+ }
}
}
Mapped.push_back(ND);
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/confusable-identifiers.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/confusable-identifiers.cpp
index b3956b97c9454..8529d1ef8e8b5 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/misc/confusable-identifiers.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/confusable-identifiers.cpp
@@ -1,9 +1,9 @@
// RUN: %check_clang_tidy %s misc-confusable-identifiers %t
int fo;
-// CHECK-MESSAGES: :[[#@LINE-1]]:5: warning: fo is confusable with 𝐟o [misc-confusable-identifiers]
int 𝐟o;
-// CHECK-MESSAGES: :[[#@LINE-1]]:5: note: other declaration found here
+// CHECK-MESSAGES: :[[#@LINE-1]]:5: warning: '𝐟o' is confusable with 'fo' [misc-confusable-identifiers]
+// CHECK-MESSAGES: :[[#@LINE-3]]:5: note: other declaration found here
void no() {
int 𝐟oo;
@@ -12,14 +12,102 @@ void no() {
void worry() {
int foo;
}
-
int 𝐟i;
-// CHECK-MESSAGES: :[[#@LINE-1]]:5: warning: 𝐟i is confusable with fi [misc-confusable-identifiers]
int fi;
-// CHECK-MESSAGES: :[[#@LINE-1]]:5: note: other declaration found here
+// CHECK-MESSAGES: :[[#@LINE-1]]:5: warning: 'fi' is confusable with '𝐟i' [misc-confusable-identifiers]
+// CHECK-MESSAGES: :[[#@LINE-3]]:5: note: other declaration found here
+
+bool f0(const char *q1, const char *ql) {
+ // CHECK-MESSAGES: :[[#@LINE-1]]:37: warning: 'ql' is confusable with 'q1' [misc-confusable-identifiers]
+ // CHECK-MESSAGES: :[[#@LINE-2]]:21: note: other declaration found here
+ return q1 < ql;
+}
// should not print anything
namespace ns {
struct Foo {};
} // namespace ns
auto f = ns::Foo();
+
+struct Test {
+ void f1(const char *pl);
+};
+
+bool f2(const char *p1, const char *ql) {
+ return p1 < ql;
+}
+
+bool f3(const char *q0, const char *q1) {
+ return q0 < q1;
+}
+
+template <typename i1>
+struct S {
+ template <typename il>
+ void f4() {}
+ // CHECK-MESSAGES: :[[#@LINE-2]]:22: warning: 'il' is confusable with 'i1' [misc-confusable-identifiers]
+ // CHECK-MESSAGES: :[[#@LINE-5]]:20: note: other declaration found here
+};
+
+template <typename i1>
+void f5(int il) {
+ // CHECK-MESSAGES: :[[#@LINE-1]]:13: warning: 'il' is confusable with 'i1' [misc-confusable-identifiers]
+ // CHECK-MESSAGES: :[[#@LINE-3]]:20: note: other declaration found here
+}
+
+template <typename O0>
+void f6() {
+ int OO = 0;
+ // CHECK-MESSAGES: :[[#@LINE-1]]:7: warning: 'OO' is confusable with 'O0' [misc-confusable-identifiers]
+ // CHECK-MESSAGES: :[[#@LINE-4]]:20: note: other declaration found here
+}
+int OO = 0; // no warning, not same scope as f6
+
+namespace f7 {
+int i1;
+}
+
+namespace f8 {
+int il; // no warning,
diff erent namespace
+}
+
+namespace f7 {
+int il;
+// CHECK-MESSAGES: :[[#@LINE-1]]:5: warning: 'il' is confusable with 'i1' [misc-confusable-identifiers]
+// CHECK-MESSAGES: :[[#@LINE-10]]:5: note: other declaration found here
+} // namespace f7
+
+template <typename t1, typename tl>
+// CHECK-MESSAGES: :[[#@LINE-1]]:33: warning: 'tl' is confusable with 't1' [misc-confusable-identifiers]
+// CHECK-MESSAGES: :[[#@LINE-2]]:20: note: other declaration found here
+void f9();
+
+struct Base0 {
+ virtual void mO0();
+
+private:
+ void mII();
+};
+
+struct Derived0 : Base0 {
+ void mOO();
+ // CHECK-MESSAGES: :[[#@LINE-1]]:8: warning: 'mOO' is confusable with 'mO0' [misc-confusable-identifiers]
+ // CHECK-MESSAGES: :[[#@LINE-9]]:16: note: other declaration found here
+
+ void mI1(); // no warning: mII is private
+};
+
+struct Base1 {
+ long mO0;
+
+private:
+ long mII;
+};
+
+struct Derived1 : Base1 {
+ long mOO;
+ // CHECK-MESSAGES: :[[#@LINE-1]]:8: warning: 'mOO' is confusable with 'mO0' [misc-confusable-identifiers]
+ // CHECK-MESSAGES: :[[#@LINE-9]]:8: note: other declaration found here
+
+ long mI1(); // no warning: mII is private
+};
More information about the cfe-commits
mailing list