[PATCH] D124748: [clang-format] Fix whitespace counting stuff

Owen Pan via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue May 3 14:50:26 PDT 2022


owenpan added a comment.

Is this patch NFC?



================
Comment at: clang/lib/Format/FormatTokenLexer.cpp:859-867
+    switch (Cur[0]) {
+    case '\n':
+    case '\r':
+    case '\f':
+    case '\v':
+    case ' ':
+    case '\t':
----------------
I'd replace the `switch` with `if (isspace(Cur[0])) ... else if (Cur[1] == '\n' ...) ...`.


================
Comment at: clang/lib/Format/FormatTokenLexer.cpp:914
+    auto LeadingWhitespace = countLeadingWhitespace(FormatTok->TokenText);
+    if (!LeadingWhitespace)
+      break;
----------------



================
Comment at: clang/lib/Format/FormatTokenLexer.cpp:923-924
+      case '\r':
+        if (i + 1 < e && Text[i + 1] == '\n')
+          break;
+        LLVM_FALLTHROUGH;
----------------
`\r\n` is a newline on Windows.


================
Comment at: clang/lib/Format/FormatTokenLexer.cpp:930
+          FormatTok->HasUnescapedNewline = true;
+        InEscape = false;
         FormatTok->LastNewlineOffset = WhitespaceLength + i + 1;
----------------



================
Comment at: clang/lib/Format/FormatTokenLexer.cpp:946-948
+      case '?':
+      case '/':
+        InEscape = true;
----------------
Don't you need to look ahead for a `??/`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124748



More information about the cfe-commits mailing list