[PATCH] D112914: Misleading identifier detection

aaron.ballman added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/misc-misleading-identifier.rst:12 + +.. code-block:: c + aaronpuchert wrote: > Sphinx can't parse that (perhaps unsurprisingly), could you declare that as > having no

[PATCH] D112914: Misleading identifier detection

serge-sans-paille added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/misc-misleading-identifier.cpp:3 + +#include + RKSimon wrote: > @serge-sans-paille Any chance you can remove the include and just forward > declare printf()? Sure,

[PATCH] D112914: Misleading identifier detection

RKSimon added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/misc-misleading-identifier.cpp:3 + +#include + @serge-sans-paille Any chance you can remove the include and just forward declare printf()? Repository: rG LLVM Github

[PATCH] D112914: Misleading identifier detection

RKSimon added a comment. @serge-sans-paille Still failing on https://lab.llvm.org/buildbot/#/builders/139/builds/12974 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112914/new/ https://reviews.llvm.org/D112914

[PATCH] D112914: Misleading identifier detection

serge-sans-paille added a comment. In D112914#3119704 , @thakis wrote: > The test seems to trigger an assert: http://45.33.8.238/linux/60293/step_9.txt > > Please take a look! It's getting late here: Reverted, I'll have a look tomorrow. Repository:

[PATCH] D112914: Misleading identifier detection

thakis added a comment. The test seems to trigger an assert: http://45.33.8.238/linux/60293/step_9.txt Please take a look! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112914/new/ https://reviews.llvm.org/D112914

[PATCH] D112914: Misleading identifier detection

RKSimon added a comment. In D112914#3119103 , @serge-sans-paille wrote: > Yep, I'm currently validating locally, I somehow messed up a rebase :-/ I'm sorry @serge-sans-paille I missed this reply before pushing the revert :( Repository: rG LLVM

[PATCH] D112914: Misleading identifier detection

serge-sans-paille added a comment. Yep, I'm currently validating locally, I somehow messed up a rebase :-/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112914/new/ https://reviews.llvm.org/D112914 ___

[PATCH] D112914: Misleading identifier detection

aaronpuchert added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/misc-misleading-identifier.rst:12 + +.. code-block:: c + Sphinx can't parse that (perhaps unsurprisingly), could you declare that as having no language? ``` Warning,

[PATCH] D112914: Misleading identifier detection

RKSimon added a comment. @serge-sans-paille Any luck with a fix for all the buildbot failures or should we revert ? https://lab.llvm.org/buildbot/#/builders/109/builds/25932 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112914/new/

[PATCH] D112914: Misleading identifier detection

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG299aa4dfa1d8: Misleading unicode identifier detection pass (authored by serge-sans-paille). Changed prior to commit:

[PATCH] D112914: Misleading identifier detection

rsmith accepted this revision. rsmith added a comment. In D112914#3102728 , @carlosgalvezp wrote: > I don't know how to remove the "Requested changes" from here so I'll just > remove myself from reviewer. Marking the change as accepted should

[PATCH] D112914: Misleading identifier detection

serge-sans-paille updated this revision to Diff 384381. serge-sans-paille added a comment. Fix comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112914/new/ https://reviews.llvm.org/D112914 Files: clang-tools-extra/clang-tidy/misc/CMakeLists.txt

[PATCH] D112914: Misleading identifier detection

rsmith added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/MisleadingIdentifier.h:2 +//===--- MisleadingIdentifierCheck.h - clang-tidy *- C++ +//-*-===// +// This needs fixing too. CHANGES SINCE LAST ACTION

[PATCH] D112914: Misleading identifier detection

serge-sans-paille updated this revision to Diff 384118. serge-sans-paille added a comment. Minor typos CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112914/new/ https://reviews.llvm.org/D112914 Files: clang-tools-extra/clang-tidy/misc/CMakeLists.txt

[PATCH] D112914: Misleading identifier detection

serge-sans-paille added a comment. In D112914#3102728 , @carlosgalvezp wrote: > Ok! I don't really know what applies when you take //part// of a file, so > I'll leave that up to people who know. I don't know how to remove the > "Requested changes"

[PATCH] D112914: Misleading identifier detection

carlosgalvezp resigned from this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. Ok! I don't really know what applies when you take //part// of a file, so I'll leave that up to people who know. I don't know how to remove the "Requested changes" from

[PATCH] D112914: Misleading identifier detection

serge-sans-paille added a comment. AKAIU, the licensing issue doesn't impact that particular review, only the one on confusable identifiers. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112914/new/ https://reviews.llvm.org/D112914 ___

[PATCH] D112914: Misleading identifier detection

carlosgalvezp added a comment. Looks good, I guess the license issue still needs to be sorted out? Comment at: clang-tools-extra/docs/clang-tidy/checks/misc-misleading-identifier.rst:6 + +Finds identifiers that contain Unicode characetrs with right-to-left direction, +which

[PATCH] D112914: Misleading identifier detection

serge-sans-paille updated this revision to Diff 384020. serge-sans-paille added a comment. Update release note too. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112914/new/ https://reviews.llvm.org/D112914 Files: clang-tools-extra/clang-tidy/misc/CMakeLists.txt

[PATCH] D112914: Misleading identifier detection

serge-sans-paille updated this revision to Diff 384017. serge-sans-paille added a comment. Also update clang-tidy doc index CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112914/new/ https://reviews.llvm.org/D112914 Files: clang-tools-extra/clang-tidy/misc/CMakeLists.txt

[PATCH] D112914: Misleading identifier detection

carlosgalvezp requested changes to this revision. carlosgalvezp added a comment. This revision now requires changes to proceed. - Add check to list of checks: https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/docs/clang-tidy/checks/list.rst - Mention check in the Release Notes:

[PATCH] D112914: Misleading identifier detection

serge-sans-paille updated this revision to Diff 384006. serge-sans-paille added a comment. - fix formatting - added documentation - *not* doing the extra work wrt. unicode error recovery CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112914/new/ https://reviews.llvm.org/D112914 Files:

[PATCH] D112914: Misleading identifier detection

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Looks good as far as it goes (though I've not checked your classification functions are correct). We should have some detection for RTL characters in string literals and comments too, at

[PATCH] D112914: Misleading identifier detection

serge-sans-paille created this revision. Herald added subscribers: carlosgalvezp, mgorny. serge-sans-paille requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Detect identifiers with right-to-left ordering through clang-tidy. It