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

[MyDeveloperDay via Phabricator via llvm-commits]

MyDeveloperDay added a comment.

@HazardyKnusperkeks  (its not without merits to add a "C" Language the more i think about it, but I'm not sure quite in the form presented here)

If we DID add a "C" language then we need to add something like you suggested  isCpp() already means more than we think.

  bool isCStyle() {
      return isCpp() || isC();
  }

Personally I really don't want to see the below change or it will change the world, so it would be better if the change was just from `!Style.isCpp()` to `!Style.isCStyle()`

  !Style.isCpp() && !Style.isC()

The ClangFormat.cpp changes are really unrelated and need to be removed (or resubmitted/discussed separately), and we really must have extensive unit tests if we are adding a new language (which this review doesn't have any) - (that means a new FormatCTests.cpp)

I'd personally want to see really limited use of `isCStyle()`  and ONLY what's needed to fix the exact example/tests we'd add. (its all to easy to find/replace thinking you are doing the right thing when actually we may not, it needs to be targetted changes)

But bottom line, I just don't see how we fix the .h problem? other than if the .clang-format section doesn't have a `Language: Cpp` then we don't assume its C++ (problem is it already does)

(actually this is a request that users can limit the guessing of languages to only those languages present in the .clang-format file)

This would be good for "C" projects but not much use for C++ or C++/C projects.

It wouldn't solve the `delete(foo);` issue this can still be a function or a delete of a bracketed variable, or a macro... so I don't see it helps

And bottom line the Lexer will give it to you as tok::kw_private and not as tok::identifier regardless of your language and so you'll need to perhaps do something in FormatTokenLexer to transform it back to an identifier (which is possible) checkout tryMergeLessLess() etc...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416