Fix bug 20892 - clang-format does not handle C-style comments

[lifted]

Summary

http://llvm.org/bugs/show_bug.cgi?id=20892

Add support of C-style formatting enabling/disabling directives. Now the following two styles are supported:

// clang-format on
/* clang-format on */

The flexibility in comments (support of extra spaces and/or slashes, etc.) is deliberately avoided to simplify search in large code bases.

[djasper]

Thanks for picking this up!

I am against supporting "///" and "/**" style comments as I don't see a good reason for using them for this purpose. I am even against trimming the whitespace. I want the comments to have a single spelling that I can easily search for in a bigger codebase.

[lifted]

I am against supporting "///" and "/**" style comments

That's reasonable. I will remove all that nontrivial logic and replace it with simple comparisons against // clang-format on and /* clang-format on */.

[lifted]

Simplify formatting directives recognition logic

[djasper]

One comment, otherwise looks good.

lib/Format/Format.cpp

1736

I don't think it is worth pulling out these functions, but I don't feel strongly.

lib/Format/Format.cpp

if (Tok.is(tok::unknown)) {
       if (!Tok.TokenText.empty() && Tok.TokenText[0] == '"') {
         Tok.Tok.setKind(tok::string_literal);
         Tok.IsUnterminatedLiteral = true;
       } else if (Style.Language == FormatStyle::LK_JavaScript &&
                  Tok.TokenText == "''") {
         Tok.Tok.setKind(tok::char_constant);
       }
     }
-    if (Tok.is(tok::comment) && Tok.TokenText == "// clang-format on")
+    if (isFormatOnDirective(Tok))
       FormattingDisabled = false;
     Tok.Finalized = FormattingDisabled;
-    if (Tok.is(tok::comment) && Tok.TokenText == "// clang-format off")
+    if (isFormatOffDirective(Tok))
       FormattingDisabled = true;
   }
+
+  static bool isFormatOffDirective(const FormatToken &Tok) {
+    return Tok.is(tok::comment) && (Tok.TokenText == "// clang-format off" ||
+                                    Tok.TokenText == "/* clang-format off */");
+  }
+
+  static bool isFormatOnDirective(const FormatToken &Tok) {
+    return Tok.is(tok::comment) && (Tok.TokenText == "// clang-format on" ||
+                                    Tok.TokenText == "/* clang-format on */");
+  }
 };
 
 static StringRef getLanguageName(FormatStyle::LanguageKind Language) {
   switch (Language) {
   case FormatStyle::LK_Cpp:
     return "C++";
   case FormatStyle::LK_JavaScript:
     return "JavaScript";

unittests/Format/FormatTest.cpp

EXPECT_EQ("int i;\n"
             "  int j;\n"
             "// clang-format on\n"
             "int k;",
             format(" int  i;\n"
                    "   // clang-format off\n"
                    "  int j;\n"
                    " // clang-format on\n"
                    "   int   k;"));
+  EXPECT_EQ("int i;\n"
+            "/* clang-format off */\n"
+            "  int j;\n"
+            "/* clang-format on */\n"
+            "int k;",
+            format(" int  i;\n"
+                   "   /* clang-format off */\n"
+                   "  int j;\n"
+                   " /* clang-format on */\n"
+                   "   int   k;"));
 }
 
 TEST_F(FormatTest, DoNotCrashOnInvalidInput) {
   format("? ) =");
 }
 
 } // end namespace tooling
 } // end namespace clang