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

[Björn Schäpers via Phabricator via llvm-commits]

HazardyKnusperkeks added a comment.

In D117416#3250415 <https://reviews.llvm.org/D117416#3250415>, @MyDeveloperDay wrote:

> I'm not a fan of this approach of adding a "C" language, mainly because of the `.h` problem so ultimately it doesn't solve your problem.
>
> I think this is overkill for what is basically the subtle handling of "public/private/protected/try/delete/new" being used as variables (which frankly is at least in my view not a bright idea in the first place!)
>
> I think ultimately we can try to catch those corner cases but IMHO it doesn't need the addition of a whole new language for that, (we are not at that much on an impasse)
>
> This review contains .arclint that needs removing, this review contains no tests

I was actually thinking that adding C as language is the right way, and still am. We have stuff only for arcane C variants, why not push these in the language C and keep C++ free from it?

But I'm fine with either solution.



================
Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:100
   int getIndentOffset(const FormatToken &RootToken) {
-    if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
+    if (Style.Language == FormatStyle::LK_C ||
+        Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
----------------
MyDeveloperDay wrote:
> isC() these large OR/AND conditions become unreadable
I'm more of a `switch` fan and not your isXXX(), but that's preference. ;)


================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1606
 
-      if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+      if ((Style.isCpp() || Style.isC()) && FormatTok->is(TT_StatementMacro)) {
         parseStatementMacro();
----------------
MyDeveloperDay wrote:
> I feel everywhere you put isCpp you'll end up doing isCpp() || is C()
> 
> This goes back to the arguments for a review I tried before {D80079} its really isCStyle() if we WERE going to pursue this, I'd want that and not have clauses added everywhere
I think one could use `isC()`, `isCpp()`, `isObjC()`, and what is now `isCpp()`: `isCish()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416