[PATCH] D112916: Confusable identifiers detection

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/misc/CMakeLists.txt:9
+add_custom_command(
+    OUTPUT confusables.h
+    COMMAND make_confusable_table ${CMAKE_CURRENT_SOURCE_DIR}/ConfusableTable/confusables.txt ${CMAKE_CURRENT_BINARY_DIR}/confusables.h
----------------
Please generate a filename that follows our normal conventions (`Confusables.inc`).


================
Comment at: clang-tools-extra/clang-tidy/misc/ConfusableTable/confusables.txt:5
+# Unicode and the Unicode Logo are registered trademarks of Unicode, Inc. in the U.S. and other countries.
+# For terms of use, see http://www.unicode.org/terms_of_use.html
+#
----------------
>From the terms of use we find this: https://www.unicode.org/license.html and in particular:

> [...] provided that either
> (a) this copyright and permission notice appear with all copies
> of the Data Files or Software, or
> (b) this copyright and permission notice appear in associated
> Documentation.

... so it looks like we might need to include a copy of at least the Unicode license. I'd like someone from the LLVM foundation to check for licensing concerns here before this lands.


================
Comment at: clang-tools-extra/clang-tidy/misc/Homoglyph.cpp:1-2
+//===--- MisleadingBidirectional.cpp - clang-tidy
+//--------------------------------===//
+//
----------------
Nit: fix the formatting here.


================
Comment at: clang-tools-extra/clang-tidy/misc/Homoglyph.cpp:89-90
+    for (auto *OND : Mapped) {
+      if (!NDDecl->isDeclInLexicalTraversal(OND))
+        continue;
+      if (OND->getName() != NDName) {
----------------
I think we also need to check for the reverse case (`OND->getDeclContext()->isDeclInLexicalTraversal(ND)`) here (assuming `a` and `A ` are homoglyphs etc):

```
int a;
namespace N {
  int A; // new is in context of old but not vice versa
  int b;
}
int B; // old is in context of new but not vice versa
namespace N {
  int c = a + B; // finds  ::a and ::B
}
```


================
Comment at: clang-tools-extra/clang-tidy/misc/Homoglyph.h:1
+//===--- Homoglyph.h - clang-tidy ------------------------*- C++ -*-===//
+//
----------------
Nit: add more `-`s to pad to 80 characters.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112916/new/

https://reviews.llvm.org/D112916