[PATCH] D44816: [clang-format] Do not insert space before closing brace in ObjC dict literal
This revision was automatically updated to reflect the committed changes. Closed by commit rC328627: [clang-format] Do not insert space before closing brace in ObjC dict literal (authored by benhamilton, committed by ). Changed prior to commit: https://reviews.llvm.org/D44816?vs=139930=139931#toc Repository: rC Clang https://reviews.llvm.org/D44816 Files: lib/Format/TokenAnnotator.cpp unittests/Format/FormatTestObjC.cpp Index: lib/Format/TokenAnnotator.cpp === --- lib/Format/TokenAnnotator.cpp +++ lib/Format/TokenAnnotator.cpp @@ -2480,6 +2480,10 @@ return false; if (Left.is(TT_TemplateCloser) && Right.is(tok::l_square)) return false; + if (Right.is(tok::r_brace) && Right.MatchingParen && + Right.MatchingParen->endsSequence(TT_DictLiteral, tok::at)) +// Objective-C dictionary literal -> no space before closing brace. +return false; return true; } Index: unittests/Format/FormatTestObjC.cpp === --- unittests/Format/FormatTestObjC.cpp +++ unittests/Format/FormatTestObjC.cpp @@ -1000,6 +1000,21 @@ " ( id)a : ( id),\n" " ( id)aa : ( id)aa,\n" "};"); + Style.ColumnLimit = 40; + verifyFormat("int Foo() {\n" + " a12345 = @{a12345 : a12345};\n" + "}"); + verifyFormat("int Foo() {\n" + " a12345 = @{(Foo *)a12345 : @(a12345)};\n" + "}"); + Style.SpacesInContainerLiterals = false; + verifyFormat("int Foo() {\n" + " b12345 = @{b12345: b12345};\n" + "}"); + verifyFormat("int Foo() {\n" + " b12345 = @{(Foo *)b12345: @(b12345)};\n" + "}"); + Style.SpacesInContainerLiterals = true; Style = getGoogleStyle(FormatStyle::LK_ObjC); verifyFormat( @@ -1055,6 +1070,21 @@ verifyFormat("[someFunction someLongParameter:@[\n" " NSBundle.mainBundle.infoDictionary[@\"a\"]\n" "]];"); + Style.ColumnLimit = 40; + verifyFormat("int Foo() {\n" + " a12345 = @[ a12345, a12345 ];\n" + "}"); + verifyFormat("int Foo() {\n" + " a123 = @[ (Foo *)a12345, @(a12345) ];\n" + "}"); + Style.SpacesInContainerLiterals = false; + verifyFormat("int Foo() {\n" + " b12345 = @[b12345, b12345];\n" + "}"); + verifyFormat("int Foo() {\n" + " b12345 = @[(Foo *)b12345, @(b12345)];\n" + "}"); + Style.SpacesInContainerLiterals = true; Style.ColumnLimit = 20; // We can't break string literals inside NSArray literals // (that raises -Wobjc-string-concatenation). Index: lib/Format/TokenAnnotator.cpp === --- lib/Format/TokenAnnotator.cpp +++ lib/Format/TokenAnnotator.cpp @@ -2480,6 +2480,10 @@ return false; if (Left.is(TT_TemplateCloser) && Right.is(tok::l_square)) return false; + if (Right.is(tok::r_brace) && Right.MatchingParen && + Right.MatchingParen->endsSequence(TT_DictLiteral, tok::at)) +// Objective-C dictionary literal -> no space before closing brace. +return false; return true; } Index: unittests/Format/FormatTestObjC.cpp === --- unittests/Format/FormatTestObjC.cpp +++ unittests/Format/FormatTestObjC.cpp @@ -1000,6 +1000,21 @@ " ( id)a : ( id),\n" " ( id)aa : ( id)aa,\n" "};"); + Style.ColumnLimit = 40; + verifyFormat("int Foo() {\n" + " a12345 = @{a12345 : a12345};\n" + "}"); + verifyFormat("int Foo() {\n" + " a12345 = @{(Foo *)a12345 : @(a12345)};\n" + "}"); + Style.SpacesInContainerLiterals = false; + verifyFormat("int Foo() {\n" + " b12345 = @{b12345: b12345};\n" + "}"); + verifyFormat("int Foo() {\n" + " b12345 = @{(Foo *)b12345: @(b12345)};\n" + "}"); + Style.SpacesInContainerLiterals = true; Style = getGoogleStyle(FormatStyle::LK_ObjC); verifyFormat( @@ -1055,6 +1070,21 @@ verifyFormat("[someFunction someLongParameter:@[\n" " NSBundle.mainBundle.infoDictionary[@\"a\"]\n" "]];"); + Style.ColumnLimit = 40; + verifyFormat("int Foo() {\n" + " a12345 = @[ a12345, a12345 ];\n" + "}"); + verifyFormat("int Foo() {\n" + " a123 = @[ (Foo *)a12345, @(a12345) ];\n" + "}"); + Style.SpacesInContainerLiterals = false; + verifyFormat("int Foo() {\n" + " b12345 = @[b12345, b12345];\n" + "}"); + verifyFormat("int Foo() {\n" + " b12345
[PATCH] D44816: [clang-format] Do not insert space before closing brace in ObjC dict literal
benhamilton added a comment. 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. Repository: rC Clang https://reviews.llvm.org/D44816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44816: [clang-format] Do not insert space before closing brace in ObjC dict literal
djasper accepted this revision. djasper added a comment. Yeah, it's one of these things where neither way would be totally intuitive to everyone. Repository: rC Clang https://reviews.llvm.org/D44816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44816: [clang-format] Do not insert space before closing brace in ObjC dict literal
benhamilton added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:2484 + if (Right.is(tok::r_brace) && Right.MatchingParen && + Right.MatchingParen->is(TT_DictLiteral) && + Right.MatchingParen->Previous && djasper wrote: > Could you use Right.MatchingParen->endsSequence(TT_DictLiteral, tok::at) here? 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.) Repository: rC Clang https://reviews.llvm.org/D44816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44816: [clang-format] Do not insert space before closing brace in ObjC dict literal
benhamilton updated this revision to Diff 139930. benhamilton marked an inline comment as done. benhamilton added a comment. - Use `Right.MatchingParen->endsSequence(TT_DictLiteral, tok::at)` Repository: rC Clang https://reviews.llvm.org/D44816 Files: lib/Format/TokenAnnotator.cpp unittests/Format/FormatTestObjC.cpp Index: unittests/Format/FormatTestObjC.cpp === --- unittests/Format/FormatTestObjC.cpp +++ unittests/Format/FormatTestObjC.cpp @@ -1000,6 +1000,21 @@ " ( id)a : ( id),\n" " ( id)aa : ( id)aa,\n" "};"); + Style.ColumnLimit = 40; + verifyFormat("int Foo() {\n" + " a12345 = @{a12345 : a12345};\n" + "}"); + verifyFormat("int Foo() {\n" + " a12345 = @{(Foo *)a12345 : @(a12345)};\n" + "}"); + Style.SpacesInContainerLiterals = false; + verifyFormat("int Foo() {\n" + " b12345 = @{b12345: b12345};\n" + "}"); + verifyFormat("int Foo() {\n" + " b12345 = @{(Foo *)b12345: @(b12345)};\n" + "}"); + Style.SpacesInContainerLiterals = true; Style = getGoogleStyle(FormatStyle::LK_ObjC); verifyFormat( @@ -1055,6 +1070,21 @@ verifyFormat("[someFunction someLongParameter:@[\n" " NSBundle.mainBundle.infoDictionary[@\"a\"]\n" "]];"); + Style.ColumnLimit = 40; + verifyFormat("int Foo() {\n" + " a12345 = @[ a12345, a12345 ];\n" + "}"); + verifyFormat("int Foo() {\n" + " a123 = @[ (Foo *)a12345, @(a12345) ];\n" + "}"); + Style.SpacesInContainerLiterals = false; + verifyFormat("int Foo() {\n" + " b12345 = @[b12345, b12345];\n" + "}"); + verifyFormat("int Foo() {\n" + " b12345 = @[(Foo *)b12345, @(b12345)];\n" + "}"); + Style.SpacesInContainerLiterals = true; Style.ColumnLimit = 20; // We can't break string literals inside NSArray literals // (that raises -Wobjc-string-concatenation). Index: lib/Format/TokenAnnotator.cpp === --- lib/Format/TokenAnnotator.cpp +++ lib/Format/TokenAnnotator.cpp @@ -2480,6 +2480,10 @@ return false; if (Left.is(TT_TemplateCloser) && Right.is(tok::l_square)) return false; + if (Right.is(tok::r_brace) && Right.MatchingParen && + Right.MatchingParen->endsSequence(TT_DictLiteral, tok::at)) +// Objective-C dictionary literal -> no space before closing brace. +return false; return true; } Index: unittests/Format/FormatTestObjC.cpp === --- unittests/Format/FormatTestObjC.cpp +++ unittests/Format/FormatTestObjC.cpp @@ -1000,6 +1000,21 @@ " ( id)a : ( id),\n" " ( id)aa : ( id)aa,\n" "};"); + Style.ColumnLimit = 40; + verifyFormat("int Foo() {\n" + " a12345 = @{a12345 : a12345};\n" + "}"); + verifyFormat("int Foo() {\n" + " a12345 = @{(Foo *)a12345 : @(a12345)};\n" + "}"); + Style.SpacesInContainerLiterals = false; + verifyFormat("int Foo() {\n" + " b12345 = @{b12345: b12345};\n" + "}"); + verifyFormat("int Foo() {\n" + " b12345 = @{(Foo *)b12345: @(b12345)};\n" + "}"); + Style.SpacesInContainerLiterals = true; Style = getGoogleStyle(FormatStyle::LK_ObjC); verifyFormat( @@ -1055,6 +1070,21 @@ verifyFormat("[someFunction someLongParameter:@[\n" " NSBundle.mainBundle.infoDictionary[@\"a\"]\n" "]];"); + Style.ColumnLimit = 40; + verifyFormat("int Foo() {\n" + " a12345 = @[ a12345, a12345 ];\n" + "}"); + verifyFormat("int Foo() {\n" + " a123 = @[ (Foo *)a12345, @(a12345) ];\n" + "}"); + Style.SpacesInContainerLiterals = false; + verifyFormat("int Foo() {\n" + " b12345 = @[b12345, b12345];\n" + "}"); + verifyFormat("int Foo() {\n" + " b12345 = @[(Foo *)b12345, @(b12345)];\n" + "}"); + Style.SpacesInContainerLiterals = true; Style.ColumnLimit = 20; // We can't break string literals inside NSArray literals // (that raises -Wobjc-string-concatenation). Index: lib/Format/TokenAnnotator.cpp === --- lib/Format/TokenAnnotator.cpp +++ lib/Format/TokenAnnotator.cpp @@ -2480,6 +2480,10 @@ return false; if (Left.is(TT_TemplateCloser) && Right.is(tok::l_square)) return false; + if (Right.is(tok::r_brace) && Right.MatchingParen && +
[PATCH] D44816: [clang-format] Do not insert space before closing brace in ObjC dict literal
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Generally looks good, one minor simplification. Comment at: lib/Format/TokenAnnotator.cpp:2484 + if (Right.is(tok::r_brace) && Right.MatchingParen && + Right.MatchingParen->is(TT_DictLiteral) && + Right.MatchingParen->Previous && Could you use Right.MatchingParen->endsSequence(TT_DictLiteral, tok::at) here? Repository: rC Clang https://reviews.llvm.org/D44816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44816: [clang-format] Do not insert space before closing brace in ObjC dict literal
benhamilton created this revision. benhamilton added reviewers: djasper, jolesiak, Wizard. Herald added subscribers: cfe-commits, klimek. 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 Repository: rC Clang https://reviews.llvm.org/D44816 Files: lib/Format/TokenAnnotator.cpp unittests/Format/FormatTestObjC.cpp Index: unittests/Format/FormatTestObjC.cpp === --- unittests/Format/FormatTestObjC.cpp +++ unittests/Format/FormatTestObjC.cpp @@ -1000,6 +1000,21 @@ " ( id)a : ( id),\n" " ( id)aa : ( id)aa,\n" "};"); + Style.ColumnLimit = 40; + verifyFormat("int Foo() {\n" + " a12345 = @{a12345 : a12345};\n" + "}"); + verifyFormat("int Foo() {\n" + " a12345 = @{(Foo *)a12345 : @(a12345)};\n" + "}"); + Style.SpacesInContainerLiterals = false; + verifyFormat("int Foo() {\n" + " b12345 = @{b12345: b12345};\n" + "}"); + verifyFormat("int Foo() {\n" + " b12345 = @{(Foo *)b12345: @(b12345)};\n" + "}"); + Style.SpacesInContainerLiterals = true; Style = getGoogleStyle(FormatStyle::LK_ObjC); verifyFormat( @@ -1055,6 +1070,21 @@ verifyFormat("[someFunction someLongParameter:@[\n" " NSBundle.mainBundle.infoDictionary[@\"a\"]\n" "]];"); + Style.ColumnLimit = 40; + verifyFormat("int Foo() {\n" + " a12345 = @[ a12345, a12345 ];\n" + "}"); + verifyFormat("int Foo() {\n" + " a123 = @[ (Foo *)a12345, @(a12345) ];\n" + "}"); + Style.SpacesInContainerLiterals = false; + verifyFormat("int Foo() {\n" + " b12345 = @[b12345, b12345];\n" + "}"); + verifyFormat("int Foo() {\n" + " b12345 = @[(Foo *)b12345, @(b12345)];\n" + "}"); + Style.SpacesInContainerLiterals = true; Style.ColumnLimit = 20; // We can't break string literals inside NSArray literals // (that raises -Wobjc-string-concatenation). Index: lib/Format/TokenAnnotator.cpp === --- lib/Format/TokenAnnotator.cpp +++ lib/Format/TokenAnnotator.cpp @@ -2480,6 +2480,12 @@ return false; if (Left.is(TT_TemplateCloser) && Right.is(tok::l_square)) return false; + if (Right.is(tok::r_brace) && Right.MatchingParen && + Right.MatchingParen->is(TT_DictLiteral) && + Right.MatchingParen->Previous && + Right.MatchingParen->Previous->is(tok::at)) +// Objective-C dictionary literal -> no space before closing brace. +return false; return true; } Index: unittests/Format/FormatTestObjC.cpp === --- unittests/Format/FormatTestObjC.cpp +++ unittests/Format/FormatTestObjC.cpp @@ -1000,6 +1000,21 @@ " ( id)a : ( id),\n" " ( id)aa : ( id)aa,\n" "};"); + Style.ColumnLimit = 40; + verifyFormat("int Foo() {\n" + " a12345 = @{a12345 : a12345};\n" + "}"); + verifyFormat("int Foo() {\n" + " a12345 = @{(Foo *)a12345 : @(a12345)};\n" + "}"); + Style.SpacesInContainerLiterals = false; + verifyFormat("int Foo() {\n" + " b12345 = @{b12345: b12345};\n" + "}"); + verifyFormat("int Foo() {\n" + " b12345 = @{(Foo *)b12345: @(b12345)};\n" + "}"); + Style.SpacesInContainerLiterals = true; Style = getGoogleStyle(FormatStyle::LK_ObjC); verifyFormat( @@ -1055,6 +1070,21 @@ verifyFormat("[someFunction someLongParameter:@[\n" " NSBundle.mainBundle.infoDictionary[@\"a\"]\n" "]];"); + Style.ColumnLimit = 40; + verifyFormat("int Foo() {\n" + " a12345 = @[ a12345, a12345 ];\n" + "}"); + verifyFormat("int Foo() {\n" + " a123 = @[ (Foo *)a12345, @(a12345) ];\n" + "}"); +