clang-format: Space and triple angle braces

[jpienaar]

Summary

clang-format changes "<<<" to "<< <" which is problematic for CUDA code. Change the behavior of clang-format to operate correctly with CUDA kernel launches without changing language options.

• No space required between "<<" and "<" in "<<<".
• No space required between identifier and "<<<". This avoids changing "f<<<" to "f <<<"
• No space required between ">" and "<<<". Avoids changing "f<t><<<" to "f<t> <<<".
• No space required between ">>>" and "("

Such that "f<param><<<1, 1>>>();" remains unchanged instead of being changed to "f<param> << <1, 1>>> ();"

[djasper]

We don't generally want to use the spacing in the input to matter for the output. Is this really important? What would be the downside of using language options?

Also, just controlling the space might not be sufficient as clang-format also might go ahead break between the tokens. Generally, it might be better to actually merge the tokens (as is done for the === operation in JavaScript), as these should be considered to be a single token. Alternatively, you need to at least control CanBreakBefore for these and add tests for BreakBeforeBinaryOperators set to All and None.

[jpienaar]

If CUDA language option is enabled then
A<A<A<int>>> a;
gets changed to
A < A < A < int >>> a;
as >>> at the end is no longer 3 tokens but 1. I could modify the
parseAngle (I believe) function to address this case though. The majority
of uses would be for regular C++, so enabling CUDA language option by
default would seem to benefit me but possibly causing others headaches, so
I went for a route where the opening triple angle bracket is retained as is
in as direct a manner as possible while affecting as few other code paths
as possible.

The importance is in the CUDA code generation side. foo<< <1,1>>>() would
not be recognized as a CUDAKernelCallExpr. So an input program that
compiled would not compile post formatting.

Would it be OK to merge the tokens in the following case?
friend std::ostream& operator << <T> (std::ostream&, const Self&);
That was pretty much the only other usage for three open angle brackets I
saw. Currently I think it would be merged, so that would not change but I
want to double check. If so, then I'll follow your suggestion and add those
tests thanks.

[jpienaar]

Version setting CUDA language option for comparison based on discussion earlier today.

The parseAngle function updated to treat >>> as 3 tokens. These are without the binary operator split tests as now both "<<<" and ">>>" are tokens. As the AST is not constructed, setting the CUDA language option should not have other side-effects here other than to recognize <<< and >>> as 1 token each.

[djasper]

lib/Format/TokenAnnotator.cpp

68

I really don't like such static variables. Also, now we have two such mechanisms (GreaterStashed and Count). Seems like the same mechanisms should be used.

69

In LLVM style, we don't allow one-liner ifs. Use clang-format ..

71–73

Do these lines actually change the behavior? Seems like the entire wrong place to do this.

1809

So, if we just set TT_TemplateOpener/Closer on these, is this really required?

unittests/Format/FormatTest.cpp

9616

nit: TripleAngleBrackets (one "p" and I don't think that these are "braces").

[jpienaar]

Removed the static variable and perform mergin in FormatTokenLexer. Also ran from clang-format -style llvm which changed some spacing.

[jpienaar]

The web ui is a bit confusing when I try to reply to comments on previous revision. So I'll reply here. Most of the previous comments probably don't apply since your pointer to GreaterStashed made me redo that part (thanks!). All the rules (except the last one about space between >>> and ( ) do effect the output, and the one about >>> and ( is probably fine. I didn't want to always merge greater, greater, greater into greatergreatergreater as then the template parsing logic needs to be modified, but if there is a lesslessless in the same instruction then that's a reasonably good signal that it is a kernel launch.

[jpienaar]

Redone following a simpler approach of splitting up << into < and <, and remerging only if not <<<. The formatting of kernel launch parameters then follows that of template instantiation, The previous approach could also have resulted in unmatched braces.

[djasper]

Could you generally upload diffs with the full file as context? That makes review here much easier.

lib/Format/Format.cpp

665

Wouldn't we want to fall back to merging in this case?

I think if the user formats the incomplete line "llvm::outs() <<", we should not rip apart the <<. Add a test for this.

674

Make this:

// Only merge if there currently is no whitespace between the two "<".

677

Indentation... Maybe use clang-format ;-).

679

I think this comment doesn't add information. Remove.

lib/Format/TokenAnnotator.cpp

2043

A few lines higher up, there is at test:

if (Left.is(tok::greater) && Right.is(tok::greater) &&
    Left.isNot(TT_TemplateCloser))
  return false;

Seems like we should combine those. Maybe simply remove the "Left.isNot(TT_TemplateCloser)"?

2044

Indentation.

[jpienaar]

Made the changes recommended. clang-format changed unrelated things in the file.

[djasper]

I think that this is basically good to go. Do you have commit access?

lib/Format/Format.cpp

665

Use early exit.

681

Maybe just use "auto" here.

[jpienaar]

Done. I've simplified around line 665 a bit too.

I have commit access and will commit in a couple of hours. Thanks.

lib/Format/Format.cpp

struct ScalarEnumerationTraits<FormatStyle::NamespaceIndentationKind> {
   static void enumeration(IO &IO,
                           FormatStyle::NamespaceIndentationKind &Value) {
     IO.enumCase(Value, "None", FormatStyle::NI_None);
     IO.enumCase(Value, "Inner", FormatStyle::NI_Inner);
     IO.enumCase(Value, "All", FormatStyle::NI_All);
   }
 };
 
-template <>
-struct ScalarEnumerationTraits<FormatStyle::PointerAlignmentStyle> {
-  static void enumeration(IO &IO,
-                          FormatStyle::PointerAlignmentStyle &Value) {
+template <> struct ScalarEnumerationTraits<FormatStyle::PointerAlignmentStyle> {
+  static void enumeration(IO &IO, FormatStyle::PointerAlignmentStyle &Value) {
     IO.enumCase(Value, "Middle", FormatStyle::PAS_Middle);
     IO.enumCase(Value, "Left", FormatStyle::PAS_Left);
     IO.enumCase(Value, "Right", FormatStyle::PAS_Right);
 
     // For backward compatibility.
     IO.enumCase(Value, "true", FormatStyle::PAS_Left);
     IO.enumCase(Value, "false", FormatStyle::PAS_Right);
   }

 };
 
 template <> struct MappingTraits<FormatStyle> {
   static void mapping(IO &IO, FormatStyle &Style) {
     // When reading, read the language first, we need it for getPredefinedStyle.
     IO.mapOptional("Language", Style.Language);
 
     if (IO.outputting()) {
-      StringRef StylesArray[] = { "LLVM",    "Google", "Chromium",
-                                  "Mozilla", "WebKit", "GNU" };
+      StringRef StylesArray[] = {"LLVM",    "Google", "Chromium",
+                                 "Mozilla", "WebKit", "GNU"};
       ArrayRef<StringRef> Styles(StylesArray);
       for (size_t i = 0, e = Styles.size(); i < e; ++i) {
         StringRef StyleName(Styles[i]);
         FormatStyle PredefinedStyle;
         if (getPredefinedStyle(StyleName, Style.Language, &PredefinedStyle) &&
             Style == PredefinedStyle) {
           IO.mapOptional("# BasedOnStyle", StyleName);
           break;
template <> struct MappingTraits<FormatStyle> {
   }
 };
 
 // Allows to read vector<FormatStyle> while keeping default values.
 // IO.getContext() should contain a pointer to the FormatStyle structure, that
 // will be used to get default values for missing keys.
 // If the first element has no Language specified, it will be treated as the
 // default one for the following elements.
-template <> struct DocumentListTraits<std::vector<FormatStyle> > {
+template <> struct DocumentListTraits<std::vector<FormatStyle>> {
   static size_t size(IO &IO, std::vector<FormatStyle> &Seq) {
     return Seq.size();
   }
   static FormatStyle &element(IO &IO, std::vector<FormatStyle> &Seq,
                               size_t Index) {
     if (Index >= Seq.size()) {
       assert(Index == Seq.size());
       FormatStyle Template;

 
 namespace {
 
 class FormatTokenLexer {
 public:
   FormatTokenLexer(SourceManager &SourceMgr, FileID ID, FormatStyle &Style,
                    encoding::Encoding Encoding)
       : FormatTok(nullptr), IsFirstToken(true), GreaterStashed(false),
-        Column(0), TrailingWhitespace(0), SourceMgr(SourceMgr), ID(ID),
-        Style(Style), IdentTable(getFormattingLangOpts(Style)),
-        Keywords(IdentTable), Encoding(Encoding), FirstInLineIndex(0),
-        FormattingDisabled(false) {
+        LessStashed(false), Column(0), TrailingWhitespace(0),
+        SourceMgr(SourceMgr), ID(ID), Style(Style),
+        IdentTable(getFormattingLangOpts(Style)), Keywords(IdentTable),
+        Encoding(Encoding), FirstInLineIndex(0), FormattingDisabled(false) {
     Lex.reset(new Lexer(ID, SourceMgr.getBuffer(ID), SourceMgr,
                         getFormattingLangOpts(Style)));
     Lex->SetKeepWhitespaceMode(true);
 
     for (const std::string &ForEachMacro : Style.ForEachMacros)
       ForEachMacros.push_back(&IdentTable.get(ForEachMacro));
     std::sort(ForEachMacros.begin(), ForEachMacros.end());
   }
public:
   const AdditionalKeywords &getKeywords() { return Keywords; }
 
 private:
   void tryMergePreviousTokens() {
     if (tryMerge_TMacro())
       return;
     if (tryMergeConflictMarkers())
       return;
+    if (tryMergeLessLess())
+      return;
 
     if (Style.Language == FormatStyle::LK_JavaScript) {
       if (tryMergeJSRegexLiteral())
         return;
       if (tryMergeEscapeSequence())
         return;
 
-      static tok::TokenKind JSIdentity[] = { tok::equalequal, tok::equal };
-      static tok::TokenKind JSNotIdentity[] = { tok::exclaimequal, tok::equal };
-      static tok::TokenKind JSShiftEqual[] = { tok::greater, tok::greater,
-                                               tok::greaterequal };
-      static tok::TokenKind JSRightArrow[] = { tok::equal, tok::greater };
+      static tok::TokenKind JSIdentity[] = {tok::equalequal, tok::equal};
+      static tok::TokenKind JSNotIdentity[] = {tok::exclaimequal, tok::equal};
+      static tok::TokenKind JSShiftEqual[] = {tok::greater, tok::greater,
+                                              tok::greaterequal};
+      static tok::TokenKind JSRightArrow[] = {tok::equal, tok::greater};
       // FIXME: We probably need to change token type to mimic operator with the
       // correct priority.
       if (tryMergeTokens(JSIdentity))
         return;
       if (tryMergeTokens(JSNotIdentity))
         return;
       if (tryMergeTokens(JSShiftEqual))
         return;
       if (tryMergeTokens(JSRightArrow))
         return;
     }
   }
 
+  bool tryMergeLessLess() {
+    // Merge X,less,less,Y into X,lessless,Y unless X or Y is less.
+    if (Tokens.size() < 4) {
+      // Merge <,<,eof to <<,eof
+      if (Tokens.back()->Tok.isNot(tok::eof))
+        return false;
+
+      auto &eof = Tokens.back();
+      Tokens.pop_back();
+      bool LessLessMerged;
+      if ((LessLessMerged = tryMergeTokens({tok::less, tok::less})))
+        Tokens.back()->Tok.setKind(tok::lessless);
+      Tokens.push_back(eof);
+      return LessLessMerged;
+    }
+
+    auto First = Tokens.end() - 4;
+    if (First[3]->is(tok::less) || First[2]->isNot(tok::less) ||
+        First[1]->isNot(tok::less) || First[0]->is(tok::less))
+      return false;
+
+    // Only merge if there currently is no whitespace between the two "<".
+    if (First[2]->WhitespaceRange.getBegin() !=
+        First[2]->WhitespaceRange.getEnd())
+      return false;
+
+    First[1]->Tok.setKind(tok::lessless);
+    First[1]->TokenText = "<<";
+    First[1]->ColumnWidth += 1;
+    Tokens.erase(Tokens.end() - 2);
+    return true;
+  }
+
   bool tryMergeTokens(ArrayRef<tok::TokenKind> Kinds) {
     if (Tokens.size() < Kinds.size())
       return false;
 
     SmallVectorImpl<FormatToken *>::const_iterator First =
         Tokens.end() - Kinds.size();
     if (!First[0]->is(Kinds[0]))
       return false;
     unsigned AddLength = 0;
     for (unsigned i = 1; i < Kinds.size(); ++i) {
-      if (!First[i]->is(Kinds[i]) || First[i]->WhitespaceRange.getBegin() !=
-                                         First[i]->WhitespaceRange.getEnd())
+      if (!First[i]->is(Kinds[i]) ||
+          First[i]->WhitespaceRange.getBegin() !=
         return false;
       AddLength += First[i]->TokenText.size();
     }
     Tokens.resize(Tokens.size() - Kinds.size() + 1);
     First[0]->TokenText = StringRef(First[0]->TokenText.data(),
                                     First[0]->TokenText.size() + AddLength);
     First[0]->ColumnWidth += AddLength;
     return true;
if (Type != TT_Unknown) {
 
       Tokens.push_back(Next);
       return true;
     }
 
     return false;
   }
 
-  FormatToken *getNextToken() {
-    if (GreaterStashed) {
-      // Create a synthesized second '>' token.
-      Token Greater = FormatTok->Tok;
-      unsigned OriginalColumn = FormatTok->OriginalColumn;
-      FormatTok = new (Allocator.Allocate()) FormatToken;
-      FormatTok->Tok = Greater;
-      SourceLocation GreaterLocation =
-          FormatTok->Tok.getLocation().getLocWithOffset(1);
-      FormatTok->WhitespaceRange =
-          SourceRange(GreaterLocation, GreaterLocation);
-      FormatTok->TokenText = ">";
-      FormatTok->ColumnWidth = 1;
-      FormatTok->OriginalColumn = OriginalColumn;
-      GreaterStashed = false;
-      return FormatTok;
-    }
+  FormatToken *getStashedToken() {
+    // Create a synthesized second '>' or '<' token.
+    Token Tok = FormatTok->Tok;
+    StringRef TokenText = FormatTok->TokenText;
+
+    FormatTok->Tok = Tok;
+    SourceLocation TokLocation =
+    FormatTok->WhitespaceRange = SourceRange(TokLocation, TokLocation);
+    FormatTok->TokenText = TokenText;
 
+  FormatToken *getNextToken() {
+    if (GreaterStashed) {
+      GreaterStashed = false;
+      return getStashedToken();
+    }
+    if (LessStashed) {
+      LessStashed = false;
+      return getStashedToken();
+    }
+
     FormatTok = new (Allocator.Allocate()) FormatToken;
     readRawToken(*FormatTok);
     SourceLocation WhitespaceStart =
         FormatTok->Tok.getLocation().getLocWithOffset(-TrailingWhitespace);
     FormatTok->IsFirst = IsFirstToken;
     IsFirstToken = false;
 
     // Consume and record whitespace until we find a significant token.
if (FormatTok->Tok.is(tok::comment)) {
           FormatTok->isOneOf(tok::kw_struct, tok::kw_union, tok::kw_delete)) {
         FormatTok->Tok.setKind(tok::identifier);
         FormatTok->Tok.setIdentifierInfo(nullptr);
       }
     } else if (FormatTok->Tok.is(tok::greatergreater)) {
       FormatTok->Tok.setKind(tok::greater);
       FormatTok->TokenText = FormatTok->TokenText.substr(0, 1);
       GreaterStashed = true;
+    } else if (FormatTok->Tok.is(tok::lessless)) {
+      FormatTok->Tok.setKind(tok::less);
+      FormatTok->TokenText = FormatTok->TokenText.substr(0, 1);
+      LessStashed = true;
     }
 
     // Now FormatTok is the next non-whitespace token.
 
     StringRef Text = FormatTok->TokenText;
     size_t FirstNewlinePos = Text.find('\n');
     if (FirstNewlinePos == StringRef::npos) {
       // FIXME: ColumnWidth actually depends on the start column, we need to
FormatTok->IsForEachMacro =
         std::binary_search(ForEachMacros.begin(), ForEachMacros.end(),
                            FormatTok->Tok.getIdentifierInfo());
 
     return FormatTok;
   }
 
   FormatToken *FormatTok;
   bool IsFirstToken;
-  bool GreaterStashed;
+  bool GreaterStashed, LessStashed;
   unsigned Column;
   unsigned TrailingWhitespace;
   std::unique_ptr<Lexer> Lex;
   SourceManager &SourceMgr;
   FileID ID;
   FormatStyle &Style;
   IdentifierTable IdentTable;
   AdditionalKeywords Keywords;

lib/Format/TokenAnnotator.cpp

while (CurrentToken) {
         Left->MatchingParen = CurrentToken;
         CurrentToken->MatchingParen = Left;
         CurrentToken->Type = TT_TemplateCloser;
         next();
         return true;
       }
       if (CurrentToken->is(tok::question) &&
           Style.Language == FormatStyle::LK_Java) {
         next();
         continue;
       }
       if (CurrentToken->isOneOf(tok::r_paren, tok::r_square, tok::r_brace,
                                 tok::colon, tok::question))
         return false;
       // If a && or || is found and interpreted as a binary operator, this set
       // of angles is likely part of something like "a < b && c > d". If the
       // angles are inside an expression, the ||/&& might also be a binary
       // operator that was misinterpreted because we are parsing template
       // parameters.
       // FIXME: This is getting out of hand, write a decent parser.
       if (CurrentToken->Previous->isOneOf(tok::pipepipe, tok::ampamp) &&
           CurrentToken->Previous->is(TT_BinaryOperator) &&
private:
   ///
   /// This is a heuristic based on whether \p Tok is an identifier following
   /// something that is likely a type.
   bool isStartOfName(const FormatToken &Tok) {
     if (Tok.isNot(tok::identifier) || !Tok.Previous)
       return false;
 
     if (Tok.Previous->is(TT_LeadingJavaAnnotation))
-        return false;
 
     // Skip "const" as it does not have an influence on whether this is a name.
     FormatToken *PreviousNotConst = Tok.Previous;
     while (PreviousNotConst && PreviousNotConst->is(tok::kw_const))
       PreviousNotConst = PreviousNotConst->Previous;
 
     if (!PreviousNotConst)
       return false;
if (Left.is(TT_CastRParen))
     return Style.SpaceAfterCStyleCast || Right.is(TT_BinaryOperator);
   if (Left.is(tok::greater) && Right.is(tok::greater)) {
     return Right.is(TT_TemplateCloser) && Left.is(TT_TemplateCloser) &&
            (Style.Standard != FormatStyle::LS_Cpp11 || Style.SpacesInAngles);
   }
   if (Right.isOneOf(tok::arrow, tok::period, tok::arrowstar, tok::periodstar) ||
       Left.isOneOf(tok::arrow, tok::period, tok::arrowstar, tok::periodstar))
     return false;
   if (!Style.SpaceBeforeAssignmentOperators &&
       Right.getPrecedence() == prec::Assignment)
     return false;
   if (Right.is(tok::coloncolon) && Left.isNot(tok::l_brace))
     return (Left.is(TT_TemplateOpener) &&
             Style.Standard == FormatStyle::LS_Cpp03) ||
            !(Left.isOneOf(tok::identifier, tok::l_paren, tok::r_paren) ||
              Left.isOneOf(TT_TemplateCloser, TT_TemplateOpener));
   if ((Left.is(TT_TemplateOpener)) != (Right.is(TT_TemplateCloser)))
if (Right.is(tok::identifier) && Right.Next && Right.Next->is(TT_DictLiteral))
     return true;
 
   if (Left.is(TT_CtorInitializerComma) &&
       Style.BreakConstructorInitializersBeforeComma)
     return false;
   if (Right.is(TT_CtorInitializerComma) &&
       Style.BreakConstructorInitializersBeforeComma)
     return true;
-  if (Left.is(tok::greater) && Right.is(tok::greater) &&
-      Left.isNot(TT_TemplateCloser))
+  if ((Left.is(tok::greater) && Right.is(tok::greater)) ||
+      (Left.is(tok::less) && Right.is(tok::less)))
     return false;
   if (Right.is(TT_BinaryOperator) &&
       Style.BreakBeforeBinaryOperators != FormatStyle::BOS_None &&
       (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_All ||
        Right.getPrecedence() != prec::Assignment))
     return true;
   if (Left.is(TT_ArrayInitializerLSquare))
     return true;
   if (Right.is(tok::kw_typename) && Left.isNot(tok::kw_const))
     return true;
   if (Left.isBinaryOperator() && !Left.isOneOf(tok::arrowstar, tok::lessless) &&
       Style.BreakBeforeBinaryOperators != FormatStyle::BOS_All &&
       (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None ||
        Left.getPrecedence() == prec::Assignment))
     return true;
   return Left.isOneOf(tok::comma, tok::coloncolon, tok::semi, tok::l_brace,
                       tok::kw_class, tok::kw_struct) ||
          Right.isMemberAccess() || Right.is(TT_TrailingReturnArrow) ||
          Right.isOneOf(tok::lessless, tok::colon, tok::l_square, tok::at) ||
          (Left.is(tok::r_paren) &&
           Right.isOneOf(tok::identifier, tok::kw_const)) ||
          (Left.is(tok::l_paren) && !Right.is(tok::r_paren));
 }
 

unittests/Format/FormatTest.cpp

verifyFormat("[myObject doSomethingWith:arg1\n"
                "    firstBlock:^(Foo *a) {\n"
                "      // ...\n"
                "      int i;\n"
                "    }\n"
                "    secondBlock:^(Bar *b) {\n"
                "      // ...\n"
                "      int i;\n"
                "    }\n"
                "    thirdBlock:^Foo(Bar *b) {\n"
                "      // ...\n"
                "      int i;\n"
                "    }];");
   verifyFormat("[myObject doSomethingWith:arg1\n"
                "               firstBlock:-1\n"
                "              secondBlock:^(Bar *b) {\n"
                "                // ...\n"
                "                int i;\n"
TEST_F(FormatTest, SpacesInAngles) {
   Spaces.Standard = FormatStyle::LS_Cpp11;
   Spaces.SpacesInAngles = true;
   verifyFormat("A< A< int > >();", Spaces);
 
   Spaces.SpacesInAngles = false;
   verifyFormat("A<A<int>>();", Spaces);
 }
 
+TEST_F(FormatTest, TripleAngleBrackets) {
+  verifyFormat("f<<<1, 1>>>();");
+  verifyFormat("f<<<1, 1, 1, s>>>();");
+  verifyFormat("f<<<a, b, c, d>>>();");
+  EXPECT_EQ("f<<<1, 1>>>();",
+            format("f <<< 1, 1 >>> ();"));
+  verifyFormat("f<param><<<1, 1>>>();");
+  verifyFormat("f<1><<<1, 1>>>();");
+  EXPECT_EQ("f<param><<<1, 1>>>();",
+            format("f< param > <<< 1, 1 >>> ();"));
+  verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+               "aaaaaaaaaaa<<<\n    1, 1>>>();");
+}
+
+TEST_F(FormatTest, MergeLessLessAtEnd) {
+  verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+               "aaallvm::outs() <<");
+  verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+               "aaaallvm::outs()\n    <<");
+}
+
 TEST_F(FormatTest, HandleUnbalancedImplicitBracesAcrossPPBranches) {
   std::string code = "#if A\n"
                      "#if B\n"
                      "a.\n"
                      "#endif\n"
                      "    a = 1;\n"
                      "#else\n"
                      "#endif\n"