[clang-format] Allow to set token types final

[HazardyKnusperkeks]

Summary

We have a little problem. TokenAnnotator::resetTokenMetadata() resets the type, except for a (growing) whitelist. This is because the TokenAnnotator visits some tokens multiple times. E.g. trying to identify if a < is an operator less or a template opener. And in some runs, which are bascially "reverted" the types are reset.

On the other hand, if the parser does already know the type, it should be able to set it, without it being reset. So we introduce the ability to set a type and make that final.

[curdeius]

Great!
@krasimir, FYI, that should fix the issue you reported.

clang/lib/Format/FormatToken.h

379

Maybe we should assert here and in setType that: assert(!TypeIsFinalized);?

[HazardyKnusperkeks]

clang/lib/Format/FormatToken.h

379

In setType we could. Here we would have the same problem I fixed in D120512.

[owenpan]

clang/lib/Format/FormatToken.h

382

I thought you didn't like using the same name for both a variable and function. :)

[HazardyKnusperkeks]

• Format
• Add Assert
• And some stuff to deal with the assert - Should be removed in the long run
• Add some documentation

[curdeius]

clang/lib/Format/FormatToken.h

394

Nit, to make everyone happy, the function could be called isTypeFinalized.

[owenpan]

clang/lib/Format/FormatToken.h

394

+1.

[HazardyKnusperkeks]

Renamed member function.

clang/lib/Format/FormatToken.h

382

Naming a getter after the member you get should (nearly) always be done.

What I still stand to is having a local variable named after the function it is initialized from (this alone is fine), but then still calling the function in a loop. IsPrecededByCommentOrPPDirective is not the same as precededByCommentOrPPDirective() since the function relies on the changing token. I still find that very confusing, but for now I know that this code does not affect me while debugging, because I don't use the option.

[owenpan]

clang/lib/Format/FormatToken.h

382

Point taken.

clang/lib/Format/FormatToken.h

struct FormatToken {
   FormatToken()
       : HasUnescapedNewline(false), IsMultiline(false), IsFirst(false),
         MustBreakBefore(false), IsUnterminatedLiteral(false),
         CanBreakBefore(false), ClosesTemplateDeclaration(false),
         StartsBinaryExpression(false), EndsBinaryExpression(false),
         PartOfMultiVariableDeclStmt(false), ContinuesLineCommentSection(false),
         Finalized(false), ClosesRequiresClause(false), BlockKind(BK_Unknown),
         Decision(FD_Unformatted), PackingKind(PPK_Inconclusive),
-        Type(TT_Unknown) {}
+        TypeIsFinalized(false), Type(TT_Unknown) {}
 
   /// The \c Token.
   Token Tok;
 
   /// The raw text of the token.
   ///
   /// Contains the raw token text without leading whitespace and without leading
   /// escaped newlines.
ParameterPackingKind getPackingKind() const {
     return static_cast<ParameterPackingKind>(PackingKind);
   }
   void setPackingKind(ParameterPackingKind K) {
     PackingKind = K;
     assert(getPackingKind() == K && "ParameterPackingKind overflow!");
   }
 
 private:
+  unsigned TypeIsFinalized : 1;
   TokenType Type;
 
 public:
   /// Returns the token's type, e.g. whether "<" is a template opener or
   /// binary operator.
   TokenType getType() const { return Type; }
-  void setType(TokenType T) { Type = T; }
+  void setType(TokenType T) {
+    assert((!TypeIsFinalized || T == Type) &&
+           "Please use overwriteFixedType to change a fixed type.");
+    Type = T;
+  }
+  /// Sets the type and also the finalized flag. This prevents the type to be
+  /// reset in TokenAnnotator::resetTokenMetadata(). If the type needs to be set
+  /// to another one please use overwriteFixedType, or even better remove the
+  /// need to reassign the type.
+  void setFinalizedType(TokenType T) {
+    Type = T;
+    TypeIsFinalized = true;
+  }
+  void overwriteFixedType(TokenType T) {
+    TypeIsFinalized = false;
+    setType(T);
+  }
+  bool isTypeFinalized() const { return TypeIsFinalized; }
 
   /// The number of newlines immediately before the \c Token.
   ///
   /// This can be used to determine what the user wrote in the original code
   /// and thereby e.g. leave an empty line between two function definitions.
   unsigned NewlinesBefore = 0;
 
   /// The offset just past the last '\n' in this token's leading

clang/lib/Format/TokenAnnotator.cpp

while (CurrentToken) {
           if (!ColonFound && CurrentToken->Previous &&
               CurrentToken->Previous->is(TT_Unknown) &&
               canBeObjCSelectorComponent(*CurrentToken->Previous))
             CurrentToken->Previous->setType(TT_SelectorName);
           // determineStarAmpUsage() thinks that '*' '[' is allocating an
           // array of pointers, but if '[' starts a selector then '*' is a
           // binary operator.
           if (Parent && Parent->is(TT_PointerOrReference))
-            Parent->setType(TT_BinaryOperator);
+            Parent->overwriteFixedType(TT_BinaryOperator);
         }
         // An arrow after an ObjC method expression is not a lambda arrow.
         if (CurrentToken->getType() == TT_ObjCMethodExpr &&
             CurrentToken->Next && CurrentToken->Next->is(TT_LambdaArrow))
-          CurrentToken->Next->setType(TT_Unknown);
+          CurrentToken->Next->overwriteFixedType(TT_Unknown);
         Left->MatchingParen = CurrentToken;
         CurrentToken->MatchingParen = Left;
         // FirstObjCSelectorName is set when a colon is found. This does
         // not work, however, when the method has no parameters.
         // Here, we set FirstObjCSelectorName when the end of the method call is
         // reached, in case it was not set already.
         if (!Contexts.back().FirstObjCSelectorName) {
           FormatToken *Previous = CurrentToken->getPreviousNonComment();
if (CurrentToken) {
                 Previous->is(tok::string_literal))
               Previous->setType(TT_SelectorName);
           }
           if (CurrentToken->is(tok::colon) || Style.isJavaScript())
             Left->setType(TT_DictLiteral);
         }
         if (CurrentToken->is(tok::comma)) {
           if (Style.isJavaScript())
-            Left->setType(TT_DictLiteral);
+            Left->overwriteFixedType(TT_DictLiteral);
           ++CommaCount;
         }
         if (!consumeToken())
           return false;
       }
     }
     return true;
   }
private:
   }
 
   void resetTokenMetadata() {
     if (!CurrentToken)
       return;
 
     // Reset token type in case we have already looked at it and then
     // recovered from an error (e.g. failure to find the matching >).
-    if (!CurrentToken->isOneOf(
+    if (!CurrentToken->isTypeFinalized() &&
+        !CurrentToken->isOneOf(
             TT_LambdaLSquare, TT_LambdaLBrace, TT_AttributeMacro, TT_IfMacro,
             TT_ForEachMacro, TT_TypenameMacro, TT_FunctionLBrace,
             TT_ImplicitStringLiteral, TT_InlineASMBrace, TT_FatArrow,
             TT_LambdaArrow, TT_NamespaceMacro, TT_OverloadedOperator,
             TT_RegexLiteral, TT_TemplateString, TT_ObjCStringLiteral,
             TT_UntouchableMacroFunc, TT_StatementAttributeLikeMacro,
             TT_FunctionLikeOrFreestandingMacro, TT_ClassLBrace, TT_EnumLBrace,
             TT_RecordLBrace, TT_StructLBrace, TT_UnionLBrace, TT_RequiresClause,
             TT_RequiresClauseInARequiresExpression, TT_RequiresExpression,
             TT_RequiresExpressionLParen, TT_RequiresExpressionLBrace,
-            TT_BinaryOperator, TT_CompoundRequirementLBrace,
-            TT_BracedListLBrace))
+            TT_CompoundRequirementLBrace, TT_BracedListLBrace))
       CurrentToken->setType(TT_Unknown);
     CurrentToken->Role.reset();
     CurrentToken->MatchingParen = nullptr;
     CurrentToken->FakeLParens.clear();
     CurrentToken->FakeRParens = 0;
   }
 
   void next() {

clang/lib/Format/UnwrappedLineParser.cpp

do {
 
     switch (kind) {
     case tok::comment:
       nextToken();
       addUnwrappedLine();
       break;
     case tok::l_brace:
       if (NextLBracesType != TT_Unknown)
-        FormatTok->setType(NextLBracesType);
+        FormatTok->setFinalizedType(NextLBracesType);
       else if (FormatTok->Previous &&
                FormatTok->Previous->ClosesRequiresClause) {
         // We need the 'default' case here to correctly parse a function
         // l_brace.
         ParseDefault();
         continue;
       }
       if (CanContainBracedList && !FormatTok->is(TT_MacroBlockBegin) &&
static bool isC78ParameterDecl(const FormatToken *Tok, const FormatToken *Next,
 
   return Tok->Previous && Tok->Previous->isOneOf(tok::l_paren, tok::comma);
 }
 
 void UnwrappedLineParser::parseModuleImport() {
   nextToken();
   while (!eof()) {
     if (FormatTok->is(tok::colon)) {
-      FormatTok->setType(TT_ModulePartitionColon);
+      FormatTok->setFinalizedType(TT_ModulePartitionColon);
     }
     // Handle import <foo/bar.h> as we would an include statement.
     else if (FormatTok->is(tok::less)) {
       nextToken();
       while (!FormatTok->isOneOf(tok::semi, tok::greater, tok::eof)) {
         // Mark tokens up to the trailing line comments as implicit string
         // literals.
         if (FormatTok->isNot(tok::comment) &&
             !FormatTok->TokenText.startswith("//"))
-          FormatTok->setType(TT_ImplicitStringLiteral);
+          FormatTok->setFinalizedType(TT_ImplicitStringLiteral);
         nextToken();
       }
     }
     if (FormatTok->is(tok::semi)) {
       nextToken();
       break;
     }
     nextToken();
if (FormatTok->is(tok::string_literal))
       nextToken();
     addUnwrappedLine();
     return;
   }
   switch (FormatTok->Tok.getKind()) {
   case tok::kw_asm:
     nextToken();
     if (FormatTok->is(tok::l_brace)) {
-      FormatTok->setType(TT_InlineASMBrace);
+      FormatTok->setFinalizedType(TT_InlineASMBrace);
       nextToken();
       while (FormatTok && FormatTok->isNot(tok::eof)) {
         if (FormatTok->is(tok::r_brace)) {
-          FormatTok->setType(TT_InlineASMBrace);
+          FormatTok->setFinalizedType(TT_InlineASMBrace);
           nextToken();
           addUnwrappedLine();
           break;
         }
         FormatTok->Finalized = true;
         nextToken();
       }
     }
case tok::caret:
         nextToken();
       if (FormatTok->is(tok::l_paren))
         parseParens();
       if (FormatTok->is(tok::l_brace))
         parseChildBlock();
       break;
     case tok::l_brace:
       if (NextLBracesType != TT_Unknown)
-        FormatTok->setType(NextLBracesType);
+        FormatTok->setFinalizedType(NextLBracesType);
       if (!tryToParsePropertyAccessor() && !tryToParseBracedList()) {
         // A block outside of parentheses must be the last part of a
         // structural element.
         // FIXME: Figure out cases where this is not true, and add projections
         // for them (the one we know is missing are lambdas).
         if (Style.Language == FormatStyle::LK_Java &&
             Line->Tokens.front().Tok->is(Keywords.kw_synchronized)) {
           // If necessary, we could set the type to something different than
           // TT_FunctionLBrace.
           if (Style.BraceWrapping.AfterControlStatement ==
               FormatStyle::BWACS_Always)
             addUnwrappedLine();
         } else if (Style.BraceWrapping.AfterFunction) {
           addUnwrappedLine();
         }
         if (!Line->InPPDirective)
-          FormatTok->setType(TT_FunctionLBrace);
+          FormatTok->setFinalizedType(TT_FunctionLBrace);
         parseBlock();
         addUnwrappedLine();
         return;
       }
       // Otherwise this was a braced init list, and the structural
       // element continues.
       break;
     case tok::kw_try:
case tok::identifier: {
 
         bool FollowedByNewline =
             CommentsBeforeNextToken.empty()
                 ? FormatTok->NewlinesBefore > 0
                 : CommentsBeforeNextToken.front()->NewlinesBefore > 0;
 
         if (FollowedByNewline && (Text.size() >= 5 || FunctionLike) &&
             tokenCanStartNewLine(*FormatTok) && Text == Text.upper()) {
-          PreviousToken->setType(TT_FunctionLikeOrFreestandingMacro);
+          PreviousToken->setFinalizedType(TT_FunctionLikeOrFreestandingMacro);
           addUnwrappedLine();
           return;
         }
       }
       break;
     }
     case tok::equal:
       if ((Style.isJavaScript() || Style.isCSharp()) &&
case tok::kw_false:
         nextToken();
         break;
       }
       return true;
     case tok::arrow:
       // This might or might not actually be a lambda arrow (this could be an
       // ObjC method invocation followed by a dereferencing arrow). We might
       // reset this back to TT_Unknown in TokenAnnotator.
-      FormatTok->setType(TT_LambdaArrow);
+      FormatTok->setFinalizedType(TT_LambdaArrow);
       SeenArrow = true;
       nextToken();
       break;
     default:
       return true;
     }
   }
-  FormatTok->setType(TT_LambdaLBrace);
-  LSquare.setType(TT_LambdaLSquare);
+  FormatTok->setFinalizedType(TT_LambdaLBrace);
+  LSquare.setFinalizedType(TT_LambdaLSquare);
   parseChildBlock();
   return true;
 }
 
 bool UnwrappedLineParser::tryToParseLambdaIntroducer() {
   const FormatToken *Previous = FormatTok->Previous;
   if (Previous &&
       (Previous->isOneOf(tok::identifier, tok::kw_operator, tok::kw_new,
assert(FormatTok->is(Keywords.kw_function) ||
          FormatTok->startsSequence(Keywords.kw_async, Keywords.kw_function));
   if (FormatTok->is(Keywords.kw_async))
     nextToken();
   // Consume "function".
   nextToken();
 
   // Consume * (generator function). Treat it like C++'s overloaded operators.
   if (FormatTok->is(tok::star)) {
-    FormatTok->setType(TT_OverloadedOperator);
+    FormatTok->setFinalizedType(TT_OverloadedOperator);
     nextToken();
   }
 
   // Consume function name.
   if (FormatTok->is(tok::identifier))
     nextToken();
 
   if (FormatTok->isNot(tok::l_paren))
do {
     case tok::kw_requires: {
       auto RequiresToken = FormatTok;
       nextToken();
       parseRequiresExpression(RequiresToken);
       break;
     }
     case tok::ampamp:
       if (AmpAmpTokenType != TT_Unknown)
-        FormatTok->setType(AmpAmpTokenType);
+        FormatTok->setFinalizedType(AmpAmpTokenType);
       LLVM_FALLTHROUGH;
     default:
       nextToken();
       break;
     }
   } while (!eof());
 }
 
void UnwrappedLineParser::parseRequiresClause(FormatToken *RequiresToken) {
 
   // If there is no previous token, we are within a requires expression,
   // otherwise we will always have the template or function declaration in front
   // of it.
   bool InRequiresExpression =
       !RequiresToken->Previous ||
       RequiresToken->Previous->is(TT_RequiresExpressionLBrace);
 
-  RequiresToken->setType(InRequiresExpression
+  RequiresToken->setFinalizedType(InRequiresExpression
                                       ? TT_RequiresClauseInARequiresExpression
                                       : TT_RequiresClause);
 
   parseConstraintExpression();
 
   if (!InRequiresExpression)
     FormatTok->Previous->ClosesRequiresClause = true;
 }
 
 /// \brief Parses a requires expression.
 /// \param RequiresToken The requires keyword token, which starts this clause.
 /// \pre We need to be on the next token after the requires keyword.
 /// \sa parseRequiresClause
 ///
 /// Returns if it either has finished parsing the expression, or it detects,
 /// that the expression is incorrect.
 void UnwrappedLineParser::parseRequiresExpression(FormatToken *RequiresToken) {
   assert(FormatTok->getPreviousNonComment() == RequiresToken);
   assert(RequiresToken->is(tok::kw_requires) && "'requires' expected");
   assert(RequiresToken->getType() == TT_Unknown);
 
-  RequiresToken->setType(TT_RequiresExpression);
+  RequiresToken->setFinalizedType(TT_RequiresExpression);
 
   if (FormatTok->is(tok::l_paren)) {
-    FormatTok->setType(TT_RequiresExpressionLParen);
+    FormatTok->setFinalizedType(TT_RequiresExpressionLParen);
     parseParens();
   }
 
   if (FormatTok->is(tok::l_brace)) {
-    FormatTok->setType(TT_RequiresExpressionLBrace);
+    FormatTok->setFinalizedType(TT_RequiresExpressionLBrace);
     parseChildBlock(/*CanContainBracedList=*/false,
                     /*NextLBracesType=*/TT_CompoundRequirementLBrace);
   }
 }
 
 /// \brief Parses a constraint expression.
 ///
 /// This is either the definition of a concept, or the body of a requires
case tok::kw_union:
       return;
 
     case tok::l_brace:
       // Potential function body.
       return;
 
     case tok::ampamp:
     case tok::pipepipe:
-      FormatTok->setType(TT_BinaryOperator);
+      FormatTok->setFinalizedType(TT_BinaryOperator);
       nextToken();
       LambdaNextTimeAllowed = true;
       break;
 
     case tok::comma:
     case tok::comment:
       LambdaNextTimeAllowed = LambdaThisTimeAllowed;
       nextToken();
if (FormatTok->is(tok::identifier)) {
       if (Style.isCpp() && FormatTok->is(tok::identifier))
         return false;
     }
   }
 
   // Just a declaration or something is wrong.
   if (FormatTok->isNot(tok::l_brace))
     return true;
-  FormatTok->setType(TT_EnumLBrace);
+  FormatTok->setFinalizedType(TT_EnumLBrace);
   FormatTok->setBlockKind(BK_Block);
 
   if (Style.Language == FormatStyle::LK_Java) {
     // Java enums are different.
     parseJavaEnumBody();
     return true;
   }
   if (Style.Language == FormatStyle::LK_Proto) {
auto GetBraceType = [](const FormatToken &RecordTok) {
     case tok::kw_union:
       return TT_UnionLBrace;
     default:
       // Useful for e.g. interface.
       return TT_RecordLBrace;
     }
   };
   if (FormatTok->is(tok::l_brace)) {
-    FormatTok->setType(GetBraceType(InitialToken));
+    FormatTok->setFinalizedType(GetBraceType(InitialToken));
     if (ParseAsExpr) {
       parseChildBlock();
     } else {
       if (ShouldBreakBeforeBrace(Style, InitialToken))
         addUnwrappedLine();
 
       unsigned AddLevels = Style.IndentAccessModifiers ? 2u : 1u;
       parseBlock(/*MustBeDeclaration=*/true, AddLevels, /*MunchSemi=*/false);

clang/unittests/Format/TokenAnnotatorTest.cpp

TEST_F(TokenAnnotatorTest, UnderstandsUsesOfStarAndAmp) {
   EXPECT_TOKEN(Tokens[9], tok::identifier, TT_Unknown);
   EXPECT_TOKEN(Tokens[10], tok::r_paren, TT_TypeDeclarationParen);
   EXPECT_TOKEN(Tokens[11], tok::amp, TT_PointerOrReference);
 
   Tokens = annotate("#define lambda [](const decltype(x) *ptr) {}");
   EXPECT_EQ(Tokens.size(), 17u) << Tokens;
   EXPECT_TOKEN(Tokens[10], tok::r_paren, TT_TypeDeclarationParen);
   EXPECT_TOKEN(Tokens[11], tok::star, TT_PointerOrReference);
+
+  Tokens = annotate("void f() {\n"
+                    "  while (p < a && *p == 'a')\n"
+                    "    p++;\n"
+                    "}");
+  EXPECT_EQ(Tokens.size(), 21u) << Tokens;
+  EXPECT_TOKEN(Tokens[10], tok::ampamp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[11], tok::star, TT_UnaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsClasses) {
   auto Tokens = annotate("class C {};");
   EXPECT_EQ(Tokens.size(), 6u) << Tokens;
   EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_ClassLBrace);
 
   Tokens = annotate("const class C {} c;");