Author: Piotr Zegar Date: 2023-06-10T11:06:49Z New Revision: 8fdedcd1a242f6b54eec969e72e35ac0a68b7ea1
URL: https://github.com/llvm/llvm-project/commit/8fdedcd1a242f6b54eec969e72e35ac0a68b7ea1 DIFF: https://github.com/llvm/llvm-project/commit/8fdedcd1a242f6b54eec969e72e35ac0a68b7ea1.diff LOG: [clang-tidy] Optimize misc-confusable-identifiers This is final optimization for this check. Main improvements comes from changing a logic order in mayShadow function, to first validate result of mayShadowImpl, then search primary context in a vectors. Secondary improvement comes from excluding all implicit code by using TK_IgnoreUnlessSpelledInSource. All other changes are just cosmetic improvements. Tested on Cataclysm-DDA open source project, result in check execution time reduction from 3682 seconds to 100 seconds (~0.25s per TU). That's 97.2% reduction for this change alone. Resulting in cumulative improvement for this check around -99.6%, finally bringing this check into a cheap category. Reviewed By: serge-sans-paille Differential Revision: https://reviews.llvm.org/D151594 Added: Modified: clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.cpp clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.h Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.cpp b/clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.cpp index 63ba663aaca9a..c2f72c8fc20a0 100644 --- a/clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.cpp @@ -11,6 +11,7 @@ #include "clang/Frontend/CompilerInstance.h" #include "clang/Lex/Preprocessor.h" +#include "llvm/ADT/SmallString.h" #include "llvm/Support/ConvertUTF.h" namespace { @@ -45,14 +46,13 @@ ConfusableIdentifierCheck::~ConfusableIdentifierCheck() = default; // We're skipping 1. and 3. for the sake of simplicity, but this can lead to // false positive. -static std::string skeleton(StringRef Name) { +static llvm::SmallString<64U> skeleton(StringRef Name) { using namespace llvm; - std::string SName = Name.str(); - std::string Skeleton; - Skeleton.reserve(1 + Name.size()); + SmallString<64U> Skeleton; + Skeleton.reserve(1U + Name.size()); - const char *Curr = SName.c_str(); - const char *End = Curr + SName.size(); + const char *Curr = Name.data(); + const char *End = Curr + Name.size(); while (Curr < End) { const char *Prev = Curr; @@ -99,8 +99,6 @@ static bool mayShadowImpl(const NamedDecl *ND0, const NamedDecl *ND1) { static bool isMemberOf(const ConfusableIdentifierCheck::ContextInfo *DC0, const ConfusableIdentifierCheck::ContextInfo *DC1) { - if (DC0->Bases.empty()) - return false; return llvm::is_contained(DC1->Bases, DC0->PrimaryContext); } @@ -117,16 +115,23 @@ static bool mayShadow(const NamedDecl *ND0, const ConfusableIdentifierCheck::ContextInfo *DC0, const NamedDecl *ND1, const ConfusableIdentifierCheck::ContextInfo *DC1) { - if (!DC0->Bases.empty() && ND1->getAccess() != AS_private && - isMemberOf(DC1, DC0)) - return true; - if (!DC1->Bases.empty() && ND0->getAccess() != AS_private && - isMemberOf(DC0, DC1)) - return true; - return enclosesContext(DC0, DC1) && - (mayShadowImpl(ND0, ND1) || mayShadowImpl(DC0->NonTransparentContext, - DC1->NonTransparentContext)); + if (!DC0->Bases.empty() && !DC1->Bases.empty()) { + // if any of the declaration is a non-private member of the other + // declaration, it's shadowed by the former + + if (ND1->getAccess() != AS_private && isMemberOf(DC1, DC0)) + return true; + + if (ND0->getAccess() != AS_private && isMemberOf(DC0, DC1)) + return true; + } + + if (!mayShadowImpl(DC0->NonTransparentContext, DC1->NonTransparentContext) && + !mayShadowImpl(ND0, ND1)) + return false; + + return enclosesContext(DC0, DC1); } const ConfusableIdentifierCheck::ContextInfo * @@ -172,26 +177,41 @@ ConfusableIdentifierCheck::getContextInfo(const DeclContext *DC) { void ConfusableIdentifierCheck::check( const ast_matchers::MatchFinder::MatchResult &Result) { - if (const auto *ND = Result.Nodes.getNodeAs<NamedDecl>("nameddecl")) { - if (IdentifierInfo *NDII = ND->getIdentifier()) { - const ContextInfo *Info = getContextInfo(ND->getDeclContext()); - StringRef NDName = NDII->getName(); - llvm::SmallVector<Entry> &Mapped = Mapper[skeleton(NDName)]; - for (const Entry &E : Mapped) { - const IdentifierInfo *ONDII = E.Declaration->getIdentifier(); - if (mayShadow(ND, Info, E.Declaration, E.Info)) { - StringRef ONDName = ONDII->getName(); - if (ONDName != NDName) { - diag(ND->getLocation(), "%0 is confusable with %1") - << ND << E.Declaration; - diag(E.Declaration->getLocation(), "other declaration found here", - DiagnosticIDs::Note); - } - } - } - Mapped.push_back({ND, Info}); - } + const auto *ND = Result.Nodes.getNodeAs<NamedDecl>("nameddecl"); + if (!ND) + return; + + IdentifierInfo *NDII = ND->getIdentifier(); + if (!NDII) + return; + + StringRef NDName = NDII->getName(); + if (NDName.empty()) + return; + + const ContextInfo *Info = getContextInfo(ND->getDeclContext()); + + llvm::SmallVector<Entry> &Mapped = Mapper[skeleton(NDName)]; + for (const Entry &E : Mapped) { + if (!mayShadow(ND, Info, E.Declaration, E.Info)) + continue; + + const IdentifierInfo *ONDII = E.Declaration->getIdentifier(); + StringRef ONDName = ONDII->getName(); + if (ONDName == NDName) + continue; + + diag(ND->getLocation(), "%0 is confusable with %1") << ND << E.Declaration; + diag(E.Declaration->getLocation(), "other declaration found here", + DiagnosticIDs::Note); } + + Mapped.push_back({ND, Info}); +} + +void ConfusableIdentifierCheck::onEndOfTranslationUnit() { + Mapper.clear(); + ContextInfos.clear(); } void ConfusableIdentifierCheck::registerMatchers( diff --git a/clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.h b/clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.h index ede058c7197b7..f3b0c8ed00306 100644 --- a/clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.h +++ b/clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.h @@ -26,6 +26,10 @@ class ConfusableIdentifierCheck : public ClangTidyCheck { void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void onEndOfTranslationUnit() override; + std::optional<TraversalKind> getCheckTraversalKind() const override { + return TK_IgnoreUnlessSpelledInSource; + } struct ContextInfo { const DeclContext *PrimaryContext; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
