diff --git a/clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.cpp b/clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.cpp --- a/clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.cpp @@ -93,22 +93,70 @@ 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(ND0) || isa(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(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(DC0)) { + if (ND1->getAccess() != AS_private && isMemberOf(ND1, RD0)) + return true; + } + if (const CXXRecordDecl *RD1 = dyn_cast(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")) { - if (IdentifierInfo *II = ND->getIdentifier()) { - StringRef NDName = II->getName(); + if (IdentifierInfo *NDII = ND->getIdentifier()) { + StringRef NDName = NDII->getName(); llvm::SmallVector &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 --- 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 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 +struct S { + template + 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 +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 +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, different 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 +// 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 +};