[clang-format] Fix a bug that joins template closer and =

[owenpan]

Summary

Also fixes the documentation for SpaceBeforeAssignmentOperators.

See the discussion here.

[MyDeveloperDay]

LGTM

[• Quuxplusone]

This comment was removed by • Quuxplusone.

[• Quuxplusone]

LGTM too, although I would still question whether Style.SpaceBeforeAssignmentOperators is providing anyone any benefit at all.

clang/unittests/Format/FormatTest.cpp

6631 ↗

(On Diff #215763)

Actually, could you add a test case specifically for the troublesome enable_if_t pattern? Just in case any future option treats default-template-arguments any differently from assignment-statements, whitespace-wise.

verifyformat("template<enable_if_t<Foo, int> = 0>");

[owenpan]

In D66384#1634405, @Quuxplusone wrote:

although I would still question whether Style.SpaceBeforeAssignmentOperators is providing anyone any benefit at all.

See here for the patch. I don't know why the option was added but think it's probably outdated and should be deprecated.

[owenpan]

clang/unittests/Format/FormatTest.cpp

6631 ↗

(On Diff #215763)

I think it duplicates a<int> = 1; on Line 6630 because they both test the insertion of a space between a template closer > and an assignment operator = that follows.

cfe/trunk/docs/ClangFormatStyleOptions.rst

.. code-block:: c++
      template <int> void foo();     vs.     template<int> void foo();
 
 **SpaceBeforeAssignmentOperators** (``bool``)
   If ``false``, spaces will be removed before assignment operators.
 
   .. code-block:: c++
 
      true:                                  false:
-     int a = 5;                     vs.     int a=5;
-     a += 42                                a+=42;
+     int a = 5;                     vs.     int a= 5;
+     a += 42;                               a+= 42;
 
 **SpaceBeforeCpp11BracedList** (``bool``)
   If ``true``, a space will be inserted before a C++11 braced list
   used to initialize an object (after the preceding identifier or type).
 
   .. code-block:: c++
 
      true:                                  false:

cfe/trunk/include/clang/Format/Format.h

   ///    true:                                  false:
   ///    template <int> void foo();     vs.     template<int> void foo();
   /// \endcode
   bool SpaceAfterTemplateKeyword;
 
   /// If ``false``, spaces will be removed before assignment operators.
   /// \code
   ///    true:                                  false:
-  ///    int a = 5;                     vs.     int a=5;
-  ///    a += 42                                a+=42;
+  ///    int a = 5;                     vs.     int a= 5;
+  ///    a += 42;                               a+= 42;
   /// \endcode
   bool SpaceBeforeAssignmentOperators;
 
   /// If ``true``, a space will be inserted before a C++11 braced list
   /// used to initialize an object (after the preceding identifier or type).
   /// \code
   ///    true:                                  false:
   ///    Foo foo { bar };               vs.     Foo foo{ bar };

cfe/trunk/lib/Format/TokenAnnotator.cpp

if (Style.Language == FormatStyle::LK_TextProto ||
       return !Style.Cpp11BracedListStyle;
     return Right.is(TT_TemplateCloser) && Left.is(TT_TemplateCloser) &&
            (Style.Standard < FormatStyle::LS_Cpp11 || Style.SpacesInAngles);
   }
   if (Right.isOneOf(tok::arrow, tok::arrowstar, tok::periodstar) ||
       Left.isOneOf(tok::arrow, tok::period, tok::arrowstar, tok::periodstar) ||
       (Right.is(tok::period) && Right.isNot(TT_DesignatedInitializerPeriod)))
     return false;
-  if (!Style.SpaceBeforeAssignmentOperators &&
+  if (!Style.SpaceBeforeAssignmentOperators && Left.isNot(TT_TemplateCloser) &&
       Right.getPrecedence() == prec::Assignment)
     return false;
   if (Style.Language == FormatStyle::LK_Java && Right.is(tok::coloncolon) &&
       (Left.is(tok::identifier) || Left.is(tok::kw_this)))
     return false;
   if (Right.is(tok::coloncolon) && Left.is(tok::identifier))
     // Generally don't remove existing spaces between an identifier and "::".
     // The identifier might actually be a macro name such as ALWAYS_INLINE. If

cfe/trunk/unittests/Format/FormatTest.cpp

TEST_F(FormatTest, UnderstandsTemplateParameters) {
   EXPECT_EQ("A<A<A<A>>> a;", format("A<A<A<A>> > a;", getGoogleStyle()));
   EXPECT_EQ("A<::A<int>> a;", format("A< ::A<int>> a;", getGoogleStyle()));
   EXPECT_EQ("A<::A<int>> a;", format("A<::A<int> > a;", getGoogleStyle()));
   EXPECT_EQ("auto x = [] { A<A<A<A>>> a; };",
             format("auto x=[]{A<A<A<A> >> a;};", getGoogleStyle()));
 
   verifyFormat("A<A<int>> a;", getChromiumStyle(FormatStyle::LK_Cpp));
 
-  verifyFormat("int i = a<1> >> 1;");
+  // template closer followed by a token that starts with > or =
   verifyFormat("bool b = a<1> > 1;");
+  verifyFormat("bool b = a<1> >= 1;");
+  verifyFormat("int i = a<1> >> 1;");
+  FormatStyle Style = getLLVMStyle();
+  Style.SpaceBeforeAssignmentOperators = false;
+  verifyFormat("bool b= a<1> == 1;", Style);
+  verifyFormat("a<int> = 1;", Style);
+  verifyFormat("a<int> >>= 1;", Style);
 
   verifyFormat("test >> a >> b;");
   verifyFormat("test << a >> b;");
 
   verifyFormat("f<int>();");
   verifyFormat("template <typename T> void f() {}");
   verifyFormat("struct A<std::enable_if<sizeof(T2) < sizeof(int32)>::type>;");
   verifyFormat("struct A<std::enable_if<sizeof(T2) ? sizeof(int32) : "