[clang-format][TableGen] Don't add spaces around items in square braces.

[rupprecht]

Summary

clang-formatting wants to add spaces around items in square braces, e.g. [1, 2] -> [ 1, 2 ]. Based on a quick check [1], it seems like most cases are using the [1, 2] format, so make that the consistent one.

[1] in llvm .td files, the regex \[[^ ] (bracket followed by not-a-space) shows up ~400 times, but \[\s[^ ] (bracket followed by one space and one not-a-space) shows up ~40 times => ~90% uses this format.

[rupprecht]

Post-holiday ping

[djasper]

This seem to conceptually be a list of things rather than an array subscript, though, right? Could we alternatively set SpacesInContainerLiterals to false for LK_TableGen?

[rupprecht]

In D55964#1351348, @djasper wrote:

This seem to conceptually be a list of things rather than an array subscript, though, right? Could we alternatively set SpacesInContainerLiterals to false for LK_TableGen?

That seems to work, e.g. if I manually set the option in the test, but I can't seem to find a way to set a per-language default. getLLVMStyle(), which seems to be the default that every style is rooted from, assumes C++ (and does not have the language provided). Is there a good way to workaround that?

[djasper]

Look at getGoogleStyle(). It has a bunch of language-specific configs at the bottom. You can do the same for TableGen in getLLVMStyle().

[rupprecht]

Right, but getGoogleStyle() has the inferred language passed in (i.e. getGoogleStyle(FormatStyle::LanguageKind Language). Whereas getLLVMStyle() takes no args and assumes C++.
Would it be worthwhile to refactor getLLVMStyle to take in an optional language arg?

[rupprecht]

Move TableGen language check to where SpacesInContainerLiterals is checked

[rupprecht]

I'll refactor getLLVMStyle() to getLLVMStyle(FormatStyle::LanguageKind Language) to support this change first.

[rupprecht]

• Rebased w/ D56943 patched in so we can override just TableGen in getLLVMStyle()

[rupprecht]

Rebase after NFC commit rC355123

[MyDeveloperDay]

LGTM

lib/Format/Format.cpp

FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
   LLVMStyle.PenaltyBreakTemplateDeclaration = prec::Relational;
 
   LLVMStyle.DisableFormat = false;
   LLVMStyle.SortIncludes = true;
   LLVMStyle.SortUsingDeclarations = true;
   LLVMStyle.StatementMacros.push_back("Q_UNUSED");
   LLVMStyle.StatementMacros.push_back("QT_REQUIRE_VERSION");
 
+  // Defaults that differ when not C++.
+  if (Language == FormatStyle::LK_TableGen) {
+    LLVMStyle.SpacesInContainerLiterals = false;
+  }
+
   return LLVMStyle;
 }
 
 FormatStyle getGoogleStyle(FormatStyle::LanguageKind Language) {
   if (Language == FormatStyle::LK_TextProto) {
     FormatStyle GoogleStyle = getGoogleStyle(FormatStyle::LK_Proto);
     GoogleStyle.Language = FormatStyle::LK_TextProto;
 

unittests/Format/FormatTestTableGen.cpp

TEST_F(FormatTestTableGen, FormatStringBreak) {
   verifyFormat("include \"OptParser.td\"\n"
                "def flag : Flag<\"--foo\">,\n"
                "           HelpText<\n"
                "               \"This is a very, very, very, very, \"\n"
                "               \"very, very, very, very, very, very, \"\n"
                "               \"very long help string\">;\n");
 }
 
+TEST_F(FormatTestTableGen, NoSpacesInSquareBracketLists) {
+  verifyFormat("def flag : Flag<[\"-\", \"--\"], \"foo\">;\n");
+}
+
 } // namespace format
 } // end namespace clang