Page MenuHomePhabricator

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

Authored by rupprecht on Dec 20 2018, 3:37 PM.

Details

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.

Diff Detail

Repository
rC Clang

Event Timeline

rupprecht created this revision.Dec 20 2018, 3:37 PM

Post-holiday ping

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?

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?

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

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 updated this revision to Diff 180882.Jan 9 2019, 10:51 AM

Move TableGen language check to where SpacesInContainerLiterals is checked

rupprecht planned changes to this revision.Jan 16 2019, 2:27 PM

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

rupprecht updated this revision to Diff 185787.Feb 7 2019, 8:54 AM
  • Rebased w/ D56943 patched in so we can override just TableGen in getLLVMStyle()
Herald added a project: Restricted Project. · View Herald TranscriptFeb 7 2019, 8:54 AM
Herald added a subscriber: arphaman. · View Herald Transcript

Rebase after NFC commit rC355123

MyDeveloperDay accepted this revision.Feb 28 2019, 11:34 AM
This revision is now accepted and ready to land.Feb 28 2019, 11:34 AM
This revision was automatically updated to reflect the committed changes.