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

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.


================
Comment at: clang/lib/Format/TokenAnnotator.cpp:123-125
+  bool braceTokenMatchesScope(FormatToken &Token) {
+    if (!Token.is(tok::l_brace) || Scopes.empty())
+      return false;
----------------



================
Comment at: clang/lib/Format/TokenAnnotator.cpp:129
+    case TT_LambdaLBrace:
+      return Scopes.back() == ST_Function;
+    case TT_ClassLBrace:
----------------



================
Comment at: clang/lib/Format/TokenAnnotator.cpp:868
+        assert(!Scopes.empty());
+        assert(braceTokenMatchesScope(OpeningBrace));
+        Scopes.pop_back();
----------------



================
Comment at: clang/lib/Format/TokenAnnotator.cpp:1169-1181
+      switch (Tok->getType()) {
+      case TT_FunctionLBrace:
+      case TT_LambdaLBrace:
+        Scopes.push_back(ST_Function);
+        break;
+      case TT_ClassLBrace:
+      case TT_StructLBrace:
----------------



================
Comment at: clang/lib/Format/TokenAnnotator.cpp:2491
+    auto IsChainedOperatorAmpOrMember = [](const FormatToken *token) {
+      return token && token->isOneOf(tok::amp, tok::period, tok::arrow,
+                                     tok::arrowstar, tok::periodstar);
----------------
Sorry about that!


================
Comment at: clang/lib/Format/TokenAnnotator.cpp:2498-2499
+    if (Tok.is(tok::amp) && PrevToken && PrevToken->Tok.isAnyIdentifier() &&
+        (!PrevToken->getPreviousNonComment() ||
+         IsChainedOperatorAmpOrMember(PrevToken->getPreviousNonComment())) &&
+        NextToken && NextToken->Tok.isAnyIdentifier()) {
----------------
Now we only need the lambda.


================
Comment at: clang/lib/Format/TokenAnnotator.cpp:1195-1198
+      // Handle unbalanced braces.
+      if (!Scopes.empty())
+        Scopes.pop_back();
       // Lines can start with '}'.
----------------
dkt01 wrote:
> owenpan wrote:
> > I don't think it's about unbalanced braces here.
> `if (!Scopes.empty())` handles unbalanced braces.  `if(Tok->Previous)` handles the case where a line starts with an rbrace.
I can't think of an example. Do you have one?


================
Comment at: clang/lib/Format/TokenAnnotator.cpp:2481-2482
+    if (Tok.is(tok::amp) && (PrevToken && PrevToken->Tok.isAnyIdentifier()) &&
+        (!PrevToken->getPreviousNonComment() ||
+         IsChainedOperatorAmpOrMember(PrevToken->getPreviousNonComment())) &&
+        (NextToken && NextToken->Tok.isAnyIdentifier()) &&
----------------
dkt01 wrote:
> owenpan wrote:
> > The lambda would check that `PrevToken` is nonnull.
> `!PrevToken->getPreviousNonComment() ||` is a different check than the null check in the lambda.  It's acceptable to have no token two tokens prior because that indicates the ampersand is the second token of the line.
I actually wanted fold this into the lambda. (See above.)


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

https://reviews.llvm.org/D141959