[PATCH] D47195: [clang-format] Fix ObjC message arguments handling
benhamilton added a comment. Can you update the diff description to reflect that it includes the original change? Repository: rC Clang https://reviews.llvm.org/D47195 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47195: [clang-format] Fix ObjC message arguments handling
jolesiak updated this revision to Diff 148038.
jolesiak added a comment.
Rebase.
Repository:
rC Clang
https://reviews.llvm.org/D47195
Files:
lib/Format/ContinuationIndenter.cpp
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -796,6 +796,33 @@
verifyFormat("[((Foo *)foo) bar];");
verifyFormat("[((Foo *)foo) bar:1 blech:2];");
+ // Message receiver taking multiple lines.
+ Style.ColumnLimit = 20;
+ // Non-corner case.
+ verifyFormat("[[object block:^{\n"
+ " return 42;\n"
+ "}] a:42 b:42];");
+ // Arguments just fit into one line.
+ verifyFormat("[[object block:^{\n"
+ " return 42;\n"
+ "}] aaa:42 b:42];");
+ // Arguments just over a column limit.
+ verifyFormat("[[object block:^{\n"
+ " return 42;\n"
+ "}] aaa:42\n"
+ "bb:42];");
+
+ // Arguments do not fit into one line with a receiver.
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42];\n");
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42\n"
+ "c:42];\n");
+ verifyFormat("[[obj aa:42\n"
+ " b:42]\n"
+ "cc:42\n"
+ " d:42];");
+
Style.ColumnLimit = 70;
verifyFormat(
"void f() {\n"
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -1386,6 +1386,29 @@
(Current.is(tok::greater) && Current.is(TT_DictLiteral
State.Stack.pop_back();
+ // Reevaluate whether ObjC message arguments fit into one line.
+ // If a receiver spans multiple lines, e.g.:
+ // [[object block:^{
+ // return 42;
+ // }] a:42 b:42];
+ // BreakBeforeParameter is calculated based on an incorrect assumption
+ // (it is checked whether the whole expression fits into one line without
+ // considering a line break inside a message receiver).
+ // We check whether arguements fit after receiver scope closer (into the same
+ // line).
+ if (Current.MatchingParen && Current.MatchingParen->Previous) {
+const FormatToken = *Current.MatchingParen->Previous;
+if (CurrentScopeOpener.is(TT_ObjCMethodExpr) &&
+CurrentScopeOpener.MatchingParen) {
+ int NecessarySpaceInLine =
+ getLengthToMatchingParen(CurrentScopeOpener, State.Stack) +
+ CurrentScopeOpener.TotalLength - Current.TotalLength - 1;
+ if (State.Column + Current.ColumnWidth + NecessarySpaceInLine <=
+ Style.ColumnLimit)
+State.Stack.back().BreakBeforeParameter = false;
+}
+ }
+
if (Current.is(tok::r_square)) {
// If this ends the array subscript expr, reset the corresponding value.
const FormatToken *NextNonComment = Current.getNextNonComment();
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -796,6 +796,33 @@
verifyFormat("[((Foo *)foo) bar];");
verifyFormat("[((Foo *)foo) bar:1 blech:2];");
+ // Message receiver taking multiple lines.
+ Style.ColumnLimit = 20;
+ // Non-corner case.
+ verifyFormat("[[object block:^{\n"
+ " return 42;\n"
+ "}] a:42 b:42];");
+ // Arguments just fit into one line.
+ verifyFormat("[[object block:^{\n"
+ " return 42;\n"
+ "}] aaa:42 b:42];");
+ // Arguments just over a column limit.
+ verifyFormat("[[object block:^{\n"
+ " return 42;\n"
+ "}] aaa:42\n"
+ "bb:42];");
+
+ // Arguments do not fit into one line with a receiver.
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42];\n");
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42\n"
+ "c:42];\n");
+ verifyFormat("[[obj aa:42\n"
+ " b:42]\n"
+ "cc:42\n"
+ " d:42];");
+
Style.ColumnLimit = 70;
verifyFormat(
"void f() {\n"
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -1386,6 +1386,29 @@
(Current.is(tok::greater) && Current.is(TT_DictLiteral
State.Stack.pop_back();
+ // Reevaluate whether ObjC message arguments fit into one line.
+ // If a receiver spans multiple lines, e.g.:
+ // [[object block:^{
+ // return 42;
+ // }] a:42 b:42];
+ // BreakBeforeParameter is calculated based on an incorrect assumption
+ // (it is checked whether the whole
[PATCH] D47195: [clang-format] Fix ObjC message arguments handling
jolesiak added inline comments.
Comment at: unittests/Format/FormatTestObjC.cpp:815
+
+ // No line break before closing receiver's scope.
+ verifyFormat("[[obj a:42] a:42\n"
krasimir wrote:
> jolesiak wrote:
> > krasimir wrote:
> > > What's the receiver's scope in this comment referring to?
> > > Also, how would the old test cases be formatted?
> > For a receiver: `[obj a:42]` I meant `]` as a token closing the scope.
> > I'll rephrase the comment to be more precise.
> >
> > Old tests were introduced in D46879. After this change the formatting will
> > be the same as it was before D46879, i.e. the same as for last test touched
> > in this change:
> > ```
> > [[obj aa:42
> >b:42]
> > cc:42
> > d:42];
> > ```
> > even if
> > ```
> > [[obj aa:42
> >b:42]
> > cc:42 d:42];
> > ```
> > satisfies the column limit.
> Ah, I think get it now: the new code should only apply to after object
> blocks and not after object receivers? Is this the intention?
The intention is to put arguments into one line if they fit but only in the
same line as last character of a receiver expression, e.g.:
```
[[object block:^{
return 42;
}] aa:42 bb:42];
```
instead of
```
[[object block:^{
return 42;
}] aa:42
bb:42];
```
but not
```
[[obj a:42]
a:42 b:42];
```
I think it gets a little bit too complicated for no reason. Let me revert
D46879 and rebase this change.
Repository:
rC Clang
https://reviews.llvm.org/D47195
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47195: [clang-format] Fix ObjC message arguments handling
krasimir added inline comments.
Comment at: unittests/Format/FormatTestObjC.cpp:815
+
+ // No line break before closing receiver's scope.
+ verifyFormat("[[obj a:42] a:42\n"
jolesiak wrote:
> krasimir wrote:
> > What's the receiver's scope in this comment referring to?
> > Also, how would the old test cases be formatted?
> For a receiver: `[obj a:42]` I meant `]` as a token closing the scope.
> I'll rephrase the comment to be more precise.
>
> Old tests were introduced in D46879. After this change the formatting will be
> the same as it was before D46879, i.e. the same as for last test touched in
> this change:
> ```
> [[obj aa:42
>b:42]
> cc:42
> d:42];
> ```
> even if
> ```
> [[obj aa:42
>b:42]
> cc:42 d:42];
> ```
> satisfies the column limit.
Ah, I think get it now: the new code should only apply to after object blocks
and not after object receivers? Is this the intention?
Repository:
rC Clang
https://reviews.llvm.org/D47195
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47195: [clang-format] Fix ObjC message arguments handling
jolesiak added inline comments.
Comment at: unittests/Format/FormatTestObjC.cpp:815
+
+ // No line break before closing receiver's scope.
+ verifyFormat("[[obj a:42] a:42\n"
krasimir wrote:
> What's the receiver's scope in this comment referring to?
> Also, how would the old test cases be formatted?
For a receiver: `[obj a:42]` I meant `]` as a token closing the scope.
I'll rephrase the comment to be more precise.
Old tests were introduced in D46879. After this change the formatting will be
the same as it was before D46879, i.e. the same as for last test touched in
this change:
```
[[obj aa:42
b:42]
cc:42
d:42];
```
even if
```
[[obj aa:42
b:42]
cc:42 d:42];
```
satisfies the column limit.
Repository:
rC Clang
https://reviews.llvm.org/D47195
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47195: [clang-format] Fix ObjC message arguments handling
benhamilton accepted this revision. benhamilton added a comment. This revision is now accepted and ready to land. Please address @krasimir's comments, but looks good to me. Repository: rC Clang https://reviews.llvm.org/D47195 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47195: [clang-format] Fix ObjC message arguments handling
krasimir added inline comments.
Comment at: unittests/Format/FormatTestObjC.cpp:815
+
+ // No line break before closing receiver's scope.
+ verifyFormat("[[obj a:42] a:42\n"
What's the receiver's scope in this comment referring to?
Also, how would the old test cases be formatted?
Repository:
rC Clang
https://reviews.llvm.org/D47195
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47195: [clang-format] Fix ObjC message arguments handling
jolesiak created this revision.
Herald added subscribers: cfe-commits, klimek.
Repository:
rC Clang
https://reviews.llvm.org/D47195
Files:
lib/Format/ContinuationIndenter.cpp
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -811,19 +811,17 @@
" return 42;\n"
"}] aaa:42\n"
"bb:42];");
- // Non-corner case.
- verifyFormat("[[object aaa:42\n"
- " b:42]\n"
- "cc:42 d:42];");
- // Arguments just fit into one line.
- verifyFormat("[[object aaa:42\n"
- " b:42]\n"
- "cc:42 d:42];");
- // Arguments just over a column limit.
- verifyFormat("[[object aaa:42\n"
+
+ // No line break before closing receiver's scope.
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42];\n");
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42\n"
+ "c:42];\n");
+ verifyFormat("[[obj aa:42\n"
" b:42]\n"
- "cc:42\n"
- "dd:42];");
+ "cc:42\n"
+ " d:42];");
Style.ColumnLimit = 70;
verifyFormat(
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -1073,34 +1073,8 @@
if (Current.isMemberAccess())
State.Stack.back().StartOfFunctionCall =
!Current.NextOperator ? 0 : State.Column;
- if (Current.is(TT_SelectorName) &&
- !State.Stack.back().ObjCSelectorNameFound) {
+ if (Current.is(TT_SelectorName))
State.Stack.back().ObjCSelectorNameFound = true;
-
-// Reevaluate whether ObjC message arguments fit into one line.
-// If a receiver spans multiple lines, e.g.:
-// [[object block:^{
-// return 42;
-// }] a:42 b:42];
-// BreakBeforeParameter is calculated based on an incorrect assumption
-// (it is checked whether the whole expression fits into one line without
-// considering a line break inside a message receiver).
-if (Current.Previous && Current.Previous->closesScope() &&
-Current.Previous->MatchingParen &&
-Current.Previous->MatchingParen->Previous) {
- const FormatToken =
- *Current.Previous->MatchingParen->Previous;
- if (CurrentScopeOpener.is(TT_ObjCMethodExpr) &&
- CurrentScopeOpener.MatchingParen) {
-int NecessarySpaceInLine =
-getLengthToMatchingParen(CurrentScopeOpener, State.Stack) +
-CurrentScopeOpener.TotalLength - Current.TotalLength - 1;
-if (State.Column + Current.ColumnWidth + NecessarySpaceInLine <=
-Style.ColumnLimit)
- State.Stack.back().BreakBeforeParameter = false;
- }
-}
- }
if (Current.is(TT_CtorInitializerColon) &&
Style.BreakConstructorInitializers != FormatStyle::BCIS_AfterColon) {
// Indent 2 from the column, so:
@@ -1412,6 +1386,29 @@
(Current.is(tok::greater) && Current.is(TT_DictLiteral
State.Stack.pop_back();
+ // Reevaluate whether ObjC message arguments fit into one line.
+ // If a receiver spans multiple lines, e.g.:
+ // [[object block:^{
+ // return 42;
+ // }] a:42 b:42];
+ // BreakBeforeParameter is calculated based on an incorrect assumption
+ // (it is checked whether the whole expression fits into one line without
+ // considering a line break inside a message receiver).
+ // We check whether arguements fit after receiver scope closer (into the same
+ // line).
+ if (Current.MatchingParen && Current.MatchingParen->Previous) {
+const FormatToken = *Current.MatchingParen->Previous;
+if (CurrentScopeOpener.is(TT_ObjCMethodExpr) &&
+CurrentScopeOpener.MatchingParen) {
+ int NecessarySpaceInLine =
+ getLengthToMatchingParen(CurrentScopeOpener, State.Stack) +
+ CurrentScopeOpener.TotalLength - Current.TotalLength - 1;
+ if (State.Column + Current.ColumnWidth + NecessarySpaceInLine <=
+ Style.ColumnLimit)
+State.Stack.back().BreakBeforeParameter = false;
+}
+ }
+
if (Current.is(tok::r_square)) {
// If this ends the array subscript expr, reset the corresponding value.
const FormatToken *NextNonComment = Current.getNextNonComment();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
