rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land.
Nits and a suggested approach for invalid code sequences that's probably important to handle better. Please fix the clang-tidy findings too. Otherwise, LGTM. ================ Comment at: clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.cpp:38 + + /* Scan each character while maintaining a count of opened bidi context. + * RLO/RLE/LRO/LRE all are closed by PDF while RLI LRI and FSI are closed by ---------------- Nit: please use `//` comments per our normal coding style. ================ Comment at: clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.cpp:49 + ++CurPtr; + // line break: https://www.unicode.org/reports/tr14/tr14-32.html + if(C == '\n' || C == '\r' || C == '\f' || C == '\v' || C == 0x85 /*next line*/) { ---------------- ================ Comment at: clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.cpp:60-62 + // If conversion fails, we stop the analysis because we don't know how many + // characters should be skipped otherwise. That's an obvious way to bypass + // the check. ---------------- Can we scan forward looking for the next non-continuation byte? (Skip while `c & 0b1100_0000 == 0b1000_0000`) ================ Comment at: clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.cpp:74 + Isolate = std::min(Isolate - 1, Isolate); + // line break: https://www.unicode.org/reports/tr14/tr14-32.html + else if (CodePoint == LS || CodePoint == PS) ---------------- Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112913/new/ https://reviews.llvm.org/D112913 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
