[PATCH] D141959: [clang-format] Fix inconsistent identification of operator&

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

HazardyKnusperkeks added a comment.

Could this be added to `Context` instead of a new type?



================
Comment at: clang/lib/Format/TokenAnnotator.cpp:1205
     case tok::r_brace:
+      if (Scopes.size() > 1)
+        Scopes.pop_back();
----------------
How does this happen? The only `push` I can see is before `parseBrace`, where a `pop` is following directly after.


================
Comment at: clang/lib/Format/TokenAnnotator.cpp:1674
 
+  enum class ScopeType {
+    // Not contained within scope block
----------------



================
Comment at: clang/lib/Format/TokenAnnotator.cpp:1675
+  enum class ScopeType {
+    // Not contained within scope block
+    None,
----------------
Full stop at the end of your comments.


================
Comment at: clang/lib/Format/TokenAnnotator.cpp:1937
       Current.setType(determineStarAmpUsage(
-          Current,
+          Current, Scopes.back(),
           Contexts.back().CanBeExpression && Contexts.back().IsExpression,
----------------
You don't need to pass that, you can access it directly from `determineStarAmpUsage`, right?


================
Comment at: clang/lib/Format/TokenAnnotator.cpp:2385
+  TokenType determineStarAmpUsage(const FormatToken &Tok,
+                                  const ScopeType TokScope, bool IsExpression,
                                   bool InTemplateArgument) {
----------------
That `const` doesn't seem fit with the LLVM style.


================
Comment at: clang/lib/Format/TokenAnnotator.cpp:2385
+  TokenType determineStarAmpUsage(const FormatToken &Tok,
+                                  const ScopeType TokScope, bool IsExpression,
                                   bool InTemplateArgument) {
----------------
HazardyKnusperkeks wrote:
> That `const` doesn't seem fit with the LLVM style.
I'd prefer you write it out.


================
Comment at: clang/lib/Format/TokenAnnotator.cpp:2490-2491
+    // Opeartors at class scope are likely pointer or reference members
+    if (TokScope == ScopeType::Class)
+      return TT_PointerOrReference;
+
----------------
What about
```
struct S {
  auto Mem = C & D;
};
```
That would be annotated, or is there another rule above with would kick in? If there isn't a test with such a construct already please add one.


================
Comment at: clang/lib/Format/TokenAnnotator.cpp:2496-2502
+        ((PrevToken->getPreviousNonComment() &&
+          (PrevToken->getPreviousNonComment()->is(tok::amp) ||
+           PrevToken->getPreviousNonComment()->is(tok::period) ||
+           PrevToken->getPreviousNonComment()->is(tok::arrow) ||
+           PrevToken->getPreviousNonComment()->is(tok::arrowstar) ||
+           PrevToken->getPreviousNonComment()->is(tok::periodstar))) ||
+         !PrevToken->getPreviousNonComment()) &&
----------------
That seems better.


================
Comment at: clang/lib/Format/TokenAnnotator.cpp:2544
   SmallVector<Context, 8> Contexts;
+  static SmallVector<ScopeType, 8> Scopes;
 
----------------
Why static?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141959