This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] Optimize misc-confusable-identifiers
ClosedPublic

Authored by PiotrZSL on May 26 2023, 3:12 PM.

Details

Summary

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.

Diff Detail

Event Timeline

PiotrZSL created this revision.May 26 2023, 3:12 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 26 2023, 3:12 PM
Herald added a subscriber: xazax.hun. · View Herald Transcript
PiotrZSL requested review of this revision.May 26 2023, 3:12 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 26 2023, 3:13 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
serge-sans-paille accepted this revision.Jun 7 2023, 7:37 AM

LGTM with a minor nit. Thanks for the optimization!

clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.cpp
124

I <3 this simplification. It's more difficult to understand though, could you add a small content along those lines: if any of the declaration is a non-private member of the other declaration, it's shadowed by the former

This revision is now accepted and ready to land.Jun 7 2023, 7:37 AM
PiotrZSL updated this revision to Diff 530185.Jun 10 2023, 2:08 AM

Add comment to code

This revision was automatically updated to reflect the committed changes.