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

MyDeveloperDay via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 19 01:30:07 PST 2022

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...

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list