[clang-format] Use an enum for context types. NFC

[sstwcw]

Summary

We currently have all those fields in AnnotatingParser::Context. They
are not inherited from the Context object for the parent scope. They
are exclusive. Now they are replaced with an enum.

InCpp11AttributeSpecifier and InCSharpAttributeSpecifier are not
handled like the rest in ContextType because they are not exclusive.

[owenpan]

If this patch is an NFC, please add [NFC] to the title.

Please add @MyDeveloperDay @curdeius @HazardyKnusperkeks to Reviewers.

clang/lib/Format/TokenAnnotator.cpp

116

If this was bug, it should be in a separate patch with test cases added.

[MyDeveloperDay]

So out of interest, what is the reason? my assumption is that you wanted to add more for Verilog and you felt adding the extra bools was the wrong design and its better an an enum

bool InCpp11AttributeSpecifier = false;
bool InCSharpAttributeSpecifier = false;

Does the fact that some aren't exclusive make you think its ok to split it into enums and bools is ok? (no real opinion just wondered what you and others think?)

[HazardyKnusperkeks]

In D121907#3390226, @MyDeveloperDay wrote:

So out of interest, what is the reason? my assumption is that you wanted to add more for Verilog and you felt adding the extra bools was the wrong design and its better an an enum

bool InCpp11AttributeSpecifier = false;
bool InCSharpAttributeSpecifier = false;

Does the fact that some aren't exclusive make you think its ok to split it into enums and bools is ok? (no real opinion just wondered what you and others think?)

It is really helpful to see that these are exclusive. It helps reasoning about the code.

clang/lib/Format/TokenAnnotator.cpp

1498

This is a comment for the change, afterwards it's strange to see for newcomers. Since it's without Context. *scnr*

[sstwcw]

Remove comment.

[MyDeveloperDay]

I think fundamentally this look ok? @curdeius and @owenpan I feel I want to bow to your better judgement.

[MyDeveloperDay]

Please wait for the other reviewers if you have commit access.

[sstwcw]

In D121907#3390226, @MyDeveloperDay wrote:

So out of interest, what is the reason? my assumption is that you wanted to add more for Verilog and you felt adding the extra bools was the wrong design and its better an an enum

You are right.

bool InCpp11AttributeSpecifier = false;
bool InCSharpAttributeSpecifier = false;

Does the fact that some aren't exclusive make you think its ok to split it into enums and bools is ok? (no real opinion just wondered what you and others think?)

Does it make me think it's ok? Yes. Good? No. I am lazy and I chose this way which doesn't require examining those two options.

clang/lib/Format/TokenAnnotator.cpp

116

Sorry that the previous patch did not include more context. Pun intended. Now you can scroll up and see that the context was just initialized, so InTemplateArgument starts being false, so it didn't matter whether the original code was InTemplateArgument = ...; or if (...) InTemplateArgument = true;.

[owenpan]

LGTM. Any comments, @curdeius?

clang/lib/Format/TokenAnnotator.cpp

116

Got it. Thanks.

[curdeius]

I really like this change. Thanks!

[MyDeveloperDay]

LGTM

clang/lib/Format/TokenAnnotator.cpp

bool parseAngle() {
 
     // If this angle is in the context of an expression, we need to be more
     // hesitant to detect it as opening template parameters.
     bool InExprContext = Contexts.back().IsExpression;
 
     Contexts.back().IsExpression = false;
     // If there's a template keyword before the opening angle bracket, this is a
     // template parameter, not an argument.
-    Contexts.back().InTemplateArgument =
-        Left->Previous && Left->Previous->isNot(tok::kw_template);
+    if (Left->Previous && Left->Previous->isNot(tok::kw_template))
+      Contexts.back().ContextType = Context::TemplateArgument;
 
     if (Style.Language == FormatStyle::LK_Java &&
         CurrentToken->is(tok::question))
       next();
 
     while (CurrentToken) {
       if (CurrentToken->is(tok::greater)) {
         // Try to do a better job at looking for ">>" within the condition of
if (OpeningParen.is(TT_OverloadedOperatorLParen)) {
                 !OpeningParen.Previous->is(tok::identifier))) {
       Contexts.back().IsExpression = true;
     } else if (Contexts[Contexts.size() - 2].CaretFound) {
       // This is the parameter list of an ObjC block.
       Contexts.back().IsExpression = false;
     } else if (OpeningParen.Previous &&
                OpeningParen.Previous->is(TT_ForEachMacro)) {
       // The first argument to a foreach macro is a declaration.
-      Contexts.back().IsForEachMacro = true;
+      Contexts.back().ContextType = Context::ForEachMacro;
       Contexts.back().IsExpression = false;
     } else if (OpeningParen.Previous && OpeningParen.Previous->MatchingParen &&
                OpeningParen.Previous->MatchingParen->is(TT_ObjCBlockLParen)) {
       Contexts.back().IsExpression = false;
     } else if (!Line.MustBeDeclaration && !Line.InPPDirective) {
       bool IsForOrCatch =
           OpeningParen.Previous &&
           OpeningParen.Previous->isOneOf(tok::kw_for, tok::kw_catch);
bool parseSquare() {
     FormatToken *Parent = Left->getPreviousNonComment();
 
     // Cases where '>' is followed by '['.
     // In C++, this can happen either in array of templates (foo<int>[10])
     // or when array is a nested template type (unique_ptr<type1<type2>[]>).
     bool CppArrayTemplates =
         Style.isCpp() && Parent && Parent->is(TT_TemplateCloser) &&
         (Contexts.back().CanBeExpression || Contexts.back().IsExpression ||
-         Contexts.back().InTemplateArgument);
+         Contexts.back().ContextType == Context::TemplateArgument);
 
     bool IsCpp11AttributeSpecifier = isCpp11AttributeSpecifier(*Left) ||
                                      Contexts.back().InCpp11AttributeSpecifier;
 
     // Treat C# Attributes [STAThread] much like C++ attributes [[...]].
     bool IsCSharpAttributeSpecifier =
         isCSharpAttributeSpecifier(*Left) ||
         Contexts.back().InCSharpAttributeSpecifier;
bool parseBrace() {
     while (CurrentToken) {
       if (CurrentToken->is(tok::r_brace)) {
         assert(OpeningBrace.Optional == CurrentToken->Optional);
         OpeningBrace.MatchingParen = CurrentToken;
         CurrentToken->MatchingParen = &OpeningBrace;
         if (Style.AlignArrayOfStructures != FormatStyle::AIAS_None) {
           if (OpeningBrace.ParentBracket == tok::l_brace &&
               couldBeInStructArrayInitializer() && CommaCount > 0)
-            Contexts.back().InStructArrayInitializer = true;
+            Contexts.back().ContextType = Context::StructArrayInitializer;
         }
         next();
         return true;
       }
       if (CurrentToken->isOneOf(tok::r_paren, tok::r_square))
         return false;
       updateParameterCount(&OpeningBrace, CurrentToken);
       if (CurrentToken->isOneOf(tok::colon, tok::l_brace, tok::less)) {
case tok::question:
         }
       }
       parseConditional();
       break;
     case tok::kw_template:
       parseTemplateDeclaration();
       break;
     case tok::comma:
-      if (Contexts.back().InCtorInitializer)
+      switch (Contexts.back().ContextType) {
+      case Context::CtorInitializer:
         Tok->setType(TT_CtorInitializerComma);
-      else if (Contexts.back().InInheritanceList)
+        break;
+      case Context::InheritanceList:
         Tok->setType(TT_InheritanceComma);
-      else if (Contexts.back().FirstStartOfName &&
-               (Contexts.size() == 1 || startsWithInitStatement(Line))) {
+        break;
+      default:
+        if (Contexts.back().FirstStartOfName &&
           Contexts.back().FirstStartOfName->PartOfMultiVariableDeclStmt = true;
           Line.IsMultiVariableDeclStmt = true;
         }
-      if (Contexts.back().IsForEachMacro)
+        break;
+      }
+      if (Contexts.back().ContextType == Context::ForEachMacro)
         Contexts.back().IsExpression = true;
       break;
     case tok::identifier:
       if (Tok->isOneOf(Keywords.kw___has_include,
                        Keywords.kw___has_include_next))
         parseHasInclude();
       if (Style.isCSharp() && Tok->is(Keywords.kw_where) && Tok->Next &&
           Tok->Next->isNot(tok::l_paren)) {
LineType parseLine() {
     if (Line.startsWith(TT_ObjCMethodSpecifier)) {
       if (Contexts.back().FirstObjCSelectorName)
         Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName =
             Contexts.back().LongestObjCSelectorName;
       return LT_ObjCMethodDecl;
     }
 
     for (const auto &ctx : Contexts)
-      if (ctx.InStructArrayInitializer)
+      if (ctx.ContextType == Context::StructArrayInitializer)
         return LT_ArrayOfStructInitializer;
 
     return LT_Other;
   }
 
 private:
   bool isClosureImportStatement(const FormatToken &Tok) {
     // FIXME: Closure-library specific stuff should not be hard-coded but be
struct Context {
     bool IsExpression;
     unsigned LongestObjCSelectorName = 0;
     bool ColonIsForRangeExpr = false;
     bool ColonIsDictLiteral = false;
     bool ColonIsObjCMethodExpr = false;
     FormatToken *FirstObjCSelectorName = nullptr;
     FormatToken *FirstStartOfName = nullptr;
     bool CanBeExpression = true;
-    bool InTemplateArgument = false;
-    bool InCtorInitializer = false;
-    bool InInheritanceList = false;
     bool CaretFound = false;
-    bool IsForEachMacro = false;
     bool InCpp11AttributeSpecifier = false;
     bool InCSharpAttributeSpecifier = false;
-    bool InStructArrayInitializer = false;
+    enum {
+      Unknown,
+      // Like the part after `:` in a constructor.
+      //   Context(...) : IsExpression(IsExpression)
+      CtorInitializer,
+      // Like in the parentheses in a foreach.
+      ForEachMacro,
+      // Like the inheritance list in a class declaration.
+      //   class Input : public IO
+      InheritanceList,
+      // Like in the braced list.
+      //   int x[] = {};
+      StructArrayInitializer,
+      // Like in `static_cast<int>`.
+      TemplateArgument,
+    } ContextType = Unknown;
   };
 
   /// Puts a new \c Context onto the stack \c Contexts for the lifetime
   /// of each instance.
   struct ScopedContextCreator {
     AnnotatingParser &P;
 
     ScopedContextCreator(AnnotatingParser &P, tok::TokenKind ContextKind,
                          unsigned Increase)
         : P(P) {
       P.Contexts.push_back(Context(ContextKind,
                                    P.Contexts.back().BindingStrength + Increase,
                                    P.Contexts.back().IsExpression));
     }
 
     ~ScopedContextCreator() {
       if (P.Style.AlignArrayOfStructures != FormatStyle::AIAS_None) {
-        if (P.Contexts.back().InStructArrayInitializer) {
+        if (P.Contexts.back().ContextType == Context::StructArrayInitializer) {
           P.Contexts.pop_back();
-          P.Contexts.back().InStructArrayInitializer = true;
+          P.Contexts.back().ContextType = Context::StructArrayInitializer;
           return;
         }
       }
       P.Contexts.pop_back();
     }
   };
 
   void modifyContext(const FormatToken &Current) {
if (AssignmentStartsExpression()) {
       Contexts.back().IsExpression = true;
     } else if (Current.is(TT_TrailingReturnArrow)) {
       Contexts.back().IsExpression = false;
     } else if (Current.is(TT_LambdaArrow) || Current.is(Keywords.kw_assert)) {
       Contexts.back().IsExpression = Style.Language == FormatStyle::LK_Java;
     } else if (Current.Previous &&
                Current.Previous->is(TT_CtorInitializerColon)) {
       Contexts.back().IsExpression = true;
-      Contexts.back().InCtorInitializer = true;
+      Contexts.back().ContextType = Context::CtorInitializer;
     } else if (Current.Previous && Current.Previous->is(TT_InheritanceColon)) {
-      Contexts.back().InInheritanceList = true;
+      Contexts.back().ContextType = Context::InheritanceList;
     } else if (Current.isOneOf(tok::r_paren, tok::greater, tok::comma)) {
       for (FormatToken *Previous = Current.Previous;
            Previous && Previous->isOneOf(tok::star, tok::amp);
            Previous = Previous->Previous)
         Previous->setType(TT_PointerOrReference);
-      if (Line.MustBeDeclaration && !Contexts.front().InCtorInitializer)
+      if (Line.MustBeDeclaration &&
+          Contexts.front().ContextType != Context::CtorInitializer)
         Contexts.back().IsExpression = false;
     } else if (Current.is(tok::kw_new)) {
       Contexts.back().CanBeExpression = false;
     } else if (Current.is(tok::semi) ||
                (Current.is(tok::exclaim) && Current.Previous &&
                 !Current.Previous->is(tok::kw_operator))) {
       // This should be the condition or increment in a for-loop.
       // But not operator !() (can't use TT_OverloadedOperator here as its not
if (Current.is(Keywords.kw_instanceof)) {
       Current.setType(TT_TrailingReturnArrow);
     } else if (isDeductionGuide(Current)) {
       // Deduction guides trailing arrow " A(...) -> A<T>;".
       Current.setType(TT_TrailingReturnArrow);
     } else if (Current.isOneOf(tok::star, tok::amp, tok::ampamp)) {
       Current.setType(determineStarAmpUsage(
           Current,
           Contexts.back().CanBeExpression && Contexts.back().IsExpression,
-          Contexts.back().InTemplateArgument));
+          Contexts.back().ContextType == Context::TemplateArgument));
     } else if (Current.isOneOf(tok::minus, tok::plus, tok::caret)) {
       Current.setType(determinePlusMinusCaretUsage(Current));
       if (Current.is(TT_UnaryOperator) && Current.is(tok::caret))
         Contexts.back().CaretFound = true;
     } else if (Current.isOneOf(tok::minusminus, tok::plusplus)) {
       Current.setType(determineIncrementUsage(Current));
     } else if (Current.isOneOf(tok::exclaim, tok::tilde)) {
       Current.setType(TT_UnaryOperator);