[PATCH] D112916: [clang-tidy] Confusable identifiers detection

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc3574ef739fb: [clang-tidy] Confusable identifiers detection (authored by serge-sans-paille). Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D112916: [clang-tidy] Confusable identifiers detection

whisperity accepted this revision. whisperity added a comment. This revision is now accepted and ready to land. LGTM. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112916/new/ https://reviews.llvm.org/D112916 ___ cfe-commits mailing l

[PATCH] D112916: [clang-tidy] Confusable identifiers detection

aaron.ballman accepted this revision. aaron.ballman added a comment. I remain concerned about the utility of this check. As I understand it, the more complete Unicode guidance is still under active discussion and hasn't been finalized (please correct me if I'm wrong @tahonermann and @cor3ntin) s

[PATCH] D112916: [clang-tidy] Confusable identifiers detection

serge-sans-paille updated this revision to Diff 438592. serge-sans-paille added a comment. Address most reviewers comment: - formatting style - reduced memory consumption - be clear about TR39 divergence - class and option renaming - getName() usage CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D112916: [clang-tidy] Confusable identifiers detection

cor3ntin added a comment. So that this doesn't get lost, I'm opposed to this change because it does not actually follow the TR39 guidance or algorithm. For an input string X, define skeleton(X) to be the following transformation on the string: 1. Convert X to NFD format, as described in

[PATCH] D112916: [clang-tidy] Confusable identifiers detection

whisperity requested changes to this revision. whisperity added a comment. This revision now requires changes to proceed. Because of the stability issues related to `getName()`-like constructs I'm putting a temporary ❌ on this (so it doesn't show up as faux accept). However, I have to emphasise

[PATCH] D112916: [clang-tidy] Confusable identifiers detection

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/Homoglyph.cpp:32-35 +/** + * Build a skeleton out of the Original identifier, following the algorithm + * described in http://www.unicode.org/reports/tr39/#def-skeleton + */ `/*` -> `

[PATCH] D112916: [clang-tidy] Confusable identifiers detection

serge-sans-paille updated this revision to Diff 436739. serge-sans-paille marked an inline comment as done. serge-sans-paille added a comment. Take into account @aaron.ballman review CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112916/new/ https://reviews.llvm.org/D112916 Files: cla

[PATCH] D112916: [clang-tidy] Confusable identifiers detection

aaron.ballman added a comment. In D112916#3554181 , @tstellar wrote: > In D112916#3554158 , @Mordante > wrote: > >> In D112916#3113140 , @tstellar >> wrote: >> >>> The L

[PATCH] D112916: [clang-tidy] Confusable identifiers detection

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Herald added a subscriber: StephenFan. LGTM. Added some clang-tidy reviewers. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112916/new/ https://reviews.llvm.org/D112916 ___

[PATCH] D112916: [clang-tidy] Confusable identifiers detection

serge-sans-paille updated this revision to Diff 436506. serge-sans-paille retitled this revision from "Confusable identifiers detection" to "[clang-tidy] Confusable identifiers detection". serge-sans-paille added a comment. Herald added a subscriber: xazax.hun. Address reviews CHANGES SINCE LAS