[clang-format][NFC] Allow getLLVMStyle() to take a language

[rupprecht]

Summary

getLLVMStyle() sets the default style, but doesn't take the language as a parameter, so can't set default parameters when they differ from C++. This change adds LanguageKind as an input to getLLVMStyle so that we can start doing that.

See D55964 as a motivation for this, where we want Tablegen to be formatted differently than C++.

[rupprecht]

ping

[rupprecht]

ping

[kristina]

The patch itself looks sound. However given that you have a specific use case in mind (TableGen files) could you provide supplementary coverage for that specific use case (unit tests for formatting td syntax using format::getLLVMStyle(format::FormatStyle::LK_TableGen)? I'm not entirely sure how useful this particular change is given that there's no linked patches related to your use case, I think adding those would help as well (possibly as a separate dependent patchset).

[rupprecht]

In D56943#1388314, @kristina wrote:

The patch itself looks sound. However given that you have a specific use case in mind (TableGen files) could you provide supplementary coverage for that specific use case (unit tests for formatting td syntax using format::getLLVMStyle(format::FormatStyle::LK_TableGen)? I'm not entirely sure how useful this particular change is given that there's no linked patches related to your use case, I think adding those would help as well (possibly as a separate dependent patchset).

The use case is a pre-refactoring for D55964, which is in the patch description -- is that what you mean by "linked patches"?
I'll update that patch to use this approach, and make it a dependent patch.

[rupprecht]

In D56943#1388985, @rupprecht wrote:

In D56943#1388314, @kristina wrote:

The patch itself looks sound. However given that you have a specific use case in mind (TableGen files) could you provide supplementary coverage for that specific use case (unit tests for formatting td syntax using format::getLLVMStyle(format::FormatStyle::LK_TableGen)? I'm not entirely sure how useful this particular change is given that there's no linked patches related to your use case, I think adding those would help as well (possibly as a separate dependent patchset).

The use case is a pre-refactoring for D55964, which is in the patch description -- is that what you mean by "linked patches"?
I'll update that patch to use this approach, and make it a dependent patch.

Added as a dependent patch. I've never linked patches together, so I might not be doing it right. Guess I'll read some phab docs now...

[MyDeveloperDay]

clang/lib/Format/ContinuationIndenter.cpp

193 ↗

(On Diff #182619)

if you've set up a default argument of LK_Cpp do you need to keep supplying it? if your going to supply it everywhere don't make it a default argument or vice versa

This might make a much smaller patch, a little more digestable and all the tests can remain as they are.

[rupprecht]

Revert getLLVMStyle(LK_Cpp) fixes

[rupprecht]

clang/lib/Format/ContinuationIndenter.cpp

193 ↗

(On Diff #182619)

My goal was to fix all the in-tree callers but leave it as an optional arg (at least initially) so as to not break out of tree callers. I reduced it to what I actually need for this patch.

[MyDeveloperDay]

I think I've seen you need this for another patch, so in the absence of the code owners this LGTM, and thank you for removing all the changes to the tests in the previous diff.

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

 private:
   FormatStyleSet StyleSet;
 
   friend std::error_code parseConfiguration(StringRef Text, FormatStyle *Style);
 };
 
 /// Returns a format style complying with the LLVM coding standards:
 /// http://llvm.org/docs/CodingStandards.html.
-FormatStyle getLLVMStyle();
+FormatStyle getLLVMStyle(
+    FormatStyle::LanguageKind Language = FormatStyle::LanguageKind::LK_Cpp);
 
 /// Returns a format style complying with one of Google's style guides:
 /// http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml.
 /// http://google-styleguide.googlecode.com/svn/trunk/javascriptguide.xml.
 /// https://developers.google.com/protocol-buffers/docs/style.
 FormatStyle getGoogleStyle(FormatStyle::LanguageKind Language);
 
 /// Returns a format style complying with Chromium's style guide:

cfe/trunk/lib/Format/Format.cpp

case FormatStyle::BS_WebKit:
     Expanded.BraceWrapping.AfterFunction = true;
     break;
   default:
     break;
   }
   return Expanded;
 }
 
-FormatStyle getLLVMStyle() {
+FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
   FormatStyle LLVMStyle;
-  LLVMStyle.Language = FormatStyle::LK_Cpp;
+  LLVMStyle.Language = Language;
   LLVMStyle.AccessModifierOffset = -2;
   LLVMStyle.AlignEscapedNewlines = FormatStyle::ENAS_Right;
   LLVMStyle.AlignAfterOpenBracket = FormatStyle::BAS_Align;
   LLVMStyle.AlignOperands = true;
   LLVMStyle.AlignTrailingComments = true;
   LLVMStyle.AlignConsecutiveAssignments = false;
   LLVMStyle.AlignConsecutiveDeclarations = false;
   LLVMStyle.AllowAllParametersOfDeclarationOnNextLine = true;

 FormatStyle getGoogleStyle(FormatStyle::LanguageKind Language) {
   if (Language == FormatStyle::LK_TextProto) {
     FormatStyle GoogleStyle = getGoogleStyle(FormatStyle::LK_Proto);
     GoogleStyle.Language = FormatStyle::LK_TextProto;
 
     return GoogleStyle;
   }
 
-  FormatStyle GoogleStyle = getLLVMStyle();
-  GoogleStyle.Language = Language;
+  FormatStyle GoogleStyle = getLLVMStyle(Language);
 
   GoogleStyle.AccessModifierOffset = -1;
   GoogleStyle.AlignEscapedNewlines = FormatStyle::ENAS_Left;
   GoogleStyle.AllowShortIfStatementsOnASingleLine = true;
   GoogleStyle.AllowShortLoopsOnASingleLine = true;
   GoogleStyle.AlwaysBreakBeforeMultilineStrings = true;
   GoogleStyle.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_Yes;
   GoogleStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = true;

 
 llvm::Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName,
                                      StringRef FallbackStyleName,
                                      StringRef Code,
                                      llvm::vfs::FileSystem *FS) {
   if (!FS) {
     FS = llvm::vfs::getRealFileSystem().get();
   }
-  FormatStyle Style = getLLVMStyle();
-  Style.Language = guessLanguage(FileName, Code);
+  FormatStyle Style = getLLVMStyle(guessLanguage(FileName, Code));
 
   FormatStyle FallbackStyle = getNoStyle();
   if (!getPredefinedStyle(FallbackStyleName, Style.Language, &FallbackStyle))
     return make_string_error("Invalid fallback style \"" + FallbackStyleName);
 
   if (StyleName.startswith("{")) {
     // Parse YAML/JSON style from the command line.
     if (std::error_code ec = parseConfiguration(StyleName, &Style))

cfe/trunk/unittests/Format/FormatTest.cpp

 TEST_F(FormatTest, MessUp) {
   EXPECT_EQ("1 2 3", test::messUp("1 2 3"));
   EXPECT_EQ("1 2 3\n", test::messUp("1\n2\n3\n"));
   EXPECT_EQ("a\n//b\nc", test::messUp("a\n//b\nc"));
   EXPECT_EQ("a\n#b\nc", test::messUp("a\n#b\nc"));
   EXPECT_EQ("a\n#b c d\ne", test::messUp("a\n#b\\\nc\\\nd\ne"));
 }
 
+TEST_F(FormatTest, DefaultLLVMStyleIsCpp) {
+  EXPECT_EQ(FormatStyle::LK_Cpp, getLLVMStyle().Language);
+}
+
+TEST_F(FormatTest, LLVMStyleOverride) {
+  EXPECT_EQ(FormatStyle::LK_Proto,
+            getLLVMStyle(FormatStyle::LK_Proto).Language);
+}
+
 //===----------------------------------------------------------------------===//
 // Basic function tests.
 //===----------------------------------------------------------------------===//
 
 TEST_F(FormatTest, DoesNotChangeCorrectlyFormattedCode) {
   EXPECT_EQ(";", format(";"));
 }