[clang-format] Properly handle the C11 _Generic keyword.

[rymiel]

Summary

This patch properly recognizes the generic selection expression
introduced in C11, by adding an additional token type for the colons
present in such expressions.

Previously, they would be recognized as
"inline ASM colons" purely by the fact that those are the last thing
checked for.

I tried to avoid adding an addition token type, but since colons by
default like having spaces around them, I chose to add a new type so
that no space is added after the type selector.

Currently, no aspect of the formatting of these expressions in able to
be configured, as I'm not sure what could even be configured here.

One notable thing is that association list is always formatted as
either entirely on one line, if it can fit, or with line breaks
after every comma in the expression (also after the controlling expr.)

This visually makes them more similar to switch statements when long,
matching the behaviour of the selection expression, being that of a sort
of switch on types, but also allows for terseness when only selecting
for a few things.

Fixes https://github.com/llvm/llvm-project/issues/18080

[rymiel]

I put WIP in the title right now because firstly, I still need to write more tests, but also, since I haven't touched the breaking and indenting part of the code before, I want to make sure what I'm doing is sensible.

Basically, after making the colons their own type so that they no longer followed the rules of InlineASMColon, I noticed the output was perfect on its own if passed BinPackArgument: false as a style option. Following this train of thought, I simply let generic selection expressions act as if regular call-like expressions, but with binpack always forcefully disabled.

Not sure if that's something that should even be done (i.e. leave it just up to BinPackArgument), or configurable, or etc.

[HazardyKnusperkeks]

Looks good to me.
And adding a token type is absolutely nothing negative. The more tokens get a type, the better.

clang/lib/Format/TokenAnnotator.cpp

1632

[MyDeveloperDay]

clang/lib/Format/ContinuationIndenter.cpp

1699

I wonder if this says isJavaScript() here because of the

foo(name: type,name2: type2) syntax

if that might be the case then given the format you present in your test then maybe it IS correct to have GenericSelection here.

[rymiel]

Going to try to make the indentation of the cases following the controlling expression not continuations, since that results in weird results such as:

#define LIMIT_MAX(T)                     \
  _Generic(                              \
      ((T)0),                            \
      unsigned int: UINT_MAX,            \
      unsigned long: ULONG_MAX,          \
      unsigned long long: ULLONG_MAX)

My desired output is instead:

#define LIMIT_MAX(T)                     \
  _Generic(((T)0),                       \
      unsigned int: UINT_MAX,            \
      unsigned long: ULONG_MAX,          \
      unsigned long long: ULLONG_MAX)

or something similar, but I'm struggling with unfamiliar parts of the codebase and taking my time getting familiar with them.

[rymiel]

Change indenting behaviour

I'm still not sure what I'm doing regarding ContinuationIndenter, since I don't fully understand its mechanisms, so I just followed my usual method of "stick a very specific check in a random spot that makes the correct fields have the correct value", which feels like a hack here. There is probably a much better way to do this, perhaps something with fake parens or something, to actually treat the "controlling expression" and the actual selectors differently, but right now I've just put in a specific check to dedent the selectors to be relative to the _Generic keyword, and not aligned to the opening paren.

It is also no surprise that I don't know how to write tests; I included organic examples of cases that are semi-realistic use cases of _Generic, but there are definitely edge cases I don't have listed.

clang/lib/Format/ContinuationIndenter.cpp

if (!Current.isTrailingComment() &&
         (!Previous || Previous->isNot(tok::kw_return) ||
          (Style.Language != FormatStyle::LK_Java && PrecedenceLevel > 0)) &&
         (Style.AlignAfterOpenBracket != FormatStyle::BAS_DontAlign ||
          PrecedenceLevel != prec::Comma || Current.NestingLevel == 0)) {
       NewParenState.Indent = std::max(
           std::max(State.Column, NewParenState.Indent), CurrentState.LastSpace);
     }
 
+    // Special case for generic selection expressions, its comma-separated
+    // expressions are not aligned to the opening paren like regular calls, but
+    // rather continuation-indented relative to the _Generic keyword.
+    if (Previous && Previous->endsSequence(tok::l_paren, tok::kw__Generic))
+      NewParenState.Indent = CurrentState.LastSpace;
+
     if (Previous &&
         (Previous->getPrecedence() == prec::Assignment ||
          Previous->isOneOf(tok::kw_return, TT_RequiresClause) ||
          (PrecedenceLevel == prec::Conditional && Previous->is(tok::question) &&
           Previous->is(TT_ConditionalExpr))) &&
         !Newline) {
       // If BreakBeforeBinaryOperators is set, un-indent a bit to account for
       // the operator and keep the operands aligned.
bool ObjCBinPackProtocolList =
         (Style.ObjCBinPackProtocolList == FormatStyle::BPS_Auto &&
          Style.BinPackParameters) ||
         Style.ObjCBinPackProtocolList == FormatStyle::BPS_Always;
 
     bool BinPackDeclaration =
         (State.Line->Type != LT_ObjCDecl && Style.BinPackParameters) ||
         (State.Line->Type == LT_ObjCDecl && ObjCBinPackProtocolList);
 
+    bool GenericSelection =
+        Current.getPreviousNonComment() &&
+        Current.getPreviousNonComment()->is(tok::kw__Generic);
+
     AvoidBinPacking =
-        (CurrentState.IsCSharpGenericTypeConstraint) ||
+        (CurrentState.IsCSharpGenericTypeConstraint) || GenericSelection ||
         (Style.isJavaScript() && EndsInComma) ||
         (State.Line->MustBeDeclaration && !BinPackDeclaration) ||
         (!State.Line->MustBeDeclaration && !Style.BinPackArguments) ||
         (Style.ExperimentalAutoDetectBinPacking &&
          (Current.is(PPK_OnePerLine) ||
           (!BinPackInconclusiveFunctions && Current.is(PPK_Inconclusive))));
 
     if (Current.is(TT_ObjCMethodExpr) && Current.MatchingParen &&
         Style.ObjCBreakBeforeNestedBlockParam) {

clang/lib/Format/FormatToken.h

#define LIST_TOKEN_TYPES                                                       \
   TYPE(EnumLBrace)                                                             \
   TYPE(FatArrow)                                                               \
   TYPE(ForEachMacro)                                                           \
   TYPE(FunctionAnnotationRParen)                                               \
   TYPE(FunctionDeclarationName)                                                \
   TYPE(FunctionLBrace)                                                         \
   TYPE(FunctionLikeOrFreestandingMacro)                                        \
   TYPE(FunctionTypeLParen)                                                     \
+  /* The colons as part of a C11 _Generic selection */                         \
+  TYPE(GenericSelectionColon)                                                  \
   /* The colon at the end of a goto label or a case label. Currently only used \
    * for Verilog. */                                                           \
   TYPE(GotoLabelColon)                                                         \
   TYPE(IfMacro)                                                                \
   TYPE(ImplicitStringLiteral)                                                  \
   TYPE(InheritanceColon)                                                       \
   TYPE(InheritanceComma)                                                       \
   TYPE(InlineASMBrace)                                                         \

clang/lib/Format/TokenAnnotator.cpp

if (OpeningParen.is(TT_OverloadedOperatorLParen)) {
                OpeningParen.Previous->is(TT_JsTypeColon)) {
       // let x: (SomeType);
       Contexts.back().IsExpression = false;
     } else if (isLambdaParameterList(&OpeningParen)) {
       // This is a parameter list of a lambda expression.
       Contexts.back().IsExpression = false;
     } else if (OpeningParen.is(TT_RequiresExpressionLParen)) {
       Contexts.back().IsExpression = false;
+    } else if (OpeningParen.Previous &&
+               OpeningParen.Previous->is(tok::kw__Generic)) {
+      Contexts.back().ContextType = Context::C11GenericSelection;
+      Contexts.back().IsExpression = true;
     } else if (Line.InPPDirective &&
                (!OpeningParen.Previous ||
                 !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 &&
case tok::colon:
                 Tok->Previous->ColumnWidth;
           }
           Tok->Previous->ParameterIndex =
               Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts;
           ++Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts;
         }
       } else if (Contexts.back().ColonIsForRangeExpr) {
         Tok->setType(TT_RangeBasedForLoopColon);
+      } else if (Contexts.back().ContextType == Context::C11GenericSelection) {
+        Tok->setType(TT_GenericSelectionColon);
       } else if (CurrentToken && CurrentToken->is(tok::numeric_constant)) {
         Tok->setType(TT_BitFieldColon);
       } else if (Contexts.size() == 1 &&
                  !Line.First->isOneOf(tok::kw_enum, tok::kw_case,
                                       tok::kw_default)) {
         FormatToken *Prev = Tok->getPreviousNonComment();
         if (!Prev)
           break;
enum {
       // 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,
+      // C11 _Generic selection.
+      C11GenericSelection,
     } ContextType = Unknown;
   };
 
   /// Puts a new \c Context onto the stack \c Contexts for the lifetime
   /// of each instance.
   struct ScopedContextCreator {
     AnnotatingParser &P;
 
unsigned TokenAnnotator::splitPenalty(const AnnotatedLine &Line,
 
   if (Left.is(tok::l_paren) && Style.PenaltyBreakOpenParenthesis != 0)
     return Style.PenaltyBreakOpenParenthesis;
   if (Left.is(tok::l_paren) && InFunctionDecl &&
       Style.AlignAfterOpenBracket != FormatStyle::BAS_DontAlign) {
     return 100;
   }
   if (Left.is(tok::l_paren) && Left.Previous &&
-      (Left.Previous->is(tok::kw_for) || Left.Previous->isIf())) {
+      (Left.Previous->isOneOf(tok::kw_for, tok::kw__Generic) ||
+       Left.Previous->isIf())) {
     return 1000;
   }
   if (Left.is(tok::equal) && InFunctionDecl)
     return 110;
   if (Right.is(tok::r_brace))
     return 1;
   if (Left.is(TT_TemplateOpener))
     return 100;
if (Right.is(tok::colon)) {
     if (Right.is(TT_InlineASMColon) && Left.is(tok::coloncolon))
       return false;
     if (Right.is(TT_DictLiteral))
       return Style.SpacesInContainerLiterals;
     if (Right.is(TT_AttributeColon))
       return false;
     if (Right.is(TT_CSharpNamedArgumentColon))
       return false;
+    if (Right.is(TT_GenericSelectionColon))
+      return false;
     if (Right.is(TT_BitFieldColon)) {
       return Style.BitFieldColonSpacing == FormatStyle::BFCS_Both ||
              Style.BitFieldColonSpacing == FormatStyle::BFCS_Before;
     }
     return true;
   }
   // Do not merge "- -" into "--".
   if ((Left.isOneOf(tok::minus, tok::minusminus) &&

clang/unittests/Format/FormatTest.cpp

TEST_F(FormatTest, AtomicQualifier) {
   Style.SpacesInParentheses = false;
   verifyFormat("x = ( _Atomic(uint64_t) )*a;", Style);
   Style.SpacesInCStyleCastParentheses = false;
   Style.SpacesInParentheses = true;
   verifyFormat("x = (_Atomic( uint64_t ))*a;", Style);
   verifyFormat("x = (_Atomic( uint64_t ))&a;", Style);
 }
 
+TEST_F(FormatTest, C11Generic) {
+  verifyFormat("_Generic(x, int: 1, default: 0)");
+  verifyFormat("#define cbrt(X) _Generic((X), float: cbrtf, default: cbrt)(X)");
+  verifyFormat("_Generic(x, const char *: 1, char *const: 16, int: 8);");
+  verifyFormat("_Generic(x, int: f1, const int: f2)();");
+  verifyFormat("_Generic(x, struct A: 1, void (*)(void): 2);");
+
+  verifyFormat("_Generic(x,\n"
+               "    float: f,\n"
+               "    default: d,\n"
+               "    long double: ld,\n"
+               "    float _Complex: fc,\n"
+               "    double _Complex: dc,\n"
+               "    long double _Complex: ldc)");
+
+  FormatStyle Style = getLLVMStyle();
+  Style.ColumnLimit = 40;
+  verifyFormat("#define LIMIT_MAX(T)                   \\\n"
+               "  _Generic(((T)0),                     \\\n"
+               "      unsigned int: UINT_MAX,          \\\n"
+               "      unsigned long: ULONG_MAX,        \\\n"
+               "      unsigned long long: ULLONG_MAX)",
+               Style);
+  verifyFormat("_Generic(x,\n"
+               "    struct A: 1,\n"
+               "    void (*)(void): 2);",
+               Style);
+
+  Style.ContinuationIndentWidth = 2;
+  verifyFormat("_Generic(x,\n"
+               "  struct A: 1,\n"
+               "  void (*)(void): 2);",
+               Style);
+}
+
 TEST_F(FormatTest, AmbersandInLamda) {
   // Test case reported in https://bugs.llvm.org/show_bug.cgi?id=41899
   FormatStyle AlignStyle = getLLVMStyle();
   AlignStyle.PointerAlignment = FormatStyle::PAS_Left;
   verifyFormat("auto lambda = [&a = a]() { a = 2; };", AlignStyle);
   AlignStyle.PointerAlignment = FormatStyle::PAS_Right;
   verifyFormat("auto lambda = [&a = a]() { a = 2; };", AlignStyle);
 }

clang/unittests/Format/TokenAnnotatorTest.cpp

TEST_F(TokenAnnotatorTest, UnderstandsFunctionDeclarationNames) {
   ASSERT_EQ(Tokens.size(), 11u) << Tokens;
   EXPECT_TOKEN(Tokens[1], tok::identifier, TT_FunctionDeclarationName);
 
   Tokens = annotate("void f [[noreturn]] () {}");
   ASSERT_EQ(Tokens.size(), 12u) << Tokens;
   EXPECT_TOKEN(Tokens[1], tok::identifier, TT_FunctionDeclarationName);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsC11GenericSelection) {
+  auto Tokens = annotate("_Generic(x, int: 1, default: 0)");
+  ASSERT_EQ(Tokens.size(), 13u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::kw__Generic, TT_Unknown);
+  EXPECT_TOKEN(Tokens[5], tok::colon, TT_GenericSelectionColon);
+  EXPECT_TOKEN(Tokens[9], tok::colon, TT_GenericSelectionColon);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsVerilogOperators) {
   auto Annotate = [this](llvm::StringRef Code) {
     return annotate(Code, getLLVMStyle(FormatStyle::LK_Verilog));
   };
   // Test that unary operators get labeled as such and that operators like '++'
   // don't get split.
   tok::TokenKind Unary[] = {tok::plus,  tok::minus,    tok::exclaim,
                             tok::tilde, tok::amp,      tok::pipe,