[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

Marek Kurdej via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Jan 16 07:19:18 PST 2022


curdeius requested changes to this revision.
curdeius added a comment.

Thanks for having a try on this.
However, I don't like this approach too much. You add many changes and a single test. That's not sufficient.
Also, handling C++ keywords in all cases (e.g. `delete` as a function name) *may* need to distinguish whether we format a C file or a C++ file. It's probably impossible to do this without user input (.h extension is used in both languages for example).
We'd maybe need to add C as language option and let the user specify the language (`-x c`?).
That in turn may be painful (because not automatic).
But, you may have a better solution.
My 2 cents.



================
Comment at: .arclint:5
       "type": "script-and-regex",
-      "script-and-regex.script": "bash utils/arcanist/clang-format.sh",
+      "script-and-regex.script": "cmd utils\\arcanist\\clang-format.sh",
       "script-and-regex.regex": "/^(?P<severity>[[:alpha:]]+)\n(?P<message>[^\n]+)\n(====|(?P<line>\\d),(?P<char>\\d)\n(?P<original>.*)>>>>\n(?P<replacement>.*)<<<<\n)$/s",
----------------
Unrelated. Please revert.


================
Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:103
       return 0;
-    if (RootToken.isAccessSpecifier(false) ||
+    if (RootToken.isAccessSpecifier(Style.Language == FormatStyle::LK_Cpp) ||
+        (RootToken.Next &&
----------------
Please refactor to e.g. a lambda to ease the understanding. And use a series of `if`s and returns.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416



More information about the cfe-commits mailing list