This is an archive of the discontinued LLVM Phabricator instance.

[clang-format] Do not insert space before closing brace in ObjC dict literal
ClosedPublic

Authored by benhamilton on Mar 22 2018, 9:12 PM.

Details

Summary

Previously, clang-format would sometimes insert a space
before the closing brace in an Objective-C dictionary literal.

Unlike array literals (which obey Style.SpacesInContainerLiterals
to add a space after [ and before ]), Objective-C dictionary
literals currently are not meant to insert a space after { and before
}, regardless of Style.SpacesInContainerLiterals.

However, some constructs like @{foo : @(bar)} caused clang-format
to insert a space between ) and }.

This fixes the issue and adds tests. (I understand the behavior is
not consistent between array literals and dictionary literals, but
that's existing behavior that's a much larger change.)

Test Plan: New tests added. Ran tests with:

% make -j12 FormatTests && ./tools/clang/unittests/Format/FormatTests

Diff Detail

Repository
rC Clang

Event Timeline

benhamilton created this revision.Mar 22 2018, 9:12 PM
djasper accepted this revision.Mar 27 2018, 5:27 AM

Generally looks good, one minor simplification.

lib/Format/TokenAnnotator.cpp
2484

Could you use Right.MatchingParen->endsSequence(TT_DictLiteral, tok::at) here?

This revision is now accepted and ready to land.Mar 27 2018, 5:27 AM
benhamilton marked an inline comment as done.
  • Use Right.MatchingParen->endsSequence(TT_DictLiteral, tok::at)
lib/Format/TokenAnnotator.cpp
2484

Yes, done. I keep forgetting about the reverse order of that method (it makes sense, I just can't keep that info in my brain for some reason.)

djasper accepted this revision.Mar 27 2018, 7:55 AM

Yeah, it's one of these things where neither way would be totally intuitive to everyone.

I filed https://bugs.llvm.org/show_bug.cgi?id=36919 to follow up and make ObjC dictionary literal spacing consistent with ObjC array literals.

This revision was automatically updated to reflect the committed changes.