aaron.ballman added a comment.

In D112916#3331592 <https://reviews.llvm.org/D112916#3331592>, @cor3ntin wrote:

> In D112916#3314184 <https://reviews.llvm.org/D112916#3314184>, @cor3ntin 
> wrote:
>
>> @aaron.ballman Thanks for the ping.
>>
>> One one hand, I agree with you, on the other hand, this tries to stick to 
>> TR39, and I think we should stick with that. It might be worth checking with 
>> the Unicode consortium what they think of i/l as confusable.
>
> Actually @aaron.ballman, Unicode does consider these confusables already
>
> from https://www.unicode.org/Public/security/14.0.0/confusables.txt
>
>   0031 ;      006C ;  MA      # ( 1 → l ) DIGIT ONE → LATIN SMALL LETTER L    
> # 
>   0049 ;      006C ;  MA      # ( I → l ) LATIN CAPITAL LETTER I → LATIN 
> SMALL LETTER L       # 
>   0030 ;      004F ;  MA      # ( 0 → O ) DIGIT ZERO → LATIN CAPITAL LETTER O 
> # 
>
> So ASCII is already taken care of. No issue here :)

Oh, so it is, good catch! I looked for it before, but missed it.

FWIW, I agree with sticking with what Unicode defines here. (I am worried about 
the results in practice and whether enough people will use this check to 
warrant its inclusion in clang-tidy, but not enough to block this patch.)


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

https://reviews.llvm.org/D112916

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to