[PATCH] D50535: Fix selective formatting of ObjC scope
jolesiak added a comment.
Nice! That looks very promising.
It still fails when we pass a longer range (maybe mimicking `PreviousRBrace`
will help), e.g.:
Base code:
@protocol A
@optional
// comment
- (void)f;
@end
MACRO
formatted with `clang-format -lines=3:6 file.m` gives:
@protocol A
@optional
// comment
- (void)f;
@end
MACRO
This is not happening for Cpp, e.g.:
Base code:
class A {
void f ();
// comment
void g ();
};
MACRO
running `clang-format -lines=3:6 file.cpp` gives:
class A {
void f ();
// comment
void g ();
};
MACRO
I think it would be good to add all these tests to unit tests, but I see that
an interface to pass line ranges is not very pleasant.
Repository:
rC Clang
https://reviews.llvm.org/D50535
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46879: [clang-format] Fix putting ObjC message arguments in one line for multiline receiver
jolesiak created this revision.
Herald added subscribers: cfe-commits, klimek.
Currently BreakBeforeParameter is set to true everytime message receiver
spans multiple lines, e.g.:
[[object block:^{
return 42;
}] aa:42 bb:42];
will be formatted:
[[object block:^{
return 42;
}] aa:42
bb:42];
even though arguments could fit into one line. This change fixes this
behavior.
Test Plan:
make -j12 FormatTests && tools/clang/unittests/Format/FormatTests
Repository:
rC Clang
https://reviews.llvm.org/D46879
Files:
lib/Format/ContinuationIndenter.cpp
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -792,6 +792,35 @@
" a = 42;\n"
"}];");
+ // 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];");
+ // 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"
+ " b:42]\n"
+ "cc:42\n"
+ "dd:42];");
+
Style.ColumnLimit = 70;
verifyFormat(
"void f() {\n"
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -1067,8 +1067,34 @@
if (Current.isMemberAccess())
State.Stack.back().StartOfFunctionCall =
!Current.NextOperator ? 0 : State.Column;
- if (Current.is(TT_SelectorName))
+ if (Current.is(TT_SelectorName) &&
+ !State.Stack.back().ObjCSelectorNameFound) {
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 &CurrentScopeOpener =
+ *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:
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -792,6 +792,35 @@
" a = 42;\n"
"}];");
+ // 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];");
+ // 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"
+ " b
[PATCH] D46879: [clang-format] Fix putting ObjC message arguments in one line for multiline receiver
This revision was automatically updated to reflect the committed changes.
Closed by commit rC332582: [clang-format] Fix putting ObjC message arguments in
one line for multiline… (authored by jolesiak, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D46879?vs=146818&id=147265#toc
Repository:
rC Clang
https://reviews.llvm.org/D46879
Files:
lib/Format/ContinuationIndenter.cpp
unittests/Format/FormatTestObjC.cpp
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -1073,8 +1073,34 @@
if (Current.isMemberAccess())
State.Stack.back().StartOfFunctionCall =
!Current.NextOperator ? 0 : State.Column;
- if (Current.is(TT_SelectorName))
+ if (Current.is(TT_SelectorName) &&
+ !State.Stack.back().ObjCSelectorNameFound) {
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 &CurrentScopeOpener =
+ *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:
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -792,6 +792,35 @@
" a = 42;\n"
"}];");
+ // 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];");
+ // 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"
+ " b:42]\n"
+ "cc:42\n"
+ "dd:42];");
+
Style.ColumnLimit = 70;
verifyFormat(
"void f() {\n"
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -1073,8 +1073,34 @@
if (Current.isMemberAccess())
State.Stack.back().StartOfFunctionCall =
!Current.NextOperator ? 0 : State.Column;
- if (Current.is(TT_SelectorName))
+ if (Current.is(TT_SelectorName) &&
+ !State.Stack.back().ObjCSelectorNameFound) {
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 &CurrentScopeOpener =
+ *Current.Previous->MatchingParen->Previous;
+ if (CurrentScopeOpener.is(TT_ObjCMethodExpr) &&
+ CurrentScopeOpener.MatchingParen) {
+int NecessarySpaceInLine =
+getLengthToMatch
[PATCH] D46879: [clang-format] Fix putting ObjC message arguments in one line for multiline receiver
This revision was automatically updated to reflect the committed changes.
Closed by commit rL332582: [clang-format] Fix putting ObjC message arguments in
one line for multiline… (authored by jolesiak, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D46879
Files:
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/unittests/Format/FormatTestObjC.cpp
Index: cfe/trunk/lib/Format/ContinuationIndenter.cpp
===
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp
@@ -1073,8 +1073,34 @@
if (Current.isMemberAccess())
State.Stack.back().StartOfFunctionCall =
!Current.NextOperator ? 0 : State.Column;
- if (Current.is(TT_SelectorName))
+ if (Current.is(TT_SelectorName) &&
+ !State.Stack.back().ObjCSelectorNameFound) {
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 &CurrentScopeOpener =
+ *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:
Index: cfe/trunk/unittests/Format/FormatTestObjC.cpp
===
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp
@@ -792,6 +792,35 @@
" a = 42;\n"
"}];");
+ // 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];");
+ // 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"
+ " b:42]\n"
+ "cc:42\n"
+ "dd:42];");
+
Style.ColumnLimit = 70;
verifyFormat(
"void f() {\n"
Index: cfe/trunk/lib/Format/ContinuationIndenter.cpp
===
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp
@@ -1073,8 +1073,34 @@
if (Current.isMemberAccess())
State.Stack.back().StartOfFunctionCall =
!Current.NextOperator ? 0 : State.Column;
- if (Current.is(TT_SelectorName))
+ if (Current.is(TT_SelectorName) &&
+ !State.Stack.back().ObjCSelectorNameFound) {
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 &CurrentScopeOpener =
+ *Current.Previous->MatchingParen->Previous;
+ if (CurrentScopeOpener.is(TT_ObjCMethodExpr) &&
+ CurrentScopeOpener.MatchingParen) {
[PATCH] D47028: [clang-format/ObjC] Correctly annotate single-component ObjC method invocations
jolesiak accepted this revision. jolesiak added inline comments. This revision is now accepted and ready to land. Comment at: lib/Format/TokenAnnotator.cpp:510 +CurrentToken->Previous->Type = TT_SelectorName; + } // determineStarAmpUsage() thinks that '*' '[' is allocating an I'd remove braces for consistency. Repository: rC Clang https://reviews.llvm.org/D47028 ___ 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 &CurrentScopeOpener =
- *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 &CurrentScopeOpener = *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
[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] D47205: Revert "[clang-format] Fix putting ObjC message arguments in one line for multiline receiver"
jolesiak created this revision.
Herald added subscribers: cfe-commits, klimek.
Repository:
rC Clang
https://reviews.llvm.org/D47205
Files:
lib/Format/ContinuationIndenter.cpp
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -796,35 +796,6 @@
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];");
- // 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"
- " b:42]\n"
- "cc:42\n"
- "dd:42];");
-
Style.ColumnLimit = 70;
verifyFormat(
"void f() {\n"
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 &CurrentScopeOpener =
- *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:
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -796,35 +796,6 @@
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];");
- // 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"
- " b:42]\n"
- "cc:42\n"
- "dd:42];");
-
Style.ColumnLimit = 70;
verifyFormat(
"void f() {\n"
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationInde
[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] D47205: Revert "[clang-format] Fix putting ObjC message arguments in one line for multiline receiver"
This revision was automatically updated to reflect the committed changes.
Closed by commit rC332998: Revert "[clang-format] Fix putting ObjC message
arguments in one line for… (authored by jolesiak, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D47205?vs=148030&id=148032#toc
Repository:
rC Clang
https://reviews.llvm.org/D47205
Files:
lib/Format/ContinuationIndenter.cpp
unittests/Format/FormatTestObjC.cpp
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 &CurrentScopeOpener =
- *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:
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -796,35 +796,6 @@
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];");
- // 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"
- " b:42]\n"
- "cc:42\n"
- "dd:42];");
-
Style.ColumnLimit = 70;
verifyFormat(
"void f() {\n"
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 &CurrentScopeOpener =
- *Current.Previous->MatchingParen->Previous;
- if (CurrentScopeOpener.is(TT_ObjCMethodExpr) &&
- CurrentScopeOpener.MatchingParen) {
-int NecessarySpaceInLine =
-
[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 &CurrentScopeOpener = *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
[PATCH] D47195: [clang-format] Fix putting ObjC message arguments in one line for multiline receiver
jolesiak updated this revision to Diff 148169.
jolesiak added a comment.
- Add test
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,41 @@
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 just fit into one line.
+ Style.ColumnLimit = 23;
+ verifyFormat("[[obj a:42\n"
+ " b:42\n"
+ " c:42\n"
+ " d:42] e:42 f:42];");
+
+ // Arguments do not fit into one line with a receiver.
+ Style.ColumnLimit = 20;
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42];");
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42\n"
+ "c:42];");
+ 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 &CurrentScopeOpener = *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,41 @@
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 just fit into one line.
+ Style.ColumnLimit = 23;
+ verifyFormat("[[obj a:42\n"
+ " b:42\n"
+ " c:42\n"
+ " d:42] e:42 f:42];");
+
+ // Arguments do not fit into one line with a receiver.
+ Style.ColumnLimit = 20;
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42];");
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42\n"
+ "c:42];");
+ 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/ContinuationI
[PATCH] D47195: [clang-format] Fix putting ObjC message arguments in one line for multiline receiver
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:
> > > 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.
> In that case, would this be allowed?
> ```
> // limit: V
> [[obj a:42
> b:42
> c:42
> d:42] e:42 f:42]
> ```
Yes, I added this test.
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] D47095: [clang-format/ObjC] Correctly parse Objective-C methods with 'class' in name
jolesiak added inline comments.
Comment at: lib/Format/UnwrappedLineParser.cpp:2137
+ do {
+if (FormatTok->Tok.isOneOf(tok::semi, tok::r_brace)) {
+ nextToken();
`tok::r_brace` could be skiped - see comment to line 2143.
Comment at: lib/Format/UnwrappedLineParser.cpp:2143
+ parseBlock(/*MustBeDeclaration=*/false);
+ addUnwrappedLine();
+} else {
We have to add `return` after `addUnwrappedLine` as `parseBlock` does consume
`tok::r_brace`. Without `return` we will consume tokens after `}`. This problem
will rarely occur as most lines end with `tok::semi` or `tok::r_brace` and it
will be terminated properly (however maybe not handled properly as we just skip
every token in `else`) by `if` branch.
Test like:
```
@implementation Foo
- (foo)foo {
}
@end
@implementation Bar
- (bar)bar {
}
@end
```
will distinguish version with `return` from one without. Therefore, I think we
should add it.
Repository:
rC Clang
https://reviews.llvm.org/D47095
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47095: [clang-format/ObjC] Correctly parse Objective-C methods with 'class' in name
jolesiak added a comment. LGTM, would be nice though if somebody else took a look (@klimek ?). Repository: rC Clang https://reviews.llvm.org/D47095 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47195: [clang-format] Fix putting ObjC message arguments in one line for multiline receiver
jolesiak added a comment. Does it look fine now @krasimir ? 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 putting ObjC message arguments in one line for multiline receiver
This revision was automatically updated to reflect the committed changes.
Closed by commit rC333171: [clang-format] Fix putting ObjC message arguments in
one line for multiline… (authored by jolesiak, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D47195?vs=148169&id=148370#toc
Repository:
rC Clang
https://reviews.llvm.org/D47195
Files:
lib/Format/ContinuationIndenter.cpp
unittests/Format/FormatTestObjC.cpp
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -1387,6 +1387,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 &CurrentScopeOpener = *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,41 @@
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 just fit into one line.
+ Style.ColumnLimit = 23;
+ verifyFormat("[[obj a:42\n"
+ " b:42\n"
+ " c:42\n"
+ " d:42] e:42 f:42];");
+
+ // Arguments do not fit into one line with a receiver.
+ Style.ColumnLimit = 20;
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42];");
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42\n"
+ "c:42];");
+ 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
@@ -1387,6 +1387,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 &CurrentScopeOpener = *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
[PATCH] D49580: [clang-format] Adding style option for absolute formatting
jolesiak added a comment. Sorry for the delay, I'll comment on that tomorrow. https://reviews.llvm.org/D49580 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49580: [clang-format] Adding style option for absolute formatting
jolesiak added a comment.
First of all, thanks, Arnaud, for looking into this.
Let's address the mentioned bug first.
Let's assume that we use `IndentReference = Relative`. I think that this
particular formatting bug is local (i.e. can be solved while processing a
`@protocol` block). Look at this example:
if (true) {
int a = 42;
int b = 42;
int c = 42;
}
int d = 42;
When we run `clang-format -lines=4:4` we get:
if (true) {
int a = 42;
int b = 42;
int c = 42;
}
int d = 42;
Please note that neither `}` nor `int d = 42` is indented.
But this is a different behavior from what we see after running `clang-format
-lines=3:3` on:
@protocol A
@optional
// comment
- (void)f;
@end
MACRO
the output is:
@protocol A
@optional
// comment
- (void)f;
@end
MACRO
Please note that `@end` is indented (has different indentation than
`@protocol`; `MACRO` is indented as well).
To clarify why this is undesired, `@end` ends `@protocol`, not `@optional`. By
the way, `clang-format` doesn't think that `@end` should match `@optional`,
check the output of an extended example:
@protocol A
@optional
// comment
- (void)f;
@end
MACRO
@end
Second `@end` still doesn't match the `@protocol`.
I think it's perfectly fine to allow users to have custom indentations if they
so choose. In this particular example, though, `clang-format` should stop
indenting when it reaches `@end`, giving the following output:
@protocol A
@optional
// comment
- (void)f;
@end
MACRO
Moving to the general case, I must say that introducing `IndentReference =
Relative/Absolute` is a very interesting approach. I can imagine situations
when somebody actually want to use `IndentReference = Absolute`. However,
adding an additional option comes at quite big cost and I think that in this
case it outweighs the benefits.
I think that a very good outcome could be to comment what is happening next to
`IndentTracker.adjustToUnmodifiedLine(TheLine);` line. E.g. "We adjust an
indentation to match the existing code to give users flexibility (instead of
ignoring it). It's their responsibility to provide a correct formatting of
lines they didn't initially change if these lines break formatting.".
https://reviews.llvm.org/D49580
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49580: [clang-format] Adding style option for absolute formatting
jolesiak added a comment. In https://reviews.llvm.org/D49580#1179924, @djasper wrote: > Ok, so IIUC, understanding that @end effective ends a section much like "}" > would address the currently observed problems? I think so. https://reviews.llvm.org/D49580 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49580: [clang-format] Adding style option for absolute formatting
jolesiak added a comment. Thanks! https://reviews.llvm.org/D49580 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42901: Test commit - fixing a comment.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL324338: Test commit - fixing a comment. (authored by
jolesiak, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D42901
Files:
cfe/trunk/lib/Format/ContinuationIndenter.h
Index: cfe/trunk/lib/Format/ContinuationIndenter.h
===
--- cfe/trunk/lib/Format/ContinuationIndenter.h
+++ cfe/trunk/lib/Format/ContinuationIndenter.h
@@ -314,8 +314,8 @@
/// the same token.
bool HasMultipleNestedBlocks : 1;
- // \brief The start of a nested block (e.g. lambda introducer in C++ or
- // "function" in JavaScript) is not wrapped to a new line.
+ /// \brief The start of a nested block (e.g. lambda introducer in C++ or
+ /// "function" in JavaScript) is not wrapped to a new line.
bool NestedBlockInlined : 1;
bool operator<(const ParenState &Other) const {
Index: cfe/trunk/lib/Format/ContinuationIndenter.h
===
--- cfe/trunk/lib/Format/ContinuationIndenter.h
+++ cfe/trunk/lib/Format/ContinuationIndenter.h
@@ -314,8 +314,8 @@
/// the same token.
bool HasMultipleNestedBlocks : 1;
- // \brief The start of a nested block (e.g. lambda introducer in C++ or
- // "function" in JavaScript) is not wrapped to a new line.
+ /// \brief The start of a nested block (e.g. lambda introducer in C++ or
+ /// "function" in JavaScript) is not wrapped to a new line.
bool NestedBlockInlined : 1;
bool operator<(const ParenState &Other) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42493: [clang-format] Fix ObjC message arguments formatting.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL324469: [clang-format] Fix ObjC message arguments
formatting. (authored by jolesiak, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D42493
Files:
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/lib/Format/FormatToken.h
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTestObjC.cpp
Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -411,6 +411,8 @@
if (Contexts.back().FirstObjCSelectorName) {
Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName =
Contexts.back().LongestObjCSelectorName;
+ Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts =
+ Left->ParameterCount;
if (Left->BlockParameterCount > 1)
Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName = 0;
}
@@ -424,6 +426,11 @@
TT_DesignatedInitializerLSquare)) {
Left->Type = TT_ObjCMethodExpr;
StartsObjCMethodExpr = true;
+ // ParameterCount might have been set to 1 before expression was
+ // recognized as ObjCMethodExpr (as '1 + number of commas' formula is
+ // used for other expression types). Parameter counter has to be,
+ // therefore, reset to 0.
+ Left->ParameterCount = 0;
Contexts.back().ColonIsObjCMethodExpr = true;
if (Parent && Parent->is(tok::r_paren))
Parent->Type = TT_CastRParen;
@@ -498,7 +505,10 @@
void updateParameterCount(FormatToken *Left, FormatToken *Current) {
if (Current->is(tok::l_brace) && Current->BlockKind == BK_Block)
++Left->BlockParameterCount;
-if (Current->is(tok::comma)) {
+if (Left->Type == TT_ObjCMethodExpr) {
+ if (Current->is(tok::colon))
+++Left->ParameterCount;
+} else if (Current->is(tok::comma)) {
++Left->ParameterCount;
if (!Left->Role)
Left->Role.reset(new CommaSeparatedList(Style));
Index: cfe/trunk/lib/Format/ContinuationIndenter.cpp
===
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp
@@ -266,6 +266,11 @@
return true;
if (Previous.is(tok::semi) && State.LineContainsContinuedForLoopSection)
return true;
+ if (Style.Language == FormatStyle::LK_ObjC &&
+ Current.ObjCSelectorNameParts > 1 &&
+ Current.startsSequence(TT_SelectorName, tok::colon, tok::caret)) {
+return true;
+ }
if ((startsNextParameter(Current, Style) || Previous.is(tok::semi) ||
(Previous.is(TT_TemplateCloser) && Current.is(TT_StartOfName) &&
Style.isCpp() &&
Index: cfe/trunk/lib/Format/FormatToken.h
===
--- cfe/trunk/lib/Format/FormatToken.h
+++ cfe/trunk/lib/Format/FormatToken.h
@@ -240,6 +240,10 @@
/// e.g. because several of them are block-type.
unsigned LongestObjCSelectorName = 0;
+ /// \brief How many parts ObjC selector have (i.e. how many parameters method
+ /// has).
+ unsigned ObjCSelectorNameParts = 0;
+
/// \brief Stores the number of required fake parentheses and the
/// corresponding operator precedence.
///
Index: cfe/trunk/unittests/Format/FormatTestObjC.cpp
===
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp
@@ -693,6 +693,39 @@
"ofSize:aa:bbb\n"
" atOrigin:cc:dd];");
+ // Inline block as a first argument.
+ verifyFormat("[object justBlock:^{\n"
+ " a = 42;\n"
+ "}];");
+ verifyFormat("[object\n"
+ "justBlock:^{\n"
+ " a = 42;\n"
+ "}\n"
+ " notBlock:42\n"
+ "a:42];");
+ verifyFormat("[object\n"
+ "firstBlock:^{\n"
+ " a = 42;\n"
+ "}\n"
+ "blockWithLongerName:^{\n"
+ " a = 42;\n"
+ "}];");
+ verifyFormat("[object\n"
+ "blockWithLongerName:^{\n"
+ " a = 42;\n"
+ "}\n"
+ "secondBlock:^{\n"
+ " a = 42;\n"
+ "}];");
+ verifyFormat("[object\n"
+ "firstBlock:^{\n"
+ " a = 42;\n"
+ "}\n"
+ "notBlock:42\n"
+ "secondBlock:^{\n"
+ " a = 42;\n"
+ "}];");
+
Style.ColumnLimit = 70;
verifyFormat(
"void f()
[PATCH] D43124: Improve ObjC headers detection
jolesiak created this revision.
jolesiak added a reviewer: benhamilton.
Herald added subscribers: cfe-commits, klimek.
Improve ObjC headers detection by adding additional keywords.
Repository:
rC Clang
https://reviews.llvm.org/D43124
Files:
lib/Format/Format.cpp
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1441,6 +1441,7 @@
"NSArray",
"NSAttributedString",
"NSCache",
+"NSCalendar",
"NSCharacterSet",
"NSCountedSet",
"NSData",
@@ -1466,6 +1467,7 @@
"NSMutableString",
"NSNumber",
"NSNumberFormatter",
+"NSObject",
"NSOrderedSet",
"NSPoint",
"NSPointerArray",
@@ -1475,11 +1477,13 @@
"NSSet",
"NSSize",
"NSString",
+"NSTimeZone",
"NSUInteger",
"NSURL",
"NSURLComponents",
"NSURLQueryItem",
"NSUUID",
+"NSValue",
};
for (auto &Line : AnnotatedLines) {
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1441,6 +1441,7 @@
"NSArray",
"NSAttributedString",
"NSCache",
+"NSCalendar",
"NSCharacterSet",
"NSCountedSet",
"NSData",
@@ -1466,6 +1467,7 @@
"NSMutableString",
"NSNumber",
"NSNumberFormatter",
+"NSObject",
"NSOrderedSet",
"NSPoint",
"NSPointerArray",
@@ -1475,11 +1477,13 @@
"NSSet",
"NSSize",
"NSString",
+"NSTimeZone",
"NSUInteger",
"NSURL",
"NSURLComponents",
"NSURLQueryItem",
"NSUUID",
+"NSValue",
};
for (auto &Line : AnnotatedLines) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43124: Improve ObjC headers detection
jolesiak updated this revision to Diff 133608.
jolesiak added a comment.
Add NSBundle.
Repository:
rC Clang
https://reviews.llvm.org/D43124
Files:
lib/Format/Format.cpp
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1440,7 +1440,9 @@
"NSAffineTransform",
"NSArray",
"NSAttributedString",
+"NSBundle",
"NSCache",
+"NSCalendar",
"NSCharacterSet",
"NSCountedSet",
"NSData",
@@ -1466,6 +1468,7 @@
"NSMutableString",
"NSNumber",
"NSNumberFormatter",
+"NSObject",
"NSOrderedSet",
"NSPoint",
"NSPointerArray",
@@ -1475,11 +1478,13 @@
"NSSet",
"NSSize",
"NSString",
+"NSTimeZone",
"NSUInteger",
"NSURL",
"NSURLComponents",
"NSURLQueryItem",
"NSUUID",
+"NSValue",
};
for (auto &Line : AnnotatedLines) {
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1440,7 +1440,9 @@
"NSAffineTransform",
"NSArray",
"NSAttributedString",
+"NSBundle",
"NSCache",
+"NSCalendar",
"NSCharacterSet",
"NSCountedSet",
"NSData",
@@ -1466,6 +1468,7 @@
"NSMutableString",
"NSNumber",
"NSNumberFormatter",
+"NSObject",
"NSOrderedSet",
"NSPoint",
"NSPointerArray",
@@ -1475,11 +1478,13 @@
"NSSet",
"NSSize",
"NSString",
+"NSTimeZone",
"NSUInteger",
"NSURL",
"NSURLComponents",
"NSURLQueryItem",
"NSUUID",
+"NSValue",
};
for (auto &Line : AnnotatedLines) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43231: [clang-format] Refactor ObjC tests
jolesiak created this revision.
Herald added subscribers: cfe-commits, klimek.
Simplifies ObjC style tests.
Repository:
rC Clang
https://reviews.llvm.org/D43231
Files:
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -68,92 +68,84 @@
FormatStyle Style;
};
+// Checks whether language detected after running \p getStyle is expected.
+// \p Style cannot be a reference to const as casting to bool can change
+// internal state.
+void checkLanguage(llvm::Expected &Style,
+ FormatStyle::LanguageKind ExpectedLanguage) {
+ ASSERT_TRUE((bool)Style);
+ EXPECT_EQ(ExpectedLanguage, Style->Language);
+}
+
TEST(FormatTestObjCStyle, DetectsObjCInHeaders) {
auto Style = getStyle("LLVM", "a.h", "none", "@interface\n"
"- (id)init;");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+ checkLanguage(Style, FormatStyle::LK_ObjC);
Style = getStyle("LLVM", "a.h", "none", "@interface\n"
"+ (id)init;");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+ checkLanguage(Style, FormatStyle::LK_ObjC);
Style = getStyle("LLVM", "a.h", "none", "@interface\n"
"@end\n"
"//comment");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+ checkLanguage(Style, FormatStyle::LK_ObjC);
Style = getStyle("LLVM", "a.h", "none", "@interface\n"
"@end //comment");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+ checkLanguage(Style, FormatStyle::LK_ObjC);
// No recognizable ObjC.
Style = getStyle("LLVM", "a.h", "none", "void f() {}");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
+ checkLanguage(Style, FormatStyle::LK_Cpp);
Style = getStyle("{}", "a.h", "none", "@interface Foo\n@end\n");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+ checkLanguage(Style, FormatStyle::LK_ObjC);
Style = getStyle("{}", "a.h", "none",
"const int interface = 1;\nconst int end = 2;\n");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
+ checkLanguage(Style, FormatStyle::LK_Cpp);
Style = getStyle("{}", "a.h", "none", "@protocol Foo\n@end\n");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+ checkLanguage(Style, FormatStyle::LK_ObjC);
Style = getStyle("{}", "a.h", "none",
"const int protocol = 1;\nconst int end = 2;\n");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
-
- Style = getStyle("{}", "a.h", "none", "extern NSString *kFoo;\n");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+ checkLanguage(Style, FormatStyle::LK_Cpp);
Style =
getStyle("{}", "a.h", "none", "typedef NS_ENUM(NSInteger, Foo) {};\n");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+ checkLanguage(Style, FormatStyle::LK_ObjC);
Style = getStyle("{}", "a.h", "none", "enum Foo {};");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
-
- Style = getStyle("{}", "a.h", "none", "extern NSInteger Foo();\n");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+ checkLanguage(Style, FormatStyle::LK_Cpp);
Style =
getStyle("{}", "a.h", "none", "inline void Foo() { Log(@\"Foo\"); }\n");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+ checkLanguage(Style, FormatStyle::LK_ObjC);
Style =
getStyle("{}", "a.h", "none", "inline void Foo() { Log(\"Foo\"); }\n");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
+ checkLanguage(Style, FormatStyle::LK_Cpp);
Style =
getStyle("{}", "a.h", "none", "inline void Foo() { id = @[1, 2, 3]; }\n");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+ checkLanguage(Style, FormatStyle::LK_ObjC);
Style = getStyle("{}", "a.h", "none",
"inline void Foo() { id foo = @{1: 2, 3: 4, 5: 6}; }\n");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+ checkLanguage(Style, FormatStyle::LK_ObjC);
Style = getStyle("{}", "a.h", "none",
"inline void Foo() { int foo[] = {1, 2, 3}; }\n");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
+ checkLanguage(Style, FormatStyle::LK_Cpp);
+
+ // ObjC specific types.
+ Style = getStyle("{}", "a.h", "none", "extern NSString *kFoo;\n");
+ checkLangua
[PATCH] D43124: [clang-format] Improve ObjC headers detection
jolesiak planned changes to this revision. jolesiak added a comment. I'll add a few test cases after submitting https://reviews.llvm.org/D43231. Repository: rC Clang https://reviews.llvm.org/D43124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43231: [clang-format] Refactor ObjC tests
jolesiak abandoned this revision. jolesiak added a comment. In https://reviews.llvm.org/D43231#1006123, @krasimir wrote: > I don't believe this is needed: test fails before would fail at the line > where test instance is checked, and after they will fail at the checkLanguage > function. Thank you for the comment! That's a valid argument. I'm reverting as a macro is a no go. Repository: rC Clang https://reviews.llvm.org/D43231 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43124: [clang-format] Improve ObjC headers detection
jolesiak updated this revision to Diff 134180.
jolesiak added a comment.
This revision is now accepted and ready to land.
Added support for ObjC characteristic keywords starting a new line.
Repository:
rC Clang
https://reviews.llvm.org/D43124
Files:
lib/Format/Format.cpp
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -113,10 +113,6 @@
ASSERT_TRUE((bool)Style);
EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
- Style = getStyle("{}", "a.h", "none", "extern NSString *kFoo;\n");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
-
Style =
getStyle("{}", "a.h", "none", "typedef NS_ENUM(NSInteger, Foo) {};\n");
ASSERT_TRUE((bool)Style);
@@ -126,10 +122,6 @@
ASSERT_TRUE((bool)Style);
EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
- Style = getStyle("{}", "a.h", "none", "extern NSInteger Foo();\n");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
-
Style =
getStyle("{}", "a.h", "none", "inline void Foo() { Log(@\"Foo\"); }\n");
ASSERT_TRUE((bool)Style);
@@ -154,6 +146,23 @@
"inline void Foo() { int foo[] = {1, 2, 3}; }\n");
ASSERT_TRUE((bool)Style);
EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
+
+ // ObjC characteristic types.
+ Style = getStyle("{}", "a.h", "none", "extern NSString *kFoo;\n");
+ ASSERT_TRUE((bool)Style);
+ EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
+ Style = getStyle("{}", "a.h", "none", "extern NSInteger Foo();\n");
+ ASSERT_TRUE((bool)Style);
+ EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
+ Style = getStyle("{}", "a.h", "none", "NSObject *Foo();\n");
+ ASSERT_TRUE((bool)Style);
+ EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
+ Style = getStyle("{}", "a.h", "none", "NSSet *Foo();\n");
+ ASSERT_TRUE((bool)Style);
+ EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
}
TEST_F(FormatTestObjC, FormatObjCTryCatch) {
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1440,7 +1440,9 @@
"NSAffineTransform",
"NSArray",
"NSAttributedString",
+"NSBundle",
"NSCache",
+"NSCalendar",
"NSCharacterSet",
"NSCountedSet",
"NSData",
@@ -1466,6 +1468,7 @@
"NSMutableString",
"NSNumber",
"NSNumberFormatter",
+"NSObject",
"NSOrderedSet",
"NSPoint",
"NSPointerArray",
@@ -1475,17 +1478,19 @@
"NSSet",
"NSSize",
"NSString",
+"NSTimeZone",
"NSUInteger",
"NSURL",
"NSURLComponents",
"NSURLQueryItem",
"NSUUID",
+"NSValue",
};
for (auto &Line : AnnotatedLines) {
- for (FormatToken *FormatTok = Line->First->Next; FormatTok;
+ for (FormatToken *FormatTok = Line->First; FormatTok;
FormatTok = FormatTok->Next) {
-if ((FormatTok->Previous->is(tok::at) &&
+if ((FormatTok->Previous && FormatTok->Previous->is(tok::at) &&
(FormatTok->isObjCAtKeyword(tok::objc_interface) ||
FormatTok->isObjCAtKeyword(tok::objc_implementation) ||
FormatTok->isObjCAtKeyword(tok::objc_protocol) ||
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -113,10 +113,6 @@
ASSERT_TRUE((bool)Style);
EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
- Style = getStyle("{}", "a.h", "none", "extern NSString *kFoo;\n");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
-
Style =
getStyle("{}", "a.h", "none", "typedef NS_ENUM(NSInteger, Foo) {};\n");
ASSERT_TRUE((bool)Style);
@@ -126,10 +122,6 @@
ASSERT_TRUE((bool)Style);
EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
- Style = getStyle("{}", "a.h", "none", "extern NSInteger Foo();\n");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
-
Style =
getStyle("{}", "a.h", "none", "inline void Foo() { Log(@\"Foo\"); }\n");
ASSERT_TRUE((bool)Style);
@@ -154,6 +146,23 @@
"inline void Foo() { int foo[] = {1, 2, 3}; }\n");
ASSERT_TRUE((bool)Style);
EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
+
+ // ObjC characteristic types.
+ Style = getStyle("{}", "a.h", "none", "extern NSString *kFoo;\n");
+ ASSERT_TRUE((bool)Style);
+ EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
+ Style = getStyle("{}", "a.h", "none", "extern NSInteger Foo();\n");
+ ASSERT_TRUE((bool)Style);
+ EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
+ Style = getStyle("{}", "a.h", "none
[PATCH] D43124: [clang-format] Improve ObjC headers detection
This revision was automatically updated to reflect the committed changes.
Closed by commit rC325221: [clang-format] Improve ObjC headers detection
(authored by jolesiak, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D43124?vs=134180&id=134388#toc
Repository:
rC Clang
https://reviews.llvm.org/D43124
Files:
lib/Format/Format.cpp
unittests/Format/FormatTestObjC.cpp
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1440,7 +1440,9 @@
"NSAffineTransform",
"NSArray",
"NSAttributedString",
+"NSBundle",
"NSCache",
+"NSCalendar",
"NSCharacterSet",
"NSCountedSet",
"NSData",
@@ -1466,6 +1468,7 @@
"NSMutableString",
"NSNumber",
"NSNumberFormatter",
+"NSObject",
"NSOrderedSet",
"NSPoint",
"NSPointerArray",
@@ -1475,17 +1478,19 @@
"NSSet",
"NSSize",
"NSString",
+"NSTimeZone",
"NSUInteger",
"NSURL",
"NSURLComponents",
"NSURLQueryItem",
"NSUUID",
+"NSValue",
};
for (auto &Line : AnnotatedLines) {
- for (FormatToken *FormatTok = Line->First->Next; FormatTok;
+ for (FormatToken *FormatTok = Line->First; FormatTok;
FormatTok = FormatTok->Next) {
-if ((FormatTok->Previous->is(tok::at) &&
+if ((FormatTok->Previous && FormatTok->Previous->is(tok::at) &&
(FormatTok->isObjCAtKeyword(tok::objc_interface) ||
FormatTok->isObjCAtKeyword(tok::objc_implementation) ||
FormatTok->isObjCAtKeyword(tok::objc_protocol) ||
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -113,10 +113,6 @@
ASSERT_TRUE((bool)Style);
EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
- Style = getStyle("{}", "a.h", "none", "extern NSString *kFoo;\n");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
-
Style =
getStyle("{}", "a.h", "none", "typedef NS_ENUM(NSInteger, Foo) {};\n");
ASSERT_TRUE((bool)Style);
@@ -126,10 +122,6 @@
ASSERT_TRUE((bool)Style);
EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
- Style = getStyle("{}", "a.h", "none", "extern NSInteger Foo();\n");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
-
Style =
getStyle("{}", "a.h", "none", "inline void Foo() { Log(@\"Foo\"); }\n");
ASSERT_TRUE((bool)Style);
@@ -154,6 +146,23 @@
"inline void Foo() { int foo[] = {1, 2, 3}; }\n");
ASSERT_TRUE((bool)Style);
EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
+
+ // ObjC characteristic types.
+ Style = getStyle("{}", "a.h", "none", "extern NSString *kFoo;\n");
+ ASSERT_TRUE((bool)Style);
+ EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
+ Style = getStyle("{}", "a.h", "none", "extern NSInteger Foo();\n");
+ ASSERT_TRUE((bool)Style);
+ EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
+ Style = getStyle("{}", "a.h", "none", "NSObject *Foo();\n");
+ ASSERT_TRUE((bool)Style);
+ EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
+ Style = getStyle("{}", "a.h", "none", "NSSet *Foo();\n");
+ ASSERT_TRUE((bool)Style);
+ EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
}
TEST_F(FormatTestObjC, FormatObjCTryCatch) {
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1440,7 +1440,9 @@
"NSAffineTransform",
"NSArray",
"NSAttributedString",
+"NSBundle",
"NSCache",
+"NSCalendar",
"NSCharacterSet",
"NSCountedSet",
"NSData",
@@ -1466,6 +1468,7 @@
"NSMutableString",
"NSNumber",
"NSNumberFormatter",
+"NSObject",
"NSOrderedSet",
"NSPoint",
"NSPointerArray",
@@ -1475,17 +1478,19 @@
"NSSet",
"NSSize",
"NSString",
+"NSTimeZone",
"NSUInteger",
"NSURL",
"NSURLComponents",
"NSURLQueryItem",
"NSUUID",
+"NSValue",
};
for (auto &Line : AnnotatedLines) {
- for (FormatToken *FormatTok = Line->First->Next; FormatTok;
+ for (FormatToken *FormatTok = Line->First; FormatTok;
FormatTok = FormatTok->Next) {
-if ((FormatTok->Previous->is(tok::at) &&
+if ((FormatTok->Previous && FormatTok->Previous->is(tok::at) &&
(FormatTok->isObjCAtKeyword(tok::objc_interface) ||
FormatTok->isObjCAtKeyword(tok::objc_implementation) ||
FormatTok->isObjCAtKeyword(tok::objc_protocol) ||
Index: unittests/Format/FormatTestObjC.cpp
[PATCH] D41448: Fix counting parameters/arguments for ObjC.
jolesiak created this revision.
jolesiak added a reviewer: krasimir.
Herald added a subscriber: klimek.
[clang-format] Fix ParameterCount for ObjC.
Repository:
rC Clang
https://reviews.llvm.org/D41448
Files:
lib/Format/TokenAnnotator.cpp
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -414,6 +414,7 @@
TT_DesignatedInitializerLSquare)) {
Left->Type = TT_ObjCMethodExpr;
StartsObjCMethodExpr = true;
+ Left->ParameterCount = 0;
Contexts.back().ColonIsObjCMethodExpr = true;
if (Parent && Parent->is(tok::r_paren))
Parent->Type = TT_CastRParen;
@@ -486,7 +487,11 @@
void updateParameterCount(FormatToken *Left, FormatToken *Current) {
if (Current->is(tok::l_brace) && Current->BlockKind == BK_Block)
++Left->BlockParameterCount;
-if (Current->is(tok::comma)) {
+
+if (Left->Type == TT_ObjCMethodExpr) {
+ if (Current->is(tok::colon))
+++Left->ParameterCount;
+} else if (Current->is(tok::comma)) {
++Left->ParameterCount;
if (!Left->Role)
Left->Role.reset(new CommaSeparatedList(Style));
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -414,6 +414,7 @@
TT_DesignatedInitializerLSquare)) {
Left->Type = TT_ObjCMethodExpr;
StartsObjCMethodExpr = true;
+ Left->ParameterCount = 0;
Contexts.back().ColonIsObjCMethodExpr = true;
if (Parent && Parent->is(tok::r_paren))
Parent->Type = TT_CastRParen;
@@ -486,7 +487,11 @@
void updateParameterCount(FormatToken *Left, FormatToken *Current) {
if (Current->is(tok::l_brace) && Current->BlockKind == BK_Block)
++Left->BlockParameterCount;
-if (Current->is(tok::comma)) {
+
+if (Left->Type == TT_ObjCMethodExpr) {
+ if (Current->is(tok::colon))
+++Left->ParameterCount;
+} else if (Current->is(tok::comma)) {
++Left->ParameterCount;
if (!Left->Role)
Left->Role.reset(new CommaSeparatedList(Style));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43902: [clang-format] Don't detect C++11 attribute specifiers as ObjC
jolesiak requested changes to this revision.
jolesiak added inline comments.
This revision now requires changes to proceed.
Comment at: lib/Format/TokenAnnotator.cpp:323
+ const FormatToken *parseCpp11Attribute(const FormatToken *Tok,
+ bool NamespaceAllowed) {
I feel like it would be clearer (and more consistent) if we used const& here
and just make a copy inside function body.
I see no benefit of passing nullptr based on usage presented in this patch.
Comment at: lib/Format/TokenAnnotator.cpp:325
+ bool NamespaceAllowed) {
+if (!Tok || !Tok->isOneOf(tok::identifier, tok::ellipsis)) return Tok;
+Tok = Tok->Next;
For consistency reasons I wouldn't use one line ifs. Let's keep existing format
with line break and without braces.
Comment at: lib/Format/TokenAnnotator.cpp:337
+ParamToken = ParamToken->Next;
+ if (!ParamToken || ParamToken->isNot(tok::r_paren)) return nullptr;
+ Tok = ParamToken->Next;
Is second part of this condition necessary?
Comment at: lib/Format/TokenAnnotator.cpp:345
+ // never a valid Objective-C or Objective-C++ method invocation.
+ bool parseCpp11AttributeSpecifier(FormatToken *Tok) {
+if (!Style.isCpp()) return false;
Pointer should be to const type, but same as above, I feel like const& would be
a better choice.
Comment at: lib/Format/TokenAnnotator.cpp:351
+if (!AttributeToken) return false;
+// C++17 '[[using namespace: foo, bar(baz, blech)]]'
+if (AttributeToken->startsSequence(tok::kw_using, tok::identifier,
How about:
```
bool NamespaceAllowed;
if (AttributeToken->startsSequence(tok::kw_using, tok::identifier,
tok::colon)) {
// C++17 '[[using namespace: foo, bar(baz, blech)]]'
AttributeToken = AttributeToken->Next->Next->Next;
NamespaceAllowed = false;
} else {
// C++11 '[[namespace::foo, namespace::bar(baz, blech)]]'
NamespaceAllowed = true;
}
while (AttributeToken) {
AttributeToken =
parseCpp11Attribute(AttributeToken, NamespaceAllowed);
if (!AttributeToken || AttributeToken->isNot(tok::comma)) break;
AttributeToken = AttributeToken->Next;
}
```
Repository:
rC Clang
https://reviews.llvm.org/D43902
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44632: [clang-format] Add a few more Core Graphics identifiers to ObjC heuristic
jolesiak requested changes to this revision.
jolesiak added inline comments.
This revision now requires changes to proceed.
Comment at: unittests/Format/FormatTest.cpp:12099
EXPECT_EQ(FormatStyle::LK_ObjC, guessLanguage("foo", "@interface
Foo\n@end\n"));
+ EXPECT_EQ(FormatStyle::LK_ObjC, guessLanguage("foo.h", "int DoStuff(CGRect
rect);\n"));
+ EXPECT_EQ(
I know that it's violated in several places in this file (even in two of the
three lines above), but I feel like we should keep 80 char limit for column
width.
Repository:
rC Clang
https://reviews.llvm.org/D44632
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44632: [clang-format] Add a few more Core Graphics identifiers to ObjC heuristic
jolesiak added inline comments.
Comment at: lib/Format/Format.cpp:1450
// Keep this array sorted, since we are binary searching over it.
static constexpr llvm::StringLiteral FoundationIdentifiers[] = {
"CGFloat",
djasper wrote:
> I have concerns about this growing lists of things. Specifically:
> - Keeping it sorted is a maintenance concern.
> - Doing binary search for basically every identifier in a header seems an
> unnecessary waste.
>
> Could we just create a hash set of these?
It was a hash set initially: D42135
Changed in: D42189
Repository:
rC Clang
https://reviews.llvm.org/D44632
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44632: [clang-format] Add a few more Core Graphics identifiers to ObjC heuristic
jolesiak accepted this revision.
jolesiak added inline comments.
This revision is now accepted and ready to land.
Comment at: lib/Format/Format.cpp:1450
// Keep this array sorted, since we are binary searching over it.
static constexpr llvm::StringLiteral FoundationIdentifiers[] = {
"CGFloat",
benhamilton wrote:
> djasper wrote:
> > benhamilton wrote:
> > > jolesiak wrote:
> > > > djasper wrote:
> > > > > I have concerns about this growing lists of things. Specifically:
> > > > > - Keeping it sorted is a maintenance concern.
> > > > > - Doing binary search for basically every identifier in a header
> > > > > seems an unnecessary waste.
> > > > >
> > > > > Could we just create a hash set of these?
> > > > It was a hash set initially: D42135
> > > > Changed in: D42189
> > > Happy to do whatever folks recommend; I assume @krasimir's concern in
> > > D42189 was the startup cost of creating the (read-only) hash set.
> > >
> > > We can automate keeping this sorted with an `arc lint` check, they're
> > > quite easy to write:
> > >
> > > https://secure.phabricator.com/book/phabricator/article/arcanist_lint/
> > Krasimir clarified this to me offline. I have no concerns staying with
> > binary search here and for this patch so long as someone builds in an
> > assert that warns us when the strings here are not in the right order at
> > some point.
> Good call, added `assert(std::is_sorted(...))`.
>
> I tried `static_assert`, but `std::is_sorted()` is not `constexpr` until
> c++2x.
Checking this type of constraints in `arc lint` sounds rather weird. I like
this assert as testing private methods is painful.
Comment at: unittests/Format/FormatTest.cpp:12099
EXPECT_EQ(FormatStyle::LK_ObjC, guessLanguage("foo", "@interface
Foo\n@end\n"));
+ EXPECT_EQ(FormatStyle::LK_ObjC, guessLanguage("foo.h", "int DoStuff(CGRect
rect);\n"));
+ EXPECT_EQ(
benhamilton wrote:
> djasper wrote:
> > jolesiak wrote:
> > > I know that it's violated in several places in this file (even in two of
> > > the three lines above), but I feel like we should keep 80 char limit for
> > > column width.
> > Agreed. Please format this file with clang-format.
> Oops! Fixed. (Should we put in an `arc lint` check that code is correctly
> `clang-format`ted?)
I don't know what is convention here, but to me using clang-format to format
clang-format code sounds good. It's a little bit surprising that it's not the
case.
Repository:
rC Clang
https://reviews.llvm.org/D44632
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47527: Revert "[clang-format] Fix putting ObjC message arguments in one line for multiline receiver"
jolesiak created this revision.
Herald added subscribers: cfe-commits, klimek.
This reverts commit db9e5e9a616d7fdd4d1ba4c3b2cd89d8a0238533.
Repository:
rC Clang
https://reviews.llvm.org/D47527
Files:
lib/Format/ContinuationIndenter.cpp
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -796,41 +796,6 @@
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 just fit into one line.
- Style.ColumnLimit = 23;
- verifyFormat("[[obj a:42\n"
- " b:42\n"
- " c:42\n"
- " d:42] e:42 f:42];");
-
- // Arguments do not fit into one line with a receiver.
- Style.ColumnLimit = 20;
- verifyFormat("[[obj a:42] a:42\n"
- "b:42];");
- verifyFormat("[[obj a:42] a:42\n"
- "b:42\n"
- "c:42];");
- 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
@@ -1387,29 +1387,6 @@
(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 &CurrentScopeOpener = *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,41 +796,6 @@
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 just fit into one line.
- Style.ColumnLimit = 23;
- verifyFormat("[[obj a:42\n"
- " b:42\n"
- " c:42\n"
- " d:42] e:42 f:42];");
-
- // Arguments do not fit into one line with a receiver.
- Style.ColumnLimit = 20;
- verifyFormat("[[obj a:42] a:42\n"
- "b:42];");
- verifyFormat("[[obj a:42] a:42\n"
- "b:42\n"
- "c:42];");
- 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
===
[PATCH] D47527: Revert "[clang-format] Fix putting ObjC message arguments in one line for multiline receiver"
This revision was automatically updated to reflect the committed changes.
Closed by commit rC333539: Revert "[clang-format] Fix putting ObjC message
arguments in one line for… (authored by jolesiak, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D47527?vs=149074&id=149101#toc
Repository:
rC Clang
https://reviews.llvm.org/D47527
Files:
lib/Format/ContinuationIndenter.cpp
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -796,41 +796,6 @@
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 just fit into one line.
- Style.ColumnLimit = 23;
- verifyFormat("[[obj a:42\n"
- " b:42\n"
- " c:42\n"
- " d:42] e:42 f:42];");
-
- // Arguments do not fit into one line with a receiver.
- Style.ColumnLimit = 20;
- verifyFormat("[[obj a:42] a:42\n"
- "b:42];");
- verifyFormat("[[obj a:42] a:42\n"
- "b:42\n"
- "c:42];");
- 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
@@ -1387,29 +1387,6 @@
(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 &CurrentScopeOpener = *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,41 +796,6 @@
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 just fit into one line.
- Style.ColumnLimit = 23;
- verifyFormat("[[obj a:42\n"
- " b:42\n"
- " c:42\n"
- " d:42] e:42 f:42];");
-
- // Arguments do not fit into one line with a receiver.
- Style.ColumnLimit = 20;
- verifyFormat("[[obj a:42] a:42\n"
- "b:42];");
- verifyFormat("[[obj a:42] a:42\n"
- "b:42\n"
- "c:42];");
- verifyFormat("[[obj aa:42\n"
- " b:42]\n"
- "cc:42\n"
- " d:42]
[PATCH] D47393: [clang-format] Disable AlwaysBreakBeforeMultilineStrings in Google style for Objective-C 📜
jolesiak added a comment. Sorry, I missed that. LGTM, I would consider adding ObjC method expression tests (multiline string as an argument). Will look into that next week. Repository: rL LLVM https://reviews.llvm.org/D47393 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48352: [clang-format] Improve ObjC method expressions formatting
jolesiak created this revision.
Herald added a subscriber: cfe-commits.
1. Fix counting parameters/arguments for ObjC.
2. Fix split priorities for ObjC methods.
3. Fix breaking after square bracket starting ObjC method expression.
4. Fix putting ObjC method arguments into one line when they fit.
Repository:
rC Clang
https://reviews.llvm.org/D48352
Files:
lib/Format/ContinuationIndenter.cpp
lib/Format/FormatToken.h
lib/Format/TokenAnnotator.cpp
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -678,6 +678,16 @@
verifyFormat("[(id)foo bar:(id) ? baz : quux];");
verifyFormat("4 > 4 ? (id)a : (id)baz;");
+ unsigned PreviousColumnLimit = Style.ColumnLimit;
+ Style.ColumnLimit = 50;
+ verifyFormat("bool a = ([object a:42] == 0 ||\n"
+ " [object a:42 b:42] == 0);");
+ // Instead of:
+ // bool a =
+ // ([object a:42] == 0 || [object a:42
+ //b:42] == 0);
+ Style.ColumnLimit = PreviousColumnLimit;
+
// This tests that the formatter doesn't break after "backing" but before ":",
// which would be at 80 columns.
verifyFormat(
@@ -803,6 +813,50 @@
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 just fit into one line.
+ Style.ColumnLimit = 23;
+ verifyFormat("[[obj a:42\n"
+ " b:42\n"
+ " c:42\n"
+ " d:42] e:42 f:42];");
+
+ // Arguments do not fit into one line with a receiver.
+ Style.ColumnLimit = 20;
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42];");
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42\n"
+ "c:42];");
+ verifyFormat("[[obj aa:42\n"
+ " b:42]\n"
+ "cc:42\n"
+ " d:42];");
+
+ // Avoid breaking receiver expression.
+ Style.ColumnLimit = 30;
+ verifyFormat("fooo =\n"
+ "[[obj fooo] aaa:42\n"
+ "aaa:42];");
+ verifyFormat("[[[obj foo] bar] aa:42\n"
+ " bb:42\n"
+ " cc:42];");
+
+
Style.ColumnLimit = 70;
verifyFormat(
"void f() {\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -515,11 +515,24 @@
}
Left->MatchingParen = CurrentToken;
CurrentToken->MatchingParen = Left;
+// FirstObjCSelectorName is set when a colon is found. This does
+// not work, however, when method has no parameters.
+// Here, we set FirstObjCSelectorName when the end of the expression is
+// reached, in case it was not set already.
+if (!Contexts.back().FirstObjCSelectorName) {
+FormatToken* Previous = CurrentToken->getPreviousNonComment();
+if (Previous && Previous->is(TT_SelectorName)) {
+ Previous->ObjCSelectorNameParts = 1;
+ Contexts.back().FirstObjCSelectorName = Previous;
+}
+} else {
+ Left->ParameterCount =
+ Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts;
+}
+
if (Contexts.back().FirstObjCSelectorName) {
Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName =
Contexts.back().LongestObjCSelectorName;
- Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts =
- Left->ParameterCount;
if (Left->BlockParameterCount > 1)
Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName = 0;
}
@@ -539,11 +552,6 @@
TT_DesignatedInitializerLSquare)) {
Left->Type = TT_ObjCMethodExpr;
StartsObjCMethodExpr = true;
- // ParameterCount might have been set to 1 before expression was
- // recognized as ObjCMethodExpr (as '1 + number of commas' formula is
- // used for other expression types). Parameter counter has to be,
- // therefore, reset to 0.
- Left->ParameterCount = 0;
Contexts.back().ColonIsObjCMethodExpr = true;
if (Parent
[PATCH] D48352: [clang-format] Improve ObjC method expressions formatting
jolesiak updated this revision to Diff 152043.
jolesiak added a comment.
- Add test
Repository:
rC Clang
https://reviews.llvm.org/D48352
Files:
lib/Format/ContinuationIndenter.cpp
lib/Format/FormatToken.h
lib/Format/TokenAnnotator.cpp
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -678,6 +678,18 @@
verifyFormat("[(id)foo bar:(id) ? baz : quux];");
verifyFormat("4 > 4 ? (id)a : (id)baz;");
+ unsigned PreviousColumnLimit = Style.ColumnLimit;
+ Style.ColumnLimit = 50;
+ // Instead of:
+ // bool a =
+ // ([object a:42] == 0 || [object a:42
+ //b:42] == 0);
+ verifyFormat("bool a = ([object a:42] == 0 ||\n"
+ " [object a:42 b:42] == 0);");
+ Style.ColumnLimit = PreviousColumnLimit;
+ verifyFormat("bool a = ([ a] == a ||\n"
+ " [ a] == );");
+
// This tests that the formatter doesn't break after "backing" but before ":",
// which would be at 80 columns.
verifyFormat(
@@ -754,11 +766,10 @@
"[self a:aaa, aaa, aaa,\n"
"aaa, aaa, aaa,\n"
"aaa, aaa];");
+
verifyFormat("[self // break\n"
" a:a\n"
"aaa:aaa];");
- verifyFormat("bool a = ([ a] == a ||\n"
- " [ a] == );");
// Formats pair-parameters.
verifyFormat("[I drawRectOn:surface ofSize:aa:bbb atOrigin:cc:dd];");
@@ -803,6 +814,53 @@
verifyFormat("[((Foo *)foo) bar];");
verifyFormat("[((Foo *)foo) bar:1 blech:2];");
+ Style.ColumnLimit = 20;
+ verifyFormat("a = [a aa:aa\n"
+ " aa:aa];");
+
+ // Message receiver taking multiple lines.
+ // 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 just fit into one line.
+ Style.ColumnLimit = 23;
+ verifyFormat("[[obj a:42\n"
+ " b:42\n"
+ " c:42\n"
+ " d:42] e:42 f:42];");
+
+ // Arguments do not fit into one line with a receiver.
+ Style.ColumnLimit = 20;
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42];");
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42\n"
+ "c:42];");
+ verifyFormat("[[obj aa:42\n"
+ " b:42]\n"
+ "cc:42\n"
+ " d:42];");
+
+ // Avoid breaking receiver expression.
+ Style.ColumnLimit = 30;
+ verifyFormat("fooo =\n"
+ "[[obj fooo] aaa:42\n"
+ "aaa:42];");
+ verifyFormat("[[[obj foo] bar] aa:42\n"
+ " bb:42\n"
+ " cc:42];");
+
+
Style.ColumnLimit = 70;
verifyFormat(
"void f() {\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -515,11 +515,24 @@
}
Left->MatchingParen = CurrentToken;
CurrentToken->MatchingParen = Left;
+// FirstObjCSelectorName is set when a colon is found. This does
+// not work, however, when method has no parameters.
+// Here, we set FirstObjCSelectorName when the end of the expression is
+// reached, in case it was not set already.
+if (!Contexts.back().FirstObjCSelectorName) {
+FormatToken* Previous = CurrentToken->getPreviousNonComment();
+if (Previous && Previous->is(TT_SelectorName)) {
+ Previous->ObjCSelectorNameParts = 1;
+ Contexts.back().FirstObjCSelectorName = Previous;
+}
+} else {
+ Left->ParameterCount =
+ Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts;
+}
+
if (Contexts.back().FirstObjCSelectorName) {
Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName =
Contexts.back().LongestObjCSelectorName;
- Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts =
- Left->ParameterCount;
if (Left->BlockParameterCount > 1)
Contexts.back().FirstObjCSel
[PATCH] D48432: [clang-format] Add AlwaysBreakBeforeMultilineString tests
jolesiak created this revision.
Herald added a subscriber: cfe-commits.
Repository:
rC Clang
https://reviews.llvm.org/D48432
Files:
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -1227,6 +1227,15 @@
" @\"\");");
verifyFormat("(qqq, @\"\"\n"
" @\"\");");
+ verifyFormat("[ :@\"\"\n"
+ " @\"\"];");
+ verifyFormat(" = [ :@\"\"\n"
+ " @\"\"];");
+ verifyFormat("[ :@\"\"\n"
+ " @\"\"\n"
+ "rr:42\n"
+ "ss:@\"ee\"\n"
+ " @\"f\"];");
}
} // end namespace
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -1227,6 +1227,15 @@
" @\"\");");
verifyFormat("(qqq, @\"\"\n"
" @\"\");");
+ verifyFormat("[ :@\"\"\n"
+ " @\"\"];");
+ verifyFormat(" = [ :@\"\"\n"
+ " @\"\"];");
+ verifyFormat("[ :@\"\"\n"
+ " @\"\"\n"
+ "rr:42\n"
+ "ss:@\"ee\"\n"
+ " @\"f\"];");
}
} // end namespace
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48432: [clang-format] Add AlwaysBreakBeforeMultilineString tests
This revision was automatically updated to reflect the committed changes.
Closed by commit rC335338: [clang-format] Add AlwaysBreakBeforeMultilineString
tests (authored by jolesiak, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D48432?vs=152298&id=152455#toc
Repository:
rC Clang
https://reviews.llvm.org/D48432
Files:
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -1227,6 +1227,15 @@
" @\"\");");
verifyFormat("(qqq, @\"\"\n"
" @\"\");");
+ verifyFormat("[ :@\"\"\n"
+ " @\"\"];");
+ verifyFormat(" = [ :@\"\"\n"
+ " @\"\"];");
+ verifyFormat("[ :@\"\"\n"
+ " @\"\"\n"
+ "rr:42\n"
+ "ss:@\"ee\"\n"
+ " @\"f\"];");
}
} // end namespace
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -1227,6 +1227,15 @@
" @\"\");");
verifyFormat("(qqq, @\"\"\n"
" @\"\");");
+ verifyFormat("[ :@\"\"\n"
+ " @\"\"];");
+ verifyFormat(" = [ :@\"\"\n"
+ " @\"\"];");
+ verifyFormat("[ :@\"\"\n"
+ " @\"\"\n"
+ "rr:42\n"
+ "ss:@\"ee\"\n"
+ " @\"f\"];");
}
} // end namespace
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48352: [clang-format] Improve ObjC method expressions formatting
jolesiak added a comment. In https://reviews.llvm.org/D48352#1145220, @benhamilton wrote: > It's really hard to understand reviews which change 4 different things. > > I hate to ask, but can you split this up into the 4 fixes? Sure, I'll split it tomorrow. Repository: rC Clang https://reviews.llvm.org/D48352 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48716: [clang-format] Fix counting parameters/arguments for ObjC
jolesiak created this revision.
Herald added a subscriber: cfe-commits.
Repository:
rC Clang
https://reviews.llvm.org/D48716
Files:
lib/Format/FormatToken.h
lib/Format/TokenAnnotator.cpp
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -515,11 +515,23 @@
}
Left->MatchingParen = CurrentToken;
CurrentToken->MatchingParen = Left;
+// FirstObjCSelectorName is set when a colon is found. This does
+// not work, however, when method has no parameters.
+// Here, we set FirstObjCSelectorName when the end of the expression is
+// reached, in case it was not set already.
+if (!Contexts.back().FirstObjCSelectorName) {
+FormatToken* Previous = CurrentToken->getPreviousNonComment();
+if (Previous && Previous->is(TT_SelectorName)) {
+ Previous->ObjCSelectorNameParts = 1;
+ Contexts.back().FirstObjCSelectorName = Previous;
+}
+} else {
+ Left->ParameterCount =
+ Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts;
+}
if (Contexts.back().FirstObjCSelectorName) {
Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName =
Contexts.back().LongestObjCSelectorName;
- Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts =
- Left->ParameterCount;
if (Left->BlockParameterCount > 1)
Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName = 0;
}
@@ -539,11 +551,6 @@
TT_DesignatedInitializerLSquare)) {
Left->Type = TT_ObjCMethodExpr;
StartsObjCMethodExpr = true;
- // ParameterCount might have been set to 1 before expression was
- // recognized as ObjCMethodExpr (as '1 + number of commas' formula is
- // used for other expression types). Parameter counter has to be,
- // therefore, reset to 0.
- Left->ParameterCount = 0;
Contexts.back().ColonIsObjCMethodExpr = true;
if (Parent && Parent->is(tok::r_paren))
// FIXME(bug 36976): ObjC return types shouldn't use TT_CastRParen.
@@ -617,12 +624,12 @@
}
void updateParameterCount(FormatToken *Left, FormatToken *Current) {
+// For ObjC methods number of parameters is calculated differently as
+// method declarations have a different structure (parameters are not
inside
+// parenthesis scope).
if (Current->is(tok::l_brace) && Current->BlockKind == BK_Block)
++Left->BlockParameterCount;
-if (Left->Type == TT_ObjCMethodExpr) {
- if (Current->is(tok::colon))
-++Left->ParameterCount;
-} else if (Current->is(tok::comma)) {
+if (Current->is(tok::comma)) {
++Left->ParameterCount;
if (!Left->Role)
Left->Role.reset(new CommaSeparatedList(Style));
@@ -712,6 +719,7 @@
Contexts.back().LongestObjCSelectorName)
Contexts.back().LongestObjCSelectorName =
Tok->Previous->ColumnWidth;
+ ++Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts;
}
} else if (Contexts.back().ColonIsForRangeExpr) {
Tok->Type = TT_RangeBasedForLoopColon;
Index: lib/Format/FormatToken.h
===
--- lib/Format/FormatToken.h
+++ lib/Format/FormatToken.h
@@ -243,8 +243,9 @@
/// e.g. because several of them are block-type.
unsigned LongestObjCSelectorName = 0;
- /// How many parts ObjC selector have (i.e. how many parameters method
- /// has).
+ /// If this is the first ObjC selector name in an ObjC method
+ /// definition or call, this contains the number of parts that whole selector
+ /// consist of.
unsigned ObjCSelectorNameParts = 0;
/// Stores the number of required fake parentheses and the
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -515,11 +515,23 @@
}
Left->MatchingParen = CurrentToken;
CurrentToken->MatchingParen = Left;
+// FirstObjCSelectorName is set when a colon is found. This does
+// not work, however, when method has no parameters.
+// Here, we set FirstObjCSelectorName when the end of the expression is
+// reached, in case it was not set already.
+if (!Contexts.back().FirstObjCSelectorName) {
+FormatToken* Previous = CurrentToken->getPreviousNonComment();
+if (Previous && Previous->is(TT_SelectorName)) {
+ Previous->ObjCSelectorNameParts = 1;
+ Contexts.back().FirstObjCSelectorName = Previous;
+}
+} else {
+ Left->ParameterCount =
+ Contex
[PATCH] D48716: [clang-format] Fix counting parameters/arguments for ObjC
jolesiak updated this revision to Diff 153298.
jolesiak added a comment.
Fix comment
Repository:
rC Clang
https://reviews.llvm.org/D48716
Files:
lib/Format/FormatToken.h
lib/Format/TokenAnnotator.cpp
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -515,11 +515,23 @@
}
Left->MatchingParen = CurrentToken;
CurrentToken->MatchingParen = Left;
+// FirstObjCSelectorName is set when a colon is found. This does
+// not work, however, when method has no parameters.
+// Here, we set FirstObjCSelectorName when the end of the expression is
+// reached, in case it was not set already.
+if (!Contexts.back().FirstObjCSelectorName) {
+FormatToken* Previous = CurrentToken->getPreviousNonComment();
+if (Previous && Previous->is(TT_SelectorName)) {
+ Previous->ObjCSelectorNameParts = 1;
+ Contexts.back().FirstObjCSelectorName = Previous;
+}
+} else {
+ Left->ParameterCount =
+ Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts;
+}
if (Contexts.back().FirstObjCSelectorName) {
Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName =
Contexts.back().LongestObjCSelectorName;
- Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts =
- Left->ParameterCount;
if (Left->BlockParameterCount > 1)
Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName = 0;
}
@@ -539,11 +551,6 @@
TT_DesignatedInitializerLSquare)) {
Left->Type = TT_ObjCMethodExpr;
StartsObjCMethodExpr = true;
- // ParameterCount might have been set to 1 before expression was
- // recognized as ObjCMethodExpr (as '1 + number of commas' formula is
- // used for other expression types). Parameter counter has to be,
- // therefore, reset to 0.
- Left->ParameterCount = 0;
Contexts.back().ColonIsObjCMethodExpr = true;
if (Parent && Parent->is(tok::r_paren))
// FIXME(bug 36976): ObjC return types shouldn't use TT_CastRParen.
@@ -617,12 +624,12 @@
}
void updateParameterCount(FormatToken *Left, FormatToken *Current) {
+// For ObjC methods number of parameters is calculated differently as
+// method declarations have different structure (parameters are not inside
+// parenthesis scope).
if (Current->is(tok::l_brace) && Current->BlockKind == BK_Block)
++Left->BlockParameterCount;
-if (Left->Type == TT_ObjCMethodExpr) {
- if (Current->is(tok::colon))
-++Left->ParameterCount;
-} else if (Current->is(tok::comma)) {
+if (Current->is(tok::comma)) {
++Left->ParameterCount;
if (!Left->Role)
Left->Role.reset(new CommaSeparatedList(Style));
@@ -712,6 +719,7 @@
Contexts.back().LongestObjCSelectorName)
Contexts.back().LongestObjCSelectorName =
Tok->Previous->ColumnWidth;
+ ++Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts;
}
} else if (Contexts.back().ColonIsForRangeExpr) {
Tok->Type = TT_RangeBasedForLoopColon;
Index: lib/Format/FormatToken.h
===
--- lib/Format/FormatToken.h
+++ lib/Format/FormatToken.h
@@ -243,8 +243,9 @@
/// e.g. because several of them are block-type.
unsigned LongestObjCSelectorName = 0;
- /// How many parts ObjC selector have (i.e. how many parameters method
- /// has).
+ /// If this is the first ObjC selector name in an ObjC method
+ /// definition or call, this contains the number of parts that whole selector
+ /// consist of.
unsigned ObjCSelectorNameParts = 0;
/// Stores the number of required fake parentheses and the
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -515,11 +515,23 @@
}
Left->MatchingParen = CurrentToken;
CurrentToken->MatchingParen = Left;
+// FirstObjCSelectorName is set when a colon is found. This does
+// not work, however, when method has no parameters.
+// Here, we set FirstObjCSelectorName when the end of the expression is
+// reached, in case it was not set already.
+if (!Contexts.back().FirstObjCSelectorName) {
+FormatToken* Previous = CurrentToken->getPreviousNonComment();
+if (Previous && Previous->is(TT_SelectorName)) {
+ Previous->ObjCSelectorNameParts = 1;
+ Contexts.back().FirstObjCSelectorName = Previous;
+}
+} else {
+ Left->ParameterCount =
+
[PATCH] D48718: [clang-format] Prohibit breaking after a bracket opening ObjC method expression
jolesiak created this revision. Herald added a subscriber: cfe-commits. Repository: rC Clang https://reviews.llvm.org/D48718 Files: lib/Format/ContinuationIndenter.cpp Index: lib/Format/ContinuationIndenter.cpp === --- lib/Format/ContinuationIndenter.cpp +++ lib/Format/ContinuationIndenter.cpp @@ -321,6 +321,9 @@ State.Stack.back().NoLineBreakInOperand) return false; + if (Previous.is(tok::l_square) && Previous.is(TT_ObjCMethodExpr)) +return false; + return !State.Stack.back().NoLineBreak; } Index: lib/Format/ContinuationIndenter.cpp === --- lib/Format/ContinuationIndenter.cpp +++ lib/Format/ContinuationIndenter.cpp @@ -321,6 +321,9 @@ State.Stack.back().NoLineBreakInOperand) return false; + if (Previous.is(tok::l_square) && Previous.is(TT_ObjCMethodExpr)) +return false; + return !State.Stack.back().NoLineBreak; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48719: [clang-format] Fix split priorities for ObjC methods
jolesiak created this revision.
Herald added a subscriber: cfe-commits.
Repository:
rC Clang
https://reviews.llvm.org/D48719
Files:
lib/Format/FormatToken.h
lib/Format/TokenAnnotator.cpp
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -678,6 +678,18 @@
verifyFormat("[(id)foo bar:(id) ? baz : quux];");
verifyFormat("4 > 4 ? (id)a : (id)baz;");
+ unsigned PreviousColumnLimit = Style.ColumnLimit;
+ Style.ColumnLimit = 50;
+ // Instead of:
+ // bool a =
+ // ([object a:42] == 0 || [object a:42
+ //b:42] == 0);
+ verifyFormat("bool a = ([object a:42] == 0 ||\n"
+ " [object a:42 b:42] == 0);");
+ Style.ColumnLimit = PreviousColumnLimit;
+ verifyFormat("bool a = ([ a] == a ||\n"
+ " [ a] == );");
+
// This tests that the formatter doesn't break after "backing" but before ":",
// which would be at 80 columns.
verifyFormat(
@@ -754,11 +766,10 @@
"[self a:aaa, aaa, aaa,\n"
"aaa, aaa, aaa,\n"
"aaa, aaa];");
+
verifyFormat("[self // break\n"
" a:a\n"
"aaa:aaa];");
- verifyFormat("bool a = ([ a] == a ||\n"
- " [ a] == );");
// Formats pair-parameters.
verifyFormat("[I drawRectOn:surface ofSize:aa:bbb atOrigin:cc:dd];");
@@ -803,6 +814,12 @@
verifyFormat("[((Foo *)foo) bar];");
verifyFormat("[((Foo *)foo) bar:1 blech:2];");
+ Style.ColumnLimit = 20;
+ verifyFormat("a = [a aa:aa\n"
+ " aa:aa];");
+ verifyFormat("aa = [aa aa:aa\n"
+ " aa:aa];");
+
Style.ColumnLimit = 70;
verifyFormat(
"void f() {\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -719,6 +719,8 @@
Contexts.back().LongestObjCSelectorName)
Contexts.back().LongestObjCSelectorName =
Tok->Previous->ColumnWidth;
+ Tok->Previous->ParameterIndex =
+ Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts;
++Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts;
}
} else if (Contexts.back().ColonIsForRangeExpr) {
@@ -2136,8 +2138,20 @@
// FIXME: Only calculate this if CanBreakBefore is true once static
// initializers etc. are sorted out.
// FIXME: Move magic numbers to a better place.
-Current->SplitPenalty = 20 * Current->BindingStrength +
-splitPenalty(Line, *Current, InFunctionDecl);
+
+// Reduce penalty for aligning ObjC method arguments using the colon
+// alignment as this is the canonical way (still prefer fitting everything
+// into one line if possible). Trying to fit a whole epression into one
+// line should not force other line breaks (e.g. when ObjC method
+// expression is a part of other expression).
+Current->SplitPenalty = splitPenalty(Line, *Current, InFunctionDecl);
+if (Style.Language == FormatStyle::LK_ObjC &&
+Current->is(TT_SelectorName) && Current->ParameterIndex > 0) {
+ if (Current->ParameterIndex == 1)
+Current->SplitPenalty += 5 * Current->BindingStrength;
+} else {
+ Current->SplitPenalty += 20 * Current->BindingStrength;
+}
Current = Current->Next;
}
Index: lib/Format/FormatToken.h
===
--- lib/Format/FormatToken.h
+++ lib/Format/FormatToken.h
@@ -248,6 +248,11 @@
/// consist of.
unsigned ObjCSelectorNameParts = 0;
+ /// The 0-based index of the parameter/argument. For ObjC it is set
+ /// for the selector name token.
+ /// For now calculated only for ObjC.
+ unsigned ParameterIndex = 0;
+
/// Stores the number of required fake parentheses and the
/// corresponding operator precedence.
///
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48720: [clang-format] Put ObjC method arguments into one line when they fit
jolesiak created this revision.
Herald added a subscriber: cfe-commits.
Repository:
rC Clang
https://reviews.llvm.org/D48720
Files:
lib/Format/ContinuationIndenter.cpp
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -820,6 +820,48 @@
verifyFormat("aa = [aa aa:aa\n"
" aa:aa];");
+ // Message receiver taking multiple lines.
+ // 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 just fit into one line.
+ Style.ColumnLimit = 23;
+ verifyFormat("[[obj a:42\n"
+ " b:42\n"
+ " c:42\n"
+ " d:42] e:42 f:42];");
+
+ // Arguments do not fit into one line with a receiver.
+ Style.ColumnLimit = 20;
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42];");
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42\n"
+ "c:42];");
+ verifyFormat("[[obj aa:42\n"
+ " b:42]\n"
+ "cc:42\n"
+ " d:42];");
+
+ // Avoid breaking receiver expression.
+ Style.ColumnLimit = 30;
+ verifyFormat("fooo =\n"
+ "[[obj fooo] aaa:42\n"
+ "aaa:42];");
+ verifyFormat("[[[obj foo] bar] aa:42\n"
+ " bb:42\n"
+ " cc:42];");
+
Style.ColumnLimit = 70;
verifyFormat(
"void f() {\n"
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -1398,6 +1398,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 &CurrentScopeOpener = *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
@@ -820,6 +820,48 @@
verifyFormat("aa = [aa aa:aa\n"
" aa:aa];");
+ // Message receiver taking multiple lines.
+ // 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 just fit into one line.
+ Style.ColumnLimit = 23;
+ verifyFormat("[[obj a:42\n"
+ " b:42\n"
+ " c:42\n"
+ " d:42] e:42 f:42];");
+
+ // Arguments do not fit into one line with a receiver.
+ Style.ColumnLimit = 20;
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42];");
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42\n"
+ "c:42];");
+ verifyFormat("[[obj aa:42\n"
+ " b:42]\n"
+ "cc:42\n"
+
[PATCH] D48719: [clang-format] Fix split priorities for ObjC methods
jolesiak updated this revision to Diff 153312.
jolesiak added a comment.
Fix comment.
Repository:
rC Clang
https://reviews.llvm.org/D48719
Files:
lib/Format/ContinuationIndenter.cpp
lib/Format/FormatToken.h
lib/Format/TokenAnnotator.cpp
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -678,6 +678,18 @@
verifyFormat("[(id)foo bar:(id) ? baz : quux];");
verifyFormat("4 > 4 ? (id)a : (id)baz;");
+ unsigned PreviousColumnLimit = Style.ColumnLimit;
+ Style.ColumnLimit = 50;
+ // Instead of:
+ // bool a =
+ // ([object a:42] == 0 || [object a:42
+ //b:42] == 0);
+ verifyFormat("bool a = ([object a:42] == 0 ||\n"
+ " [object a:42 b:42] == 0);");
+ Style.ColumnLimit = PreviousColumnLimit;
+ verifyFormat("bool a = ([ a] == a ||\n"
+ " [ a] == );");
+
// This tests that the formatter doesn't break after "backing" but before ":",
// which would be at 80 columns.
verifyFormat(
@@ -754,11 +766,10 @@
"[self a:aaa, aaa, aaa,\n"
"aaa, aaa, aaa,\n"
"aaa, aaa];");
+
verifyFormat("[self // break\n"
" a:a\n"
"aaa:aaa];");
- verifyFormat("bool a = ([ a] == a ||\n"
- " [ a] == );");
// Formats pair-parameters.
verifyFormat("[I drawRectOn:surface ofSize:aa:bbb atOrigin:cc:dd];");
@@ -803,6 +814,12 @@
verifyFormat("[((Foo *)foo) bar];");
verifyFormat("[((Foo *)foo) bar:1 blech:2];");
+ Style.ColumnLimit = 20;
+ verifyFormat("a = [a aa:aa\n"
+ " aa:aa];");
+ verifyFormat("aa = [aa aa:aa\n"
+ " aa:aa];");
+
Style.ColumnLimit = 70;
verifyFormat(
"void f() {\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -515,11 +515,23 @@
}
Left->MatchingParen = CurrentToken;
CurrentToken->MatchingParen = Left;
+// FirstObjCSelectorName is set when a colon is found. This does
+// not work, however, when method has no parameters.
+// Here, we set FirstObjCSelectorName when the end of the expression is
+// reached, in case it was not set already.
+if (!Contexts.back().FirstObjCSelectorName) {
+FormatToken* Previous = CurrentToken->getPreviousNonComment();
+if (Previous && Previous->is(TT_SelectorName)) {
+ Previous->ObjCSelectorNameParts = 1;
+ Contexts.back().FirstObjCSelectorName = Previous;
+}
+} else {
+ Left->ParameterCount =
+ Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts;
+}
if (Contexts.back().FirstObjCSelectorName) {
Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName =
Contexts.back().LongestObjCSelectorName;
- Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts =
- Left->ParameterCount;
if (Left->BlockParameterCount > 1)
Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName = 0;
}
@@ -539,11 +551,6 @@
TT_DesignatedInitializerLSquare)) {
Left->Type = TT_ObjCMethodExpr;
StartsObjCMethodExpr = true;
- // ParameterCount might have been set to 1 before expression was
- // recognized as ObjCMethodExpr (as '1 + number of commas' formula is
- // used for other expression types). Parameter counter has to be,
- // therefore, reset to 0.
- Left->ParameterCount = 0;
Contexts.back().ColonIsObjCMethodExpr = true;
if (Parent && Parent->is(tok::r_paren))
// FIXME(bug 36976): ObjC return types shouldn't use TT_CastRParen.
@@ -617,12 +624,12 @@
}
void updateParameterCount(FormatToken *Left, FormatToken *Current) {
+// For ObjC methods number of parameters is calculated differently as
+// method declarations have different structure (parameters are not inside
+// parenthesis scope).
if (Current->is(tok::l_brace) && Current->BlockKind == BK_Block)
++Left->BlockParameterCount;
-if (Left->Type == TT_ObjCMethodExpr) {
- if (Current->is(tok::colon))
-++Left->ParameterCount;
-} else if (Current->is(tok::comma)) {
+if (Current->is(tok::comma)) {
++Left->ParameterCount;
if (!Left->Role)
Le
[PATCH] D48719: [clang-format] Fix split priorities for ObjC methods
jolesiak updated this revision to Diff 153313.
jolesiak added a comment.
Fix base change.
Repository:
rC Clang
https://reviews.llvm.org/D48719
Files:
lib/Format/FormatToken.h
lib/Format/TokenAnnotator.cpp
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -678,6 +678,18 @@
verifyFormat("[(id)foo bar:(id) ? baz : quux];");
verifyFormat("4 > 4 ? (id)a : (id)baz;");
+ unsigned PreviousColumnLimit = Style.ColumnLimit;
+ Style.ColumnLimit = 50;
+ // Instead of:
+ // bool a =
+ // ([object a:42] == 0 || [object a:42
+ //b:42] == 0);
+ verifyFormat("bool a = ([object a:42] == 0 ||\n"
+ " [object a:42 b:42] == 0);");
+ Style.ColumnLimit = PreviousColumnLimit;
+ verifyFormat("bool a = ([ a] == a ||\n"
+ " [ a] == );");
+
// This tests that the formatter doesn't break after "backing" but before ":",
// which would be at 80 columns.
verifyFormat(
@@ -754,11 +766,10 @@
"[self a:aaa, aaa, aaa,\n"
"aaa, aaa, aaa,\n"
"aaa, aaa];");
+
verifyFormat("[self // break\n"
" a:a\n"
"aaa:aaa];");
- verifyFormat("bool a = ([ a] == a ||\n"
- " [ a] == );");
// Formats pair-parameters.
verifyFormat("[I drawRectOn:surface ofSize:aa:bbb atOrigin:cc:dd];");
@@ -803,6 +814,12 @@
verifyFormat("[((Foo *)foo) bar];");
verifyFormat("[((Foo *)foo) bar:1 blech:2];");
+ Style.ColumnLimit = 20;
+ verifyFormat("a = [a aa:aa\n"
+ " aa:aa];");
+ verifyFormat("aa = [aa aa:aa\n"
+ " aa:aa];");
+
Style.ColumnLimit = 70;
verifyFormat(
"void f() {\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -719,6 +719,8 @@
Contexts.back().LongestObjCSelectorName)
Contexts.back().LongestObjCSelectorName =
Tok->Previous->ColumnWidth;
+ Tok->Previous->ParameterIndex =
+ Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts;
++Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts;
}
} else if (Contexts.back().ColonIsForRangeExpr) {
@@ -2136,8 +2138,20 @@
// FIXME: Only calculate this if CanBreakBefore is true once static
// initializers etc. are sorted out.
// FIXME: Move magic numbers to a better place.
-Current->SplitPenalty = 20 * Current->BindingStrength +
-splitPenalty(Line, *Current, InFunctionDecl);
+
+// Reduce penalty for aligning ObjC method arguments using the colon
+// alignment as this is the canonical way (still prefer fitting everything
+// into one line if possible). Trying to fit a whole expression into one
+// line should not force other line breaks (e.g. when ObjC method
+// expression is a part of other expression).
+Current->SplitPenalty = splitPenalty(Line, *Current, InFunctionDecl);
+if (Style.Language == FormatStyle::LK_ObjC &&
+Current->is(TT_SelectorName) && Current->ParameterIndex > 0) {
+ if (Current->ParameterIndex == 1)
+Current->SplitPenalty += 5 * Current->BindingStrength;
+} else {
+ Current->SplitPenalty += 20 * Current->BindingStrength;
+}
Current = Current->Next;
}
Index: lib/Format/FormatToken.h
===
--- lib/Format/FormatToken.h
+++ lib/Format/FormatToken.h
@@ -248,6 +248,11 @@
/// consist of.
unsigned ObjCSelectorNameParts = 0;
+ /// The 0-based index of the parameter/argument. For ObjC it is set
+ /// for the selector name token.
+ /// For now calculated only for ObjC.
+ unsigned ParameterIndex = 0;
+
/// Stores the number of required fake parentheses and the
/// corresponding operator precedence.
///
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48352: [clang-format] Improve ObjC method expressions formatting
jolesiak abandoned this revision. jolesiak added a comment. Split: https://reviews.llvm.org/D48716 https://reviews.llvm.org/D48718 https://reviews.llvm.org/D48719 https://reviews.llvm.org/D48720 Repository: rC Clang https://reviews.llvm.org/D48352 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48716: [clang-format] Fix counting parameters/arguments for ObjC
jolesiak marked 6 inline comments as done.
jolesiak added inline comments.
Comment at: lib/Format/FormatToken.h:247-248
+ /// If this is the first ObjC selector name in an ObjC method
+ /// definition or call, this contains the number of parts that whole selector
+ /// consist of.
unsigned ObjCSelectorNameParts = 0;
benhamilton wrote:
> Grammar nit: that whole selector consist of -> that the whole selector
> consists of
>
Thanks!
Comment at: lib/Format/TokenAnnotator.cpp:519
+// FirstObjCSelectorName is set when a colon is found. This does
+// not work, however, when method has no parameters.
+// Here, we set FirstObjCSelectorName when the end of the expression is
benhamilton wrote:
> Grammar nit-pick: when method -> when the method
>
Thanks!
Comment at: lib/Format/TokenAnnotator.cpp:520
+// not work, however, when method has no parameters.
+// Here, we set FirstObjCSelectorName when the end of the expression is
+// reached, in case it was not set already.
benhamilton wrote:
> expression -> method call
>
Thanks!
Comment at: lib/Format/TokenAnnotator.cpp:627
void updateParameterCount(FormatToken *Left, FormatToken *Current) {
+// For ObjC methods number of parameters is calculated differently as
+// method declarations have different structure (parameters are not inside
benhamilton wrote:
> Grammar nit-pick: methods number -> methods, the number
>
Thanks!
Comment at: lib/Format/TokenAnnotator.cpp:628
+// For ObjC methods number of parameters is calculated differently as
+// method declarations have different structure (parameters are not inside
+// parenthesis scope).
benhamilton wrote:
> benhamilton wrote:
> > Grammar nit-picks:
> >
> > * have different structure -> have a different structure
> > * parameters are not -> the parameters are not
> >
> Why does parenthesis scope matter here? `updateParameterCount()` is called
> from `parseSquare()`.
>
> I'm not sure what the goal of this change is.
>
Thanks!
Comment at: lib/Format/TokenAnnotator.cpp:628-629
+// For ObjC methods number of parameters is calculated differently as
+// method declarations have different structure (parameters are not inside
+// parenthesis scope).
if (Current->is(tok::l_brace) && Current->BlockKind == BK_Block)
jolesiak wrote:
> benhamilton wrote:
> > benhamilton wrote:
> > > Grammar nit-picks:
> > >
> > > * have different structure -> have a different structure
> > > * parameters are not -> the parameters are not
> > >
> > Why does parenthesis scope matter here? `updateParameterCount()` is called
> > from `parseSquare()`.
> >
> > I'm not sure what the goal of this change is.
> >
> Thanks!
My bad, I thought the word "parenthesis" can also mean any of "(", "[", "{",
"<" (not only specifically the first one).
I've replaced with "a bracket scope".
I'll comment on the goal of this change in non-inline comment.
Repository:
rC Clang
https://reviews.llvm.org/D48716
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48716: [clang-format] Fix counting parameters/arguments for ObjC
jolesiak marked 6 inline comments as done. jolesiak added a comment. In https://reviews.llvm.org/D48716#1146796, @benhamilton wrote: > > Count selector parts also for method declarations. > > What bug does this fix? Can you add a test which breaks before this change > and is fixed by this change? Sorry for the confusion. General comment to changes https://reviews.llvm.org/D48716, https://reviews.llvm.org/D48718, https://reviews.llvm.org/D48719, https://reviews.llvm.org/D48720 (which are the split of https://reviews.llvm.org/D48352): These changes are separate, in the sense that they fix different issues. However, they should be chained as every change (apart from the first one) is based on previous ones: https://reviews.llvm.org/D48716 -> https://reviews.llvm.org/D48718 -> https://reviews.llvm.org/D48719 -> https://reviews.llvm.org/D48720. I don't know how to chain them in Phabricator (I should probably leave some comments about what every change is based on though). https://reviews.llvm.org/D48716 fixes the mechanism of counting parameters. This is an internal change though and doesn't influence formatting on its own (at the current state). Its lack would be visible after applying https://reviews.llvm.org/D48719. Therefore, I'm not aware of any formatting test that fails before applying this change and succeeds after. As far as I know internal functions/methods are not tested at all. Thus, the tests are part of https://reviews.llvm.org/D48719. This is why initially I put everything into one change: https://reviews.llvm.org/D48352, but I agree that it was not readable at all. Repository: rC Clang https://reviews.llvm.org/D48716 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48716: [clang-format] Fix counting parameters/arguments for ObjC
jolesiak updated this revision to Diff 153689.
jolesiak added a comment.
Fix grammar mistakes.
Repository:
rC Clang
https://reviews.llvm.org/D48716
Files:
lib/Format/FormatToken.h
lib/Format/TokenAnnotator.cpp
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -515,11 +515,23 @@
}
Left->MatchingParen = CurrentToken;
CurrentToken->MatchingParen = Left;
+// FirstObjCSelectorName is set when a colon is found. This does
+// not work, however, when the method has no parameters.
+// Here, we set FirstObjCSelectorName when the end of the method call
is
+// reached, in case it was not set already.
+if (!Contexts.back().FirstObjCSelectorName) {
+FormatToken* Previous = CurrentToken->getPreviousNonComment();
+if (Previous && Previous->is(TT_SelectorName)) {
+ Previous->ObjCSelectorNameParts = 1;
+ Contexts.back().FirstObjCSelectorName = Previous;
+}
+} else {
+ Left->ParameterCount =
+ Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts;
+}
if (Contexts.back().FirstObjCSelectorName) {
Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName =
Contexts.back().LongestObjCSelectorName;
- Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts =
- Left->ParameterCount;
if (Left->BlockParameterCount > 1)
Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName = 0;
}
@@ -539,11 +551,6 @@
TT_DesignatedInitializerLSquare)) {
Left->Type = TT_ObjCMethodExpr;
StartsObjCMethodExpr = true;
- // ParameterCount might have been set to 1 before expression was
- // recognized as ObjCMethodExpr (as '1 + number of commas' formula is
- // used for other expression types). Parameter counter has to be,
- // therefore, reset to 0.
- Left->ParameterCount = 0;
Contexts.back().ColonIsObjCMethodExpr = true;
if (Parent && Parent->is(tok::r_paren))
// FIXME(bug 36976): ObjC return types shouldn't use TT_CastRParen.
@@ -617,12 +624,12 @@
}
void updateParameterCount(FormatToken *Left, FormatToken *Current) {
+// For ObjC methods, the number of parameters is calculated differently as
+// method declarations have a different structure (the parameters are not
+// inside a bracket scope).
if (Current->is(tok::l_brace) && Current->BlockKind == BK_Block)
++Left->BlockParameterCount;
-if (Left->Type == TT_ObjCMethodExpr) {
- if (Current->is(tok::colon))
-++Left->ParameterCount;
-} else if (Current->is(tok::comma)) {
+if (Current->is(tok::comma)) {
++Left->ParameterCount;
if (!Left->Role)
Left->Role.reset(new CommaSeparatedList(Style));
@@ -712,6 +719,7 @@
Contexts.back().LongestObjCSelectorName)
Contexts.back().LongestObjCSelectorName =
Tok->Previous->ColumnWidth;
+ ++Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts;
}
} else if (Contexts.back().ColonIsForRangeExpr) {
Tok->Type = TT_RangeBasedForLoopColon;
Index: lib/Format/FormatToken.h
===
--- lib/Format/FormatToken.h
+++ lib/Format/FormatToken.h
@@ -243,8 +243,9 @@
/// e.g. because several of them are block-type.
unsigned LongestObjCSelectorName = 0;
- /// How many parts ObjC selector have (i.e. how many parameters method
- /// has).
+ /// If this is the first ObjC selector name in an ObjC method
+ /// definition or call, this contains the number of parts that the whole
+ /// selector consist of.
unsigned ObjCSelectorNameParts = 0;
/// Stores the number of required fake parentheses and the
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -515,11 +515,23 @@
}
Left->MatchingParen = CurrentToken;
CurrentToken->MatchingParen = Left;
+// FirstObjCSelectorName is set when a colon is found. This does
+// not work, however, when the method has no parameters.
+// Here, we set FirstObjCSelectorName when the end of the method call is
+// reached, in case it was not set already.
+if (!Contexts.back().FirstObjCSelectorName) {
+FormatToken* Previous = CurrentToken->getPreviousNonComment();
+if (Previous && Previous->is(TT_SelectorName)) {
+ Previous->ObjCSelectorNameParts = 1;
+ Contexts.back().FirstObjCSelectorName = Previous;
+}
+} else {
+
[PATCH] D48718: [clang-format] Prohibit breaking after a bracket opening ObjC method expression
jolesiak added a comment. In https://reviews.llvm.org/D48718#1146805, @benhamilton wrote: > Can you add a test, please? The test could look like: a = [a aa:aa aa:aa]; with appropriate column limit. Right now, however, this would be formatted: a = [a aa:aa aa:aa]; This behavior is improved in https://reviews.llvm.org/D48719. To avoid adding a = [a aa:aa aa:aa]; test in this change and changing it to: a = [a aa:aa aa:aa]; in https://reviews.llvm.org/D48719 I decided to omit it altogether in this change. Sorry for the confusion. If you prefer me to add this test and modify in later commit I'll do it. Repository: rC Clang https://reviews.llvm.org/D48718 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48720: [clang-format] Put ObjC method arguments into one line when they fit
jolesiak updated this revision to Diff 153705.
jolesiak added a comment.
Improve condition.
Repository:
rC Clang
https://reviews.llvm.org/D48720
Files:
lib/Format/ContinuationIndenter.cpp
lib/Format/FormatToken.h
lib/Format/TokenAnnotator.cpp
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -678,6 +678,18 @@
verifyFormat("[(id)foo bar:(id) ? baz : quux];");
verifyFormat("4 > 4 ? (id)a : (id)baz;");
+ unsigned PreviousColumnLimit = Style.ColumnLimit;
+ Style.ColumnLimit = 50;
+ // Instead of:
+ // bool a =
+ // ([object a:42] == 0 || [object a:42
+ //b:42] == 0);
+ verifyFormat("bool a = ([object a:42] == 0 ||\n"
+ " [object a:42 b:42] == 0);");
+ Style.ColumnLimit = PreviousColumnLimit;
+ verifyFormat("bool a = ([ a] == a ||\n"
+ " [ a] == );");
+
// This tests that the formatter doesn't break after "backing" but before ":",
// which would be at 80 columns.
verifyFormat(
@@ -754,11 +766,10 @@
"[self a:aaa, aaa, aaa,\n"
"aaa, aaa, aaa,\n"
"aaa, aaa];");
+
verifyFormat("[self // break\n"
" a:a\n"
"aaa:aaa];");
- verifyFormat("bool a = ([ a] == a ||\n"
- " [ a] == );");
// Formats pair-parameters.
verifyFormat("[I drawRectOn:surface ofSize:aa:bbb atOrigin:cc:dd];");
@@ -803,6 +814,54 @@
verifyFormat("[((Foo *)foo) bar];");
verifyFormat("[((Foo *)foo) bar:1 blech:2];");
+ Style.ColumnLimit = 20;
+ verifyFormat("a = [a aa:aa\n"
+ " aa:aa];");
+ verifyFormat("aa = [aa aa:aa\n"
+ " aa:aa];");
+
+ // Message receiver taking multiple lines.
+ // 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 just fit into one line.
+ Style.ColumnLimit = 23;
+ verifyFormat("[[obj a:42\n"
+ " b:42\n"
+ " c:42\n"
+ " d:42] e:42 f:42];");
+
+ // Arguments do not fit into one line with a receiver.
+ Style.ColumnLimit = 20;
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42];");
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42\n"
+ "c:42];");
+ verifyFormat("[[obj aa:42\n"
+ " b:42]\n"
+ "cc:42\n"
+ " d:42];");
+
+ // Avoid breaking receiver expression.
+ Style.ColumnLimit = 30;
+ verifyFormat("fooo =\n"
+ "[[obj fooo] aaa:42\n"
+ "aaa:42];");
+ verifyFormat("[[[obj foo] bar] aa:42\n"
+ " bb:42\n"
+ " cc:42];");
+
Style.ColumnLimit = 70;
verifyFormat(
"void f() {\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -515,11 +515,23 @@
}
Left->MatchingParen = CurrentToken;
CurrentToken->MatchingParen = Left;
+// FirstObjCSelectorName is set when a colon is found. This does
+// not work, however, when the method has no parameters.
+// Here, we set FirstObjCSelectorName when the end of the method call is
+// reached, in case it was not set already.
+if (!Contexts.back().FirstObjCSelectorName) {
+FormatToken* Previous = CurrentToken->getPreviousNonComment();
+if (Previous && Previous->is(TT_SelectorName)) {
+ Previous->ObjCSelectorNameParts = 1;
+ Contexts.back().FirstObjCSelectorName = Previous;
+}
+} else {
+ Left->ParameterCount =
+ Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts;
+}
if (Contexts.back().FirstObjCSelectorName) {
Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName =
Contexts.back().LongestObjCSelectorName;
- Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts =
- Left->ParameterCount
[PATCH] D48720: [clang-format] Put ObjC method arguments into one line when they fit
jolesiak updated this revision to Diff 153706.
jolesiak added a comment.
Rebase.
Repository:
rC Clang
https://reviews.llvm.org/D48720
Files:
lib/Format/ContinuationIndenter.cpp
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -820,6 +820,48 @@
verifyFormat("aa = [aa aa:aa\n"
" aa:aa];");
+ // Message receiver taking multiple lines.
+ // 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 just fit into one line.
+ Style.ColumnLimit = 23;
+ verifyFormat("[[obj a:42\n"
+ " b:42\n"
+ " c:42\n"
+ " d:42] e:42 f:42];");
+
+ // Arguments do not fit into one line with a receiver.
+ Style.ColumnLimit = 20;
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42];");
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42\n"
+ "c:42];");
+ verifyFormat("[[obj aa:42\n"
+ " b:42]\n"
+ "cc:42\n"
+ " d:42];");
+
+ // Avoid breaking receiver expression.
+ Style.ColumnLimit = 30;
+ verifyFormat("fooo =\n"
+ "[[obj fooo] aaa:42\n"
+ "aaa:42];");
+ verifyFormat("[[[obj foo] bar] aa:42\n"
+ " bb:42\n"
+ " cc:42];");
+
Style.ColumnLimit = 70;
verifyFormat(
"void f() {\n"
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -1398,6 +1398,30 @@
(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 (State.Stack.back().BreakBeforeParameter && Current.MatchingParen &&
+ Current.MatchingParen->Previous) {
+const FormatToken &CurrentScopeOpener = *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
@@ -820,6 +820,48 @@
verifyFormat("aa = [aa aa:aa\n"
" aa:aa];");
+ // Message receiver taking multiple lines.
+ // 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 just fit into one line.
+ Style.ColumnLimit = 23;
+ verifyFormat("[[obj a:42\n"
+ " b:42\n"
+ " c:42\n"
+ " d:42] e:42 f:42];");
+
+ // Arguments do not fit into one line with a receiver.
+ Style.ColumnLimit = 20;
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42];");
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42\n"
+ "c:42];");
+ verifyFormat("[[obj aa:42\n"
+
[PATCH] D48720: [clang-format] Put ObjC method arguments into one line when they fit
jolesiak marked an inline comment as done.
jolesiak added inline comments.
Comment at: lib/Format/ContinuationIndenter.cpp:1411
+ // line).
+ if (Current.MatchingParen && Current.MatchingParen->Previous) {
+const FormatToken &CurrentScopeOpener = *Current.MatchingParen->Previous;
benhamilton wrote:
> Should we check if `State.Stack.back().BreakBeforeParameter` is `true` before
> doing any of this?
I think that performance-wise it wouldn't be worth adding.
However, as this section is about "reevaluation" I think it's better to add
this check, as what we mean is really "If we decided earlier that breaks are
necessary, check once again".
Repository:
rC Clang
https://reviews.llvm.org/D48720
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48716: [clang-format/ObjC] Fix counting selector name parts for ObjC
jolesiak added a comment. In https://reviews.llvm.org/D48716#1149733, @benhamilton wrote: > Phabricator has an `Edit Related Revisions` feature where you can tag other > revisions as being dependencies of or depending upon the current revision: Thanks! That's what I was looking for. Repository: rC Clang https://reviews.llvm.org/D48716 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48716: [clang-format/ObjC] Fix counting selector name parts for ObjC
This revision was automatically updated to reflect the committed changes.
Closed by commit rL336518: [clang-format/ObjC] Fix counting selector name parts
for ObjC (authored by jolesiak, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D48716
Files:
cfe/trunk/lib/Format/FormatToken.h
cfe/trunk/lib/Format/TokenAnnotator.cpp
Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -515,11 +515,23 @@
}
Left->MatchingParen = CurrentToken;
CurrentToken->MatchingParen = Left;
+// FirstObjCSelectorName is set when a colon is found. This does
+// not work, however, when the method has no parameters.
+// Here, we set FirstObjCSelectorName when the end of the method call
is
+// reached, in case it was not set already.
+if (!Contexts.back().FirstObjCSelectorName) {
+FormatToken* Previous = CurrentToken->getPreviousNonComment();
+if (Previous && Previous->is(TT_SelectorName)) {
+ Previous->ObjCSelectorNameParts = 1;
+ Contexts.back().FirstObjCSelectorName = Previous;
+}
+} else {
+ Left->ParameterCount =
+ Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts;
+}
if (Contexts.back().FirstObjCSelectorName) {
Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName =
Contexts.back().LongestObjCSelectorName;
- Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts =
- Left->ParameterCount;
if (Left->BlockParameterCount > 1)
Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName = 0;
}
@@ -539,11 +551,6 @@
TT_DesignatedInitializerLSquare)) {
Left->Type = TT_ObjCMethodExpr;
StartsObjCMethodExpr = true;
- // ParameterCount might have been set to 1 before expression was
- // recognized as ObjCMethodExpr (as '1 + number of commas' formula is
- // used for other expression types). Parameter counter has to be,
- // therefore, reset to 0.
- Left->ParameterCount = 0;
Contexts.back().ColonIsObjCMethodExpr = true;
if (Parent && Parent->is(tok::r_paren))
// FIXME(bug 36976): ObjC return types shouldn't use TT_CastRParen.
@@ -617,12 +624,12 @@
}
void updateParameterCount(FormatToken *Left, FormatToken *Current) {
+// For ObjC methods, the number of parameters is calculated differently as
+// method declarations have a different structure (the parameters are not
+// inside a bracket scope).
if (Current->is(tok::l_brace) && Current->BlockKind == BK_Block)
++Left->BlockParameterCount;
-if (Left->Type == TT_ObjCMethodExpr) {
- if (Current->is(tok::colon))
-++Left->ParameterCount;
-} else if (Current->is(tok::comma)) {
+if (Current->is(tok::comma)) {
++Left->ParameterCount;
if (!Left->Role)
Left->Role.reset(new CommaSeparatedList(Style));
@@ -718,6 +725,7 @@
Contexts.back().LongestObjCSelectorName)
Contexts.back().LongestObjCSelectorName =
Tok->Previous->ColumnWidth;
+ ++Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts;
}
} else if (Contexts.back().ColonIsForRangeExpr) {
Tok->Type = TT_RangeBasedForLoopColon;
Index: cfe/trunk/lib/Format/FormatToken.h
===
--- cfe/trunk/lib/Format/FormatToken.h
+++ cfe/trunk/lib/Format/FormatToken.h
@@ -243,8 +243,9 @@
/// e.g. because several of them are block-type.
unsigned LongestObjCSelectorName = 0;
- /// How many parts ObjC selector have (i.e. how many parameters method
- /// has).
+ /// If this is the first ObjC selector name in an ObjC method
+ /// definition or call, this contains the number of parts that the whole
+ /// selector consist of.
unsigned ObjCSelectorNameParts = 0;
/// Stores the number of required fake parentheses and the
Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -515,11 +515,23 @@
}
Left->MatchingParen = CurrentToken;
CurrentToken->MatchingParen = Left;
+// FirstObjCSelectorName is set when a colon is found. This does
+// not work, however, when the method has no parameters.
+// Here, we set FirstObjCSelectorName when the end of the method call is
+// reached, in case it was not set already.
+if (!Contexts.back().FirstObjCSelectorName) {
+FormatToken* Previous = Curr
[PATCH] D48718: [clang-format/ObjC] Prohibit breaking after a bracket opening ObjC method expression
This revision was automatically updated to reflect the committed changes. Closed by commit rL336519: [clang-format/ObjC] Prohibit breaking after a bracket opening ObjC method… (authored by jolesiak, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D48718 Files: cfe/trunk/lib/Format/ContinuationIndenter.cpp Index: cfe/trunk/lib/Format/ContinuationIndenter.cpp === --- cfe/trunk/lib/Format/ContinuationIndenter.cpp +++ cfe/trunk/lib/Format/ContinuationIndenter.cpp @@ -321,6 +321,9 @@ State.Stack.back().NoLineBreakInOperand) return false; + if (Previous.is(tok::l_square) && Previous.is(TT_ObjCMethodExpr)) +return false; + return !State.Stack.back().NoLineBreak; } Index: cfe/trunk/lib/Format/ContinuationIndenter.cpp === --- cfe/trunk/lib/Format/ContinuationIndenter.cpp +++ cfe/trunk/lib/Format/ContinuationIndenter.cpp @@ -321,6 +321,9 @@ State.Stack.back().NoLineBreakInOperand) return false; + if (Previous.is(tok::l_square) && Previous.is(TT_ObjCMethodExpr)) +return false; + return !State.Stack.back().NoLineBreak; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48719: [clang-format/ObjC] Improve split priorities for ObjC methods
jolesiak closed this revision. jolesiak added a comment. This change was submitted (for some reason (probably some rebase operations) it was not automatically connected to the commit): https://reviews.llvm.org/rC336520, https://reviews.llvm.org/rL336520 Repository: rC Clang https://reviews.llvm.org/D48719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48720: [clang-format/ObjC] Put ObjC method arguments into one line when they fit
jolesiak closed this revision. jolesiak added a comment. This change was submitted (for some reason (probably some rebase operations) it was not automatically connected to the commit): https://reviews.llvm.org/rC336521, https://reviews.llvm.org/rL336521. Repository: rC Clang https://reviews.llvm.org/D48720 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44634: [clang-format] Detect Objective-C for #import
jolesiak added a comment. Herald added a subscriber: acoomans. I think we should reconsider applying this change, at least for a very common "#import ". Repository: rC Clang https://reviews.llvm.org/D44634 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41108: [clang-format] Improve ObjC headers detection.
jolesiak created this revision.
jolesiak added a reviewer: krasimir.
Herald added a subscriber: klimek.
This patch improves detection of ObjC header files.
Right now many ObjC headers, especially short ones, are categorized as C/C++.
Way of filtering still isn't the best, as most likely it should be token-based.
Repository:
rC Clang
https://reviews.llvm.org/D41108
Files:
lib/Format/Format.cpp
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -79,6 +79,17 @@
ASSERT_TRUE((bool)Style);
EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+ Style = getStyle("LLVM", "a.h", "none", "@interface\n"
+ "@end\n"
+ "//comment");
+ ASSERT_TRUE((bool)Style);
+ EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
+ Style = getStyle("LLVM", "a.h", "none", "@interface\n"
+ "@end //comment");
+ ASSERT_TRUE((bool)Style);
+ EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
// No recognizable ObjC.
Style = getStyle("LLVM", "a.h", "none", "void f() {}");
ASSERT_TRUE((bool)Style);
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -2129,7 +2129,9 @@
// should be improved over time and probably be done on tokens, not one the
// bare content of the file.
if (Style.Language == FormatStyle::LK_Cpp && FileName.endswith(".h") &&
- (Code.contains("\n- (") || Code.contains("\n+ (")))
+ (Code.contains("\n- (") || Code.contains("\n+ (") ||
+ Code.contains("\n@end\n") || Code.contains("\n@end ") ||
+ Code.endswith("@end")))
Style.Language = FormatStyle::LK_ObjC;
FormatStyle FallbackStyle = getNoStyle();
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -79,6 +79,17 @@
ASSERT_TRUE((bool)Style);
EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+ Style = getStyle("LLVM", "a.h", "none", "@interface\n"
+ "@end\n"
+ "//comment");
+ ASSERT_TRUE((bool)Style);
+ EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
+ Style = getStyle("LLVM", "a.h", "none", "@interface\n"
+ "@end //comment");
+ ASSERT_TRUE((bool)Style);
+ EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
// No recognizable ObjC.
Style = getStyle("LLVM", "a.h", "none", "void f() {}");
ASSERT_TRUE((bool)Style);
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -2129,7 +2129,9 @@
// should be improved over time and probably be done on tokens, not one the
// bare content of the file.
if (Style.Language == FormatStyle::LK_Cpp && FileName.endswith(".h") &&
- (Code.contains("\n- (") || Code.contains("\n+ (")))
+ (Code.contains("\n- (") || Code.contains("\n+ (") ||
+ Code.contains("\n@end\n") || Code.contains("\n@end ") ||
+ Code.endswith("@end")))
Style.Language = FormatStyle::LK_ObjC;
FormatStyle FallbackStyle = getNoStyle();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D77039: [clang-format] Don't break multi block parameters on ObjCBreakBeforeNestedBlockParam
jolesiak accepted this revision. jolesiak added a comment. This revision is now accepted and ready to land. Makes sense, thanks for patching this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77039/new/ https://reviews.llvm.org/D77039 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44632: [clang-format] Add a few more Core Graphics identifiers to ObjC heuristic
jolesiak added inline comments.
Comment at: lib/Format/Format.cpp:1450
// Keep this array sorted, since we are binary searching over it.
static constexpr llvm::StringLiteral FoundationIdentifiers[] = {
"CGFloat",
krasimir wrote:
> jolesiak wrote:
> > benhamilton wrote:
> > > djasper wrote:
> > > > benhamilton wrote:
> > > > > jolesiak wrote:
> > > > > > djasper wrote:
> > > > > > > I have concerns about this growing lists of things. Specifically:
> > > > > > > - Keeping it sorted is a maintenance concern.
> > > > > > > - Doing binary search for basically every identifier in a header
> > > > > > > seems an unnecessary waste.
> > > > > > >
> > > > > > > Could we just create a hash set of these?
> > > > > > It was a hash set initially: D42135
> > > > > > Changed in: D42189
> > > > > Happy to do whatever folks recommend; I assume @krasimir's concern in
> > > > > D42189 was the startup cost of creating the (read-only) hash set.
> > > > >
> > > > > We can automate keeping this sorted with an `arc lint` check, they're
> > > > > quite easy to write:
> > > > >
> > > > > https://secure.phabricator.com/book/phabricator/article/arcanist_lint/
> > > > Krasimir clarified this to me offline. I have no concerns staying with
> > > > binary search here and for this patch so long as someone builds in an
> > > > assert that warns us when the strings here are not in the right order
> > > > at some point.
> > > Good call, added `assert(std::is_sorted(...))`.
> > >
> > > I tried `static_assert`, but `std::is_sorted()` is not `constexpr` until
> > > c++2x.
> > Checking this type of constraints in `arc lint` sounds rather weird. I like
> > this assert as testing private methods is painful.
> I now think a hash set here is better. Sent https://reviews.llvm.org/D44695
> to replace the array with that.
>
> Sorry for wasting everybody's time.
Then assertion is no longer needed.
Repository:
rC Clang
https://reviews.llvm.org/D44632
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44994: [clang-format] Ensure wrapped ObjC selectors with 1 arg obey IndentWrappedFunctionNames
jolesiak accepted this revision. jolesiak added a comment. This revision is now accepted and ready to land. Well spotted. Repository: rC Clang https://reviews.llvm.org/D44994 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44996: [clang-format] Ensure ObjC selectors with 0 args are annotated correctly
jolesiak added inline comments.
Comment at: unittests/Format/FormatTestObjC.cpp:527
+ // Make sure selectors with 0, 1, or more arguments are not indented
+ // when IndentWrappedFunctionNames is false.
+ verifyFormat("- (a)\n"
I know that `Style.IndentWrappedFunctionNames` is false by default, but how
about adding
```
Style.IndentWrappedFunctionNames = false;
```
before these `verifyFormat` calls? I feel like that makes it more readable and
makes test independent on this `Style` constant.
Repository:
rC Clang
https://reviews.llvm.org/D44996
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44996: [clang-format] Ensure ObjC selectors with 0 args are annotated correctly
jolesiak added inline comments.
Comment at: unittests/Format/FormatTestObjC.cpp:527
+ // Make sure selectors with 0, 1, or more arguments are not indented
+ // when IndentWrappedFunctionNames is false.
+ verifyFormat("- (a)\n"
jolesiak wrote:
> I know that `Style.IndentWrappedFunctionNames` is false by default, but how
> about adding
> ```
> Style.IndentWrappedFunctionNames = false;
> ```
> before these `verifyFormat` calls? I feel like that makes it more readable
> and makes test independent on this `Style` constant.
This comment is kind of irrelevant as this change is included in D45005.
Repository:
rC Clang
https://reviews.llvm.org/D44996
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44996: [clang-format] Ensure ObjC selectors with 0 args are annotated correctly
jolesiak added inline comments.
Comment at: unittests/Format/FormatTestObjC.cpp:527
+ // Make sure selectors with 0, 1, or more arguments are not indented
+ // when IndentWrappedFunctionNames is false.
+ verifyFormat("- (a)\n"
jolesiak wrote:
> jolesiak wrote:
> > I know that `Style.IndentWrappedFunctionNames` is false by default, but how
> > about adding
> > ```
> > Style.IndentWrappedFunctionNames = false;
> > ```
> > before these `verifyFormat` calls? I feel like that makes it more readable
> > and makes test independent on this `Style` constant.
> This comment is kind of irrelevant as this change is included in D45005.
My bad, this change is NOT included in D45005. (I don't know how to remove
inline comment)
Repository:
rC Clang
https://reviews.llvm.org/D44996
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44996: [clang-format] Ensure ObjC selectors with 0 args are annotated correctly
jolesiak accepted this revision. jolesiak added a comment. This revision is now accepted and ready to land. lgtm Repository: rC Clang https://reviews.llvm.org/D44996 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45004: [clang-format] New style option IndentWrappedObjCMethodNames
jolesiak added a comment. > I am proposing to update the Google Objective-C style guide to explicitly > describe the desired indentation behavior. +1 Repository: rC Clang https://reviews.llvm.org/D45004 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42493: [clang-format] Fix ObjC message arguments formatting.
jolesiak created this revision.
Herald added subscribers: cfe-commits, klimek.
jolesiak edited the summary of this revision.
Fixes formatting of ObjC message arguments when inline block is a first
argument.
Having inline block as a first when method has multiple parameters is
discouraged by Apple:
"It’s best practice to use only one block argument to a method. If the
method also needs other non-block arguments, the block should come last"
(https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/ProgrammingWithObjectiveC/WorkingwithBlocks/WorkingwithBlocks.html#//apple_ref/doc/uid/TP40011210-CH8-SW7),
it should be correctly formatted nevertheless.
Current formatting:
[object blockArgument:^{
a = 42;
}
anotherArg:42];
Fixed (colon alignment):
[object
blockArgument:^{
a = 42;
}
anotherArg:42];
Test Plan: make -j12 FormatTests && tools/clang/unittests/Format/FormatTests
Repository:
rC Clang
https://reviews.llvm.org/D42493
Files:
lib/Format/ContinuationIndenter.cpp
lib/Format/FormatToken.h
lib/Format/TokenAnnotator.cpp
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -655,6 +655,31 @@
"ofSize:aa:bbb\n"
" atOrigin:cc:dd];");
+ // Inline block as a first argument.
+ verifyFormat("[object justBlock:^{\n"
+ " a = 42;\n"
+ "}];");
+ verifyFormat("[object\n"
+ "justBlock:^{\n"
+ " a = 42;\n"
+ "}\n"
+ " notBlock:42\n"
+ "a:42];");
+ verifyFormat("[object\n"
+ "firstBlock:^{\n"
+ " a = 42;\n"
+ "}\n"
+ "blockWithLongerName:^{\n"
+ " a = 42;\n"
+ "}];");
+ verifyFormat("[object\n"
+ "blockWithLongerName:^{\n"
+ " a = 42;\n"
+ "}\n"
+ "secondBlock:^{\n"
+ " a = 42;\n"
+ "}];");
+
Style.ColumnLimit = 70;
verifyFormat(
"void f() {\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -401,6 +401,8 @@
if (Contexts.back().FirstObjCSelectorName) {
Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName =
Contexts.back().LongestObjCSelectorName;
+ Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts =
+ Left->ParameterCount;
if (Left->BlockParameterCount > 1)
Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName = 0;
}
@@ -414,6 +416,7 @@
TT_DesignatedInitializerLSquare)) {
Left->Type = TT_ObjCMethodExpr;
StartsObjCMethodExpr = true;
+ Left->ParameterCount = 0;
Contexts.back().ColonIsObjCMethodExpr = true;
if (Parent && Parent->is(tok::r_paren))
Parent->Type = TT_CastRParen;
@@ -486,7 +489,10 @@
void updateParameterCount(FormatToken *Left, FormatToken *Current) {
if (Current->is(tok::l_brace) && Current->BlockKind == BK_Block)
++Left->BlockParameterCount;
-if (Current->is(tok::comma)) {
+if (Left->Type == TT_ObjCMethodExpr) {
+ if (Current->is(tok::colon))
+++Left->ParameterCount;
+} else if (Current->is(tok::comma)) {
++Left->ParameterCount;
if (!Left->Role)
Left->Role.reset(new CommaSeparatedList(Style));
Index: lib/Format/FormatToken.h
===
--- lib/Format/FormatToken.h
+++ lib/Format/FormatToken.h
@@ -240,6 +240,10 @@
/// e.g. because several of them are block-type.
unsigned LongestObjCSelectorName = 0;
+ /// \brief How many parts ObjC selector have (i.e. how many parameters method
+ /// has).
+ unsigned ObjCSelectorNameParts = 0;
+
/// \brief Stores the number of required fake parentheses and the
/// corresponding operator precedence.
///
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -266,6 +266,11 @@
return true;
if (Previous.is(tok::semi) && State.LineContainsContinuedForLoopSection)
return true;
+ if (Style.Language == FormatStyle::LK_ObjC &&
+ Current.ObjCSelectorNameParts > 1 &&
+ Current.startsSequence(TT_SelectorName, tok::colon, tok::caret)) {
+return true;
+ }
if ((startsNextParameter(Current, Style) || Previous.is(tok::semi) ||
(Previous.is(TT_TemplateCloser) && Current.is(TT_StartOfName) &&
Style.isCpp() &&
[PATCH] D42395: [clang-format] Fix bug where -dump-config failed on ObjC header
jolesiak requested changes to this revision. jolesiak added inline comments. This revision now requires changes to proceed. Comment at: test/Format/lit.local.cfg:2-3 +# Suffixes supported by clang-format. +config.suffixes = ['.cpp', '.h', '.m', '.mm', '.java', '.js', '.ts', '.proto', + '.protodevel', '.pb.txt', '.textproto', '.asciipb', '.td'] Where does this list come from? Shouldn't '.textpb', 'METADATA', '.c', '.cc' be added here? Repository: rC Clang https://reviews.llvm.org/D42395 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42493: [clang-format] Fix ObjC message arguments formatting.
jolesiak added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:419 StartsObjCMethodExpr = true; + Left->ParameterCount = 0; Contexts.back().ColonIsObjCMethodExpr = true; benhamilton wrote: > What does this line do? Seems like it's initialized to 0 already, right? It is indeed initialized to 0. However, before 'Left' bracket is recognized as TT_ObjCMethodExpr it has a different type assigned. Hence it gets updated here: https://github.com/llvm-mirror/clang/blob/release_60/lib/Format/TokenAnnotator.cpp#L495 This assignment is due to fact that for other languages number of parameters is calculated as (1 + number_of_commas). Repository: rC Clang https://reviews.llvm.org/D42493 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42493: [clang-format] Fix ObjC message arguments formatting.
jolesiak updated this revision to Diff 131825.
jolesiak added a comment.
- Add comment explaining ParameterCount reset.
Repository:
rC Clang
https://reviews.llvm.org/D42493
Files:
lib/Format/ContinuationIndenter.cpp
lib/Format/FormatToken.h
lib/Format/TokenAnnotator.cpp
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -655,6 +655,31 @@
"ofSize:aa:bbb\n"
" atOrigin:cc:dd];");
+ // Inline block as a first argument.
+ verifyFormat("[object justBlock:^{\n"
+ " a = 42;\n"
+ "}];");
+ verifyFormat("[object\n"
+ "justBlock:^{\n"
+ " a = 42;\n"
+ "}\n"
+ " notBlock:42\n"
+ "a:42];");
+ verifyFormat("[object\n"
+ "firstBlock:^{\n"
+ " a = 42;\n"
+ "}\n"
+ "blockWithLongerName:^{\n"
+ " a = 42;\n"
+ "}];");
+ verifyFormat("[object\n"
+ "blockWithLongerName:^{\n"
+ " a = 42;\n"
+ "}\n"
+ "secondBlock:^{\n"
+ " a = 42;\n"
+ "}];");
+
Style.ColumnLimit = 70;
verifyFormat(
"void f() {\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -401,6 +401,8 @@
if (Contexts.back().FirstObjCSelectorName) {
Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName =
Contexts.back().LongestObjCSelectorName;
+ Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts =
+ Left->ParameterCount;
if (Left->BlockParameterCount > 1)
Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName = 0;
}
@@ -414,6 +416,11 @@
TT_DesignatedInitializerLSquare)) {
Left->Type = TT_ObjCMethodExpr;
StartsObjCMethodExpr = true;
+ // ParameterCount might have been set to 1 before expression was
+ // recognized as ObjCMethodExpr (as '1 + number of commas' formula is
+ // used for other expression types). Parameter counter has to be,
+ // therefore, reset to 0.
+ Left->ParameterCount = 0;
Contexts.back().ColonIsObjCMethodExpr = true;
if (Parent && Parent->is(tok::r_paren))
Parent->Type = TT_CastRParen;
@@ -486,7 +493,10 @@
void updateParameterCount(FormatToken *Left, FormatToken *Current) {
if (Current->is(tok::l_brace) && Current->BlockKind == BK_Block)
++Left->BlockParameterCount;
-if (Current->is(tok::comma)) {
+if (Left->Type == TT_ObjCMethodExpr) {
+ if (Current->is(tok::colon))
+++Left->ParameterCount;
+} else if (Current->is(tok::comma)) {
++Left->ParameterCount;
if (!Left->Role)
Left->Role.reset(new CommaSeparatedList(Style));
Index: lib/Format/FormatToken.h
===
--- lib/Format/FormatToken.h
+++ lib/Format/FormatToken.h
@@ -240,6 +240,10 @@
/// e.g. because several of them are block-type.
unsigned LongestObjCSelectorName = 0;
+ /// \brief How many parts ObjC selector have (i.e. how many parameters method
+ /// has).
+ unsigned ObjCSelectorNameParts = 0;
+
/// \brief Stores the number of required fake parentheses and the
/// corresponding operator precedence.
///
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -266,6 +266,11 @@
return true;
if (Previous.is(tok::semi) && State.LineContainsContinuedForLoopSection)
return true;
+ if (Style.Language == FormatStyle::LK_ObjC &&
+ Current.ObjCSelectorNameParts > 1 &&
+ Current.startsSequence(TT_SelectorName, tok::colon, tok::caret)) {
+return true;
+ }
if ((startsNextParameter(Current, Style) || Previous.is(tok::semi) ||
(Previous.is(TT_TemplateCloser) && Current.is(TT_StartOfName) &&
Style.isCpp() &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42493: [clang-format] Fix ObjC message arguments formatting.
jolesiak updated this revision to Diff 131943.
jolesiak added a comment.
- Add test
Repository:
rC Clang
https://reviews.llvm.org/D42493
Files:
lib/Format/ContinuationIndenter.cpp
lib/Format/FormatToken.h
lib/Format/TokenAnnotator.cpp
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -655,6 +655,39 @@
"ofSize:aa:bbb\n"
" atOrigin:cc:dd];");
+ // Inline block as a first argument.
+ verifyFormat("[object justBlock:^{\n"
+ " a = 42;\n"
+ "}];");
+ verifyFormat("[object\n"
+ "justBlock:^{\n"
+ " a = 42;\n"
+ "}\n"
+ " notBlock:42\n"
+ "a:42];");
+ verifyFormat("[object\n"
+ "firstBlock:^{\n"
+ " a = 42;\n"
+ "}\n"
+ "blockWithLongerName:^{\n"
+ " a = 42;\n"
+ "}];");
+ verifyFormat("[object\n"
+ "blockWithLongerName:^{\n"
+ " a = 42;\n"
+ "}\n"
+ "secondBlock:^{\n"
+ " a = 42;\n"
+ "}];");
+ verifyFormat("[object\n"
+ "firstBlock:^{\n"
+ " a = 42;\n"
+ "}\n"
+ "notBlock:42\n"
+ "secondBlock:^{\n"
+ " a = 42;\n"
+ "}];");
+
Style.ColumnLimit = 70;
verifyFormat(
"void f() {\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -401,6 +401,8 @@
if (Contexts.back().FirstObjCSelectorName) {
Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName =
Contexts.back().LongestObjCSelectorName;
+ Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts =
+ Left->ParameterCount;
if (Left->BlockParameterCount > 1)
Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName = 0;
}
@@ -414,6 +416,11 @@
TT_DesignatedInitializerLSquare)) {
Left->Type = TT_ObjCMethodExpr;
StartsObjCMethodExpr = true;
+ // ParameterCount might have been set to 1 before expression was
+ // recognized as ObjCMethodExpr (as '1 + number of commas' formula is
+ // used for other expression types). Parameter counter has to be,
+ // therefore, reset to 0.
+ Left->ParameterCount = 0;
Contexts.back().ColonIsObjCMethodExpr = true;
if (Parent && Parent->is(tok::r_paren))
Parent->Type = TT_CastRParen;
@@ -486,7 +493,10 @@
void updateParameterCount(FormatToken *Left, FormatToken *Current) {
if (Current->is(tok::l_brace) && Current->BlockKind == BK_Block)
++Left->BlockParameterCount;
-if (Current->is(tok::comma)) {
+if (Left->Type == TT_ObjCMethodExpr) {
+ if (Current->is(tok::colon))
+++Left->ParameterCount;
+} else if (Current->is(tok::comma)) {
++Left->ParameterCount;
if (!Left->Role)
Left->Role.reset(new CommaSeparatedList(Style));
Index: lib/Format/FormatToken.h
===
--- lib/Format/FormatToken.h
+++ lib/Format/FormatToken.h
@@ -240,6 +240,10 @@
/// e.g. because several of them are block-type.
unsigned LongestObjCSelectorName = 0;
+ /// \brief How many parts ObjC selector have (i.e. how many parameters method
+ /// has).
+ unsigned ObjCSelectorNameParts = 0;
+
/// \brief Stores the number of required fake parentheses and the
/// corresponding operator precedence.
///
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -266,6 +266,11 @@
return true;
if (Previous.is(tok::semi) && State.LineContainsContinuedForLoopSection)
return true;
+ if (Style.Language == FormatStyle::LK_ObjC &&
+ Current.ObjCSelectorNameParts > 1 &&
+ Current.startsSequence(TT_SelectorName, tok::colon, tok::caret)) {
+return true;
+ }
if ((startsNextParameter(Current, Style) || Previous.is(tok::semi) ||
(Previous.is(TT_TemplateCloser) && Current.is(TT_StartOfName) &&
Style.isCpp() &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42708: [clang-format] Set BinPackObjCProtocolList to Never for google style
jolesiak added inline comments. Comment at: lib/Format/Format.cpp:765 GoogleStyle.ColumnLimit = 100; +GoogleStyle.BinPackObjCProtocolList = FormatStyle::BPS_Never; } If I understand correctly this is meant to be ObjC-specific flag. I feel like this should be put in general config (next to GoogleStyle.ObjcSpaceAfterProperty and GoogleStyle.ObjCSpaceBeforeProtocolList; ~50 lines above). To me putting it inside 'if' check means different value is used for ObjC than for other languages (what isn't the case here, as this flag shouldn't be used in formatting other languages code). Repository: rC Clang https://reviews.llvm.org/D42708 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42901: Test commit - fixing a comment.
jolesiak created this revision.
Herald added subscribers: cfe-commits, klimek.
jolesiak added reviewers: krasimir, benhamilton.
A test commit.
Repository:
rC Clang
https://reviews.llvm.org/D42901
Files:
lib/Format/ContinuationIndenter.h
Index: lib/Format/ContinuationIndenter.h
===
--- lib/Format/ContinuationIndenter.h
+++ lib/Format/ContinuationIndenter.h
@@ -314,8 +314,8 @@
/// the same token.
bool HasMultipleNestedBlocks : 1;
- // \brief The start of a nested block (e.g. lambda introducer in C++ or
- // "function" in JavaScript) is not wrapped to a new line.
+ /// \brief The start of a nested block (e.g. lambda introducer in C++ or
+ /// "function" in JavaScript) is not wrapped to a new line.
bool NestedBlockInlined : 1;
bool operator<(const ParenState &Other) const {
Index: lib/Format/ContinuationIndenter.h
===
--- lib/Format/ContinuationIndenter.h
+++ lib/Format/ContinuationIndenter.h
@@ -314,8 +314,8 @@
/// the same token.
bool HasMultipleNestedBlocks : 1;
- // \brief The start of a nested block (e.g. lambda introducer in C++ or
- // "function" in JavaScript) is not wrapped to a new line.
+ /// \brief The start of a nested block (e.g. lambda introducer in C++ or
+ /// "function" in JavaScript) is not wrapped to a new line.
bool NestedBlockInlined : 1;
bool operator<(const ParenState &Other) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
