[clang-format] Break template declarations followed by comments

[krasimir]

Summary

This patch fixes two bugs in clang-format where the template wrapper doesn't skip over
comments causing a long template declaration to not be split into multiple lines.
These were latent and exposed by r332436.

[sammccall]

Thanks for chasing this, good detective work!
Changes look plausible and tests are nice, so LG assuming you know what you're doing. If you're unsure, Manuel will give you better advice than me :)

lib/Format/ContinuationIndenter.cpp

if (startsSegmentOfBuilderTypeCall(Current) &&
       // FIXME: We should find a more generic solution to this problem.
       !(State.Column <= NewLineColumn &&
         Style.Language == FormatStyle::LK_JavaScript))
     return true;
 
   // If the template declaration spans multiple lines, force wrap before the
   // function/class declaration
   if (Previous.ClosesTemplateDeclaration &&
-      State.Stack.back().BreakBeforeParameter)
+      State.Stack.back().BreakBeforeParameter && Current.CanBreakBefore)
     return true;
 
   if (State.Column <= NewLineColumn)
     return false;
 
   if (Style.AlwaysBreakBeforeMultilineStrings &&
       (NewLineColumn == State.FirstIndent + Style.ContinuationIndentWidth ||
        Previous.is(tok::comma) || Current.NestingLevel < 2) &&
if (State.Stack.size() > 1)
           Style.ContinuationIndentWidth;
   }
 
   if ((PreviousNonComment &&
        PreviousNonComment->isOneOf(tok::comma, tok::semi) &&
        !State.Stack.back().AvoidBinPacking) ||
       Previous.is(TT_BinaryOperator))
     State.Stack.back().BreakBeforeParameter = false;
-  if (Previous.isOneOf(TT_TemplateCloser, TT_JavaAnnotation) &&
+  if (PreviousNonComment &&
+      PreviousNonComment->isOneOf(TT_TemplateCloser, TT_JavaAnnotation) &&
       Current.NestingLevel == 0)
     State.Stack.back().BreakBeforeParameter = false;
   if (NextNonComment->is(tok::question) ||
       (PreviousNonComment && PreviousNonComment->is(tok::question)))
     State.Stack.back().BreakBeforeParameter = true;
   if (Current.is(TT_BinaryOperator) && Current.CanBreakBefore)
     State.Stack.back().BreakBeforeParameter = false;
 

unittests/Format/FormatTest.cpp

verifyFormat("template <enum E> class A {\n"
                "public:\n"
                "  E *f();\n"
                "};", NeverBreak);
   NeverBreak.PenaltyBreakTemplateDeclaration = 100;
   verifyFormat("template <typename T> void\nfoo(aaaaaaaaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbbbb) {}",
                NeverBreak);
 }
 
+TEST_F(FormatTest, WrapsTemplateDeclarationsWithComments) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_Cpp);
+  Style.ColumnLimit = 60;
+  EXPECT_EQ(R"test(
+// Baseline - no comments.
+template <
+    typename aaaaaaaaaaaaaaaaaaaaaa<bbbbbbbbbbbb>::value>
+void f() {})test",
+            format(R"test(
+// Baseline - no comments.
+template <
+    typename aaaaaaaaaaaaaaaaaaaaaa<bbbbbbbbbbbb>::value>
+void f() {})test", Style));
+
+  EXPECT_EQ(R"test(
+template <
+    typename aaaaaaaaaa<bbbbbbbbbbbb>::value>  // trailing
+void f() {})test",
+            format(R"test(
+template <
+    typename aaaaaaaaaa<bbbbbbbbbbbb>::value> // trailing
+void f() {})test", Style));
+
+  EXPECT_EQ(R"test(
+template <
+    typename aaaaaaaaaa<bbbbbbbbbbbb>::value> /* line */
+void f() {})test",
+            format(R"test(
+template <typename aaaaaaaaaa<bbbbbbbbbbbb>::value>  /* line */
+void f() {})test", Style));
+
+  EXPECT_EQ(R"test(
+template <
+    typename aaaaaaaaaa<bbbbbbbbbbbb>::value>  // trailing
+                                               // multiline
+void f() {})test",
+            format(R"test(
+template <
+    typename aaaaaaaaaa<bbbbbbbbbbbb>::value> // trailing
+                                              // multiline
+void f() {})test", Style));
+
+  EXPECT_EQ(R"test(
+template <typename aaaaaaaaaa<
+    bbbbbbbbbbbb>::value>  // trailing loooong
+void f() {})test",
+            format(R"test(
+template <
+    typename aaaaaaaaaa<bbbbbbbbbbbb>::value> // trailing loooong
+void f() {})test", Style));
+}
+
 TEST_F(FormatTest, WrapsTemplateParameters) {
   FormatStyle Style = getLLVMStyle();
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
   Style.BreakBeforeBinaryOperators = FormatStyle::BOS_None;
   verifyFormat(
       "template <typename... a> struct q {};\n"
       "extern q<aaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaa,\n"
       "    aaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaa>\n"