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

[Marek Kurdej via Phabricator via cfe-commits]

curdeius requested changes to this revision.
curdeius added inline comments.
This revision now requires changes to proceed.


================
Comment at: clang/lib/Format/FormatToken.h:125
 
+/// Sorted Operators that can follow a C variable.
+static const std::vector<clang::tok::TokenKind> COperatorsFollowingVar = {
----------------



================
Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:122-129
+        auto COperatorMatch = std::lower_bound(COperatorsFollowingVar.begin(),
+                                               COperatorsFollowingVar.end(),
+                                               RootToken.Next->Tok.getKind());
+        if ((COperatorMatch == COperatorsFollowingVar.end() ||
+             *COperatorMatch != RootToken.Next->Tok.getKind()) &&
+            !RootToken.Next->Tok.is(tok::coloncolon)) {
+          return true;
----------------
HazardyKnusperkeks wrote:
> Use `std::binary_search` instead of `std::lower_bound`. That should simplify the following `if`.
Please do something along these lines. The idea is to put a cheaper check first.

Note. I haven't tested nor formatted these lines.


================
Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:905
   /// break after the "{", format all lines with correct indentation and the put
-  /// the closing "}" on yet another new line.
+  ///  the closing "}" on yet another new line.
   ///
----------------
Revert.


================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2716-2718
+  auto COperatorMatch =
+      std::lower_bound(COperatorsFollowingVar.begin(),
+                       COperatorsFollowingVar.end(), FormatTok->Tok.getKind());
----------------
Please use `binary_search` and put it inside the `else` branch to avoid it if the first condition is satisfied.
Something like:
```
if (FormatTok->Tok.is(tok::colon)) {
...
} else if (!binary_search(...) {
} else if (...) {
}
```

Also, this code and the code in `UnwrappedLineFormatter` are pretty much similar.
Can we remove this duplication by e.g. setting the token kind here and checking it in the formatter?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416