[PATCH] D43906: [clang-format] Improve detection of Objective-C block types
This revision was automatically updated to reflect the committed changes.
benhamilton marked an inline comment as done.
Closed by commit rC327285: [clang-format] Improve detection of Objective-C
block types (authored by benhamilton, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D43906?vs=138024=138026#toc
Repository:
rC Clang
https://reviews.llvm.org/D43906
Files:
lib/Format/TokenAnnotator.cpp
unittests/Format/FormatTest.cpp
unittests/Format/FormatTestObjC.cpp
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -141,10 +141,7 @@
Contexts.size() == 2 && Contexts[0].ColonIsForRangeExpr;
bool StartsObjCMethodExpr = false;
-if (CurrentToken->is(tok::caret)) {
- // (^ can start a block type.
- Left->Type = TT_ObjCBlockLParen;
-} else if (FormatToken *MaybeSel = Left->Previous) {
+if (FormatToken *MaybeSel = Left->Previous) {
// @selector( starts a selector.
if (MaybeSel->isObjCAtKeyword(tok::objc_selector) && MaybeSel->Previous
&&
MaybeSel->Previous->is(tok::at)) {
@@ -210,8 +207,16 @@
Left->Type = TT_ObjCMethodExpr;
}
+// MightBeFunctionType and ProbablyFunctionType are used for
+// function pointer and reference types as well as Objective-C
+// block types:
+//
+// void (*FunctionPointer)(void);
+// void ()(void);
+// void (^ObjCBlock)(void);
bool MightBeFunctionType = !Contexts[Contexts.size() - 2].IsExpression;
-bool ProbablyFunctionType = CurrentToken->isOneOf(tok::star, tok::amp);
+bool ProbablyFunctionType =
+CurrentToken->isOneOf(tok::star, tok::amp, tok::caret);
bool HasMultipleLines = false;
bool HasMultipleParametersOnALine = false;
bool MightBeObjCForRangeLoop =
@@ -248,7 +253,8 @@
if (MightBeFunctionType && ProbablyFunctionType && CurrentToken->Next
&&
(CurrentToken->Next->is(tok::l_paren) ||
(CurrentToken->Next->is(tok::l_square) &&
Line.MustBeDeclaration)))
- Left->Type = TT_FunctionTypeLParen;
+ Left->Type = Left->Next->is(tok::caret) ? TT_ObjCBlockLParen
+ : TT_FunctionTypeLParen;
Left->MatchingParen = CurrentToken;
CurrentToken->MatchingParen = Left;
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12128,6 +12128,22 @@
EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "[[foo::bar, ...]]"));
}
+TEST_F(FormatTest, GuessLanguageWithCaret) {
+ EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "FOO(^);"));
+ EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "FOO(^, Bar);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^)(char, float);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^foo)(char, float);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^foo[10])(char, float);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^foo[kNumEntries])(char, float);"));
+ EXPECT_EQ(
+ FormatStyle::LK_ObjC,
+ guessLanguage("foo.h", "int(^foo[(kNumEntries + 10)])(char, float);"));
+}
+
} // end namespace
} // end namespace format
} // end namespace clang
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -845,6 +845,15 @@
verifyFormat("@ /*foo*/ interface");
}
+TEST_F(FormatTestObjC, ObjCBlockTypesAndVariables) {
+ verifyFormat("void DoStuffWithBlockType(int (^)(char));");
+ verifyFormat("int (^foo)(char, float);");
+ verifyFormat("int (^foo[10])(char, float);");
+ verifyFormat("int (^foo[kNumEntries])(char, float);");
+ verifyFormat("int (^foo[kNumEntries + 10])(char, float);");
+ verifyFormat("int (^foo[(kNumEntries + 10)])(char, float);");
+}
+
TEST_F(FormatTestObjC, ObjCSnippets) {
verifyFormat("@autoreleasepool {\n"
" foo();\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -141,10 +141,7 @@
Contexts.size() == 2 && Contexts[0].ColonIsForRangeExpr;
bool StartsObjCMethodExpr = false;
-if (CurrentToken->is(tok::caret)) {
- // (^ can start a block type.
- Left->Type = TT_ObjCBlockLParen;
-} else if (FormatToken *MaybeSel = Left->Previous) {
+if (FormatToken *MaybeSel = Left->Previous) {
// @selector( starts a selector.
if (MaybeSel->isObjCAtKeyword(tok::objc_selector) && MaybeSel->Previous &&
MaybeSel->Previous->is(tok::at)) {
@@ -210,8
[PATCH] D43906: [clang-format] Improve detection of Objective-C block types
benhamilton updated this revision to Diff 138024.
benhamilton added a comment.
- Restore short functionn type variable names and add clarifying comment.
Repository:
rC Clang
https://reviews.llvm.org/D43906
Files:
lib/Format/TokenAnnotator.cpp
unittests/Format/FormatTest.cpp
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -845,6 +845,15 @@
verifyFormat("@ /*foo*/ interface");
}
+TEST_F(FormatTestObjC, ObjCBlockTypesAndVariables) {
+ verifyFormat("void DoStuffWithBlockType(int (^)(char));");
+ verifyFormat("int (^foo)(char, float);");
+ verifyFormat("int (^foo[10])(char, float);");
+ verifyFormat("int (^foo[kNumEntries])(char, float);");
+ verifyFormat("int (^foo[kNumEntries + 10])(char, float);");
+ verifyFormat("int (^foo[(kNumEntries + 10)])(char, float);");
+}
+
TEST_F(FormatTestObjC, ObjCSnippets) {
verifyFormat("@autoreleasepool {\n"
" foo();\n"
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12128,6 +12128,22 @@
EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "[[foo::bar, ...]]"));
}
+TEST_F(FormatTest, GuessLanguageWithCaret) {
+ EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "FOO(^);"));
+ EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "FOO(^, Bar);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^)(char, float);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^foo)(char, float);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^foo[10])(char, float);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^foo[kNumEntries])(char, float);"));
+ EXPECT_EQ(
+ FormatStyle::LK_ObjC,
+ guessLanguage("foo.h", "int(^foo[(kNumEntries + 10)])(char, float);"));
+}
+
} // end namespace
} // end namespace format
} // end namespace clang
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -141,10 +141,7 @@
Contexts.size() == 2 && Contexts[0].ColonIsForRangeExpr;
bool StartsObjCMethodExpr = false;
-if (CurrentToken->is(tok::caret)) {
- // (^ can start a block type.
- Left->Type = TT_ObjCBlockLParen;
-} else if (FormatToken *MaybeSel = Left->Previous) {
+if (FormatToken *MaybeSel = Left->Previous) {
// @selector( starts a selector.
if (MaybeSel->isObjCAtKeyword(tok::objc_selector) && MaybeSel->Previous
&&
MaybeSel->Previous->is(tok::at)) {
@@ -210,8 +207,16 @@
Left->Type = TT_ObjCMethodExpr;
}
+// MightBeFunctionType and ProbablyFunctionType are used for
+// function pointer and reference types as well as Objective-C
+// block types:
+//
+// void (*FunctionPointer)(void);
+// void ()(void);
+// void (^ObjCBlock)(void);
bool MightBeFunctionType = !Contexts[Contexts.size() - 2].IsExpression;
-bool ProbablyFunctionType = CurrentToken->isOneOf(tok::star, tok::amp);
+bool ProbablyFunctionType =
+CurrentToken->isOneOf(tok::star, tok::amp, tok::caret);
bool HasMultipleLines = false;
bool HasMultipleParametersOnALine = false;
bool MightBeObjCForRangeLoop =
@@ -248,7 +253,8 @@
if (MightBeFunctionType && ProbablyFunctionType && CurrentToken->Next
&&
(CurrentToken->Next->is(tok::l_paren) ||
(CurrentToken->Next->is(tok::l_square) &&
Line.MustBeDeclaration)))
- Left->Type = TT_FunctionTypeLParen;
+ Left->Type = Left->Next->is(tok::caret) ? TT_ObjCBlockLParen
+ : TT_FunctionTypeLParen;
Left->MatchingParen = CurrentToken;
CurrentToken->MatchingParen = Left;
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -845,6 +845,15 @@
verifyFormat("@ /*foo*/ interface");
}
+TEST_F(FormatTestObjC, ObjCBlockTypesAndVariables) {
+ verifyFormat("void DoStuffWithBlockType(int (^)(char));");
+ verifyFormat("int (^foo)(char, float);");
+ verifyFormat("int (^foo[10])(char, float);");
+ verifyFormat("int (^foo[kNumEntries])(char, float);");
+ verifyFormat("int (^foo[kNumEntries + 10])(char, float);");
+ verifyFormat("int (^foo[(kNumEntries + 10)])(char, float);");
+}
+
TEST_F(FormatTestObjC, ObjCSnippets) {
verifyFormat("@autoreleasepool {\n"
" foo();\n"
Index: unittests/Format/FormatTest.cpp
===
---
[PATCH] D43906: [clang-format] Improve detection of Objective-C block types
benhamilton marked an inline comment as done. benhamilton added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:210 -bool MightBeFunctionType = !Contexts[Contexts.size() - 2].IsExpression; -bool ProbablyFunctionType = CurrentToken->isOneOf(tok::star, tok::amp); +bool MightBeFunctionOrObjCBlockType = +!Contexts[Contexts.size() - 2].IsExpression; djasper wrote: > I'd suggest to put a comment here saying that this is for both ObjC blocks > and Function types, because they look very similar in nature (maybe giving > examples) and then not actually rename the variables. To me, the long names > make the code harder to read. > > But if you feel strongly the other way, I'd be ok with it. Restored old names and added comment. Repository: rC Clang https://reviews.llvm.org/D43906 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43906: [clang-format] Improve detection of Objective-C block types
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Nice.. Removed a lot of complexity :). Let's see whether this heuristic is good enough. Comment at: lib/Format/TokenAnnotator.cpp:210 -bool MightBeFunctionType = !Contexts[Contexts.size() - 2].IsExpression; -bool ProbablyFunctionType = CurrentToken->isOneOf(tok::star, tok::amp); +bool MightBeFunctionOrObjCBlockType = +!Contexts[Contexts.size() - 2].IsExpression; I'd suggest to put a comment here saying that this is for both ObjC blocks and Function types, because they look very similar in nature (maybe giving examples) and then not actually rename the variables. To me, the long names make the code harder to read. But if you feel strongly the other way, I'd be ok with it. Repository: rC Clang https://reviews.llvm.org/D43906 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43906: [clang-format] Improve detection of Objective-C block types
benhamilton added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:152 + const FormatToken *Next = CurrentToken->getNextNonComment(); + int ParenDepth = 1; + // Handle nested parens in case we have an array of blocks with djasper wrote: > No. Don't implement yet another parenthesis counting. This function already > iterates over all tokens until the closing paren. Just store a pointer to the > caret here and mark it (or unmark it) when encountering the closing brace > (line 271). There is already very similar logic there to set > TT_FunctionTypeLParen (which is actually doing the exact same parsing now > that I think of it). Oh! Yes, re-using the logic for `TT_FunctionTypeLParen` is clearly the right thing to do, since it has exactly the same syntax (just `*` or `&` instead of `^`). Done, code is much cleaner now. Repository: rC Clang https://reviews.llvm.org/D43906 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43906: [clang-format] Improve detection of Objective-C block types
benhamilton updated this revision to Diff 137394.
benhamilton added a comment.
- Greatly clean up ObjC block type logic by re-using `TT_FunctionTypeLParen`
type logic.
Repository:
rC Clang
https://reviews.llvm.org/D43906
Files:
lib/Format/TokenAnnotator.cpp
unittests/Format/FormatTest.cpp
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -845,6 +845,15 @@
verifyFormat("@ /*foo*/ interface");
}
+TEST_F(FormatTestObjC, ObjCBlockTypesAndVariables) {
+ verifyFormat("void DoStuffWithBlockType(int (^)(char));");
+ verifyFormat("int (^foo)(char, float);");
+ verifyFormat("int (^foo[10])(char, float);");
+ verifyFormat("int (^foo[kNumEntries])(char, float);");
+ verifyFormat("int (^foo[kNumEntries + 10])(char, float);");
+ verifyFormat("int (^foo[(kNumEntries + 10)])(char, float);");
+}
+
TEST_F(FormatTestObjC, ObjCSnippets) {
verifyFormat("@autoreleasepool {\n"
" foo();\n"
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12128,6 +12128,22 @@
EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "[[foo::bar, ...]]"));
}
+TEST_F(FormatTest, GuessLanguageWithCaret) {
+ EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "FOO(^);"));
+ EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "FOO(^, Bar);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^)(char, float);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^foo)(char, float);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^foo[10])(char, float);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^foo[kNumEntries])(char, float);"));
+ EXPECT_EQ(
+ FormatStyle::LK_ObjC,
+ guessLanguage("foo.h", "int(^foo[(kNumEntries + 10)])(char, float);"));
+}
+
} // end namespace
} // end namespace format
} // end namespace clang
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -141,10 +141,7 @@
Contexts.size() == 2 && Contexts[0].ColonIsForRangeExpr;
bool StartsObjCMethodExpr = false;
-if (CurrentToken->is(tok::caret)) {
- // (^ can start a block type.
- Left->Type = TT_ObjCBlockLParen;
-} else if (FormatToken *MaybeSel = Left->Previous) {
+if (FormatToken *MaybeSel = Left->Previous) {
// @selector( starts a selector.
if (MaybeSel->isObjCAtKeyword(tok::objc_selector) && MaybeSel->Previous
&&
MaybeSel->Previous->is(tok::at)) {
@@ -210,8 +207,10 @@
Left->Type = TT_ObjCMethodExpr;
}
-bool MightBeFunctionType = !Contexts[Contexts.size() - 2].IsExpression;
-bool ProbablyFunctionType = CurrentToken->isOneOf(tok::star, tok::amp);
+bool MightBeFunctionOrObjCBlockType =
+!Contexts[Contexts.size() - 2].IsExpression;
+bool ProbablyFunctionOrObjCBlockType =
+CurrentToken->isOneOf(tok::star, tok::amp, tok::caret);
bool HasMultipleLines = false;
bool HasMultipleParametersOnALine = false;
bool MightBeObjCForRangeLoop =
@@ -239,16 +238,18 @@
if (CurrentToken->Previous->is(TT_PointerOrReference) &&
CurrentToken->Previous->Previous->isOneOf(tok::l_paren,
tok::coloncolon))
-ProbablyFunctionType = true;
+ProbablyFunctionOrObjCBlockType = true;
if (CurrentToken->is(tok::comma))
-MightBeFunctionType = false;
+MightBeFunctionOrObjCBlockType = false;
if (CurrentToken->Previous->is(TT_BinaryOperator))
Contexts.back().IsExpression = true;
if (CurrentToken->is(tok::r_paren)) {
-if (MightBeFunctionType && ProbablyFunctionType && CurrentToken->Next
&&
+if (MightBeFunctionOrObjCBlockType && ProbablyFunctionOrObjCBlockType
&&
+CurrentToken->Next &&
(CurrentToken->Next->is(tok::l_paren) ||
(CurrentToken->Next->is(tok::l_square) &&
Line.MustBeDeclaration)))
- Left->Type = TT_FunctionTypeLParen;
+ Left->Type = Left->Next->is(tok::caret) ? TT_ObjCBlockLParen
+ : TT_FunctionTypeLParen;
Left->MatchingParen = CurrentToken;
CurrentToken->MatchingParen = Left;
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -845,6 +845,15 @@
verifyFormat("@ /*foo*/ interface");
}
+TEST_F(FormatTestObjC, ObjCBlockTypesAndVariables) {
+ verifyFormat("void
[PATCH] D43906: [clang-format] Improve detection of Objective-C block types
benhamilton updated this revision to Diff 137320.
benhamilton added a comment.
Fix comment.
Repository:
rC Clang
https://reviews.llvm.org/D43906
Files:
lib/Format/TokenAnnotator.cpp
unittests/Format/FormatTest.cpp
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -845,6 +845,15 @@
verifyFormat("@ /*foo*/ interface");
}
+TEST_F(FormatTestObjC, ObjCBlockTypesAndVariables) {
+ verifyFormat("void DoStuffWithBlockType(int (^)(char));");
+ verifyFormat("int (^foo)(char, float);");
+ verifyFormat("int (^foo[10])(char, float);");
+ verifyFormat("int (^foo[kNumEntries])(char, float);");
+ verifyFormat("int (^foo[kNumEntries + 10])(char, float);");
+ verifyFormat("int (^foo[(kNumEntries + 10)])(char, float);");
+}
+
TEST_F(FormatTestObjC, ObjCSnippets) {
verifyFormat("@autoreleasepool {\n"
" foo();\n"
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12128,6 +12128,22 @@
EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "[[foo::bar, ...]]"));
}
+TEST_F(FormatTest, GuessLanguageWithCaret) {
+ EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "FOO(^);"));
+ EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "FOO(^, Bar);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^)(char, float);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^foo)(char, float);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^foo[10])(char, float);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^foo[kNumEntries])(char, float);"));
+ EXPECT_EQ(
+ FormatStyle::LK_ObjC,
+ guessLanguage("foo.h", "int(^foo[(kNumEntries + 10)])(char, float);"));
+}
+
} // end namespace
} // end namespace format
} // end namespace clang
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -142,8 +142,32 @@
bool StartsObjCMethodExpr = false;
if (CurrentToken->is(tok::caret)) {
- // (^ can start a block type.
- Left->Type = TT_ObjCBlockLParen;
+ // The following are valid ObjC block types and block variables:
+ //
+ // int (^)(char, float)
+ // int (^block)(char, float)
+ // int (^arrayOfTenBlocks[10])(char, float)
+ // int (^arrayOfManyBlocks[(kLen + 42)])(char, float)
+ const FormatToken *Next = CurrentToken->getNextNonComment();
+ int ParenDepth = 1;
+ // Handle nested parens in case we have an array of blocks with
+ // a parenthesized length.
+ while (Next) {
+if (Next->is(tok::l_paren))
+ ++ParenDepth;
+else if (Next->is(tok::r_paren))
+ --ParenDepth;
+if (ParenDepth == 0)
+ break;
+Next = Next->getNextNonComment();
+ }
+ if (Next) {
+const FormatToken *NextNext = Next->getNextNonComment();
+// If we have an opening paren at the end of (^...)( we assume
+// we have an Objective-C block.
+if (NextNext && NextNext->is(tok::l_paren))
+ Left->Type = TT_ObjCBlockLParen;
+ }
} else if (FormatToken *MaybeSel = Left->Previous) {
// @selector( starts a selector.
if (MaybeSel->isObjCAtKeyword(tok::objc_selector) && MaybeSel->Previous
&&
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -845,6 +845,15 @@
verifyFormat("@ /*foo*/ interface");
}
+TEST_F(FormatTestObjC, ObjCBlockTypesAndVariables) {
+ verifyFormat("void DoStuffWithBlockType(int (^)(char));");
+ verifyFormat("int (^foo)(char, float);");
+ verifyFormat("int (^foo[10])(char, float);");
+ verifyFormat("int (^foo[kNumEntries])(char, float);");
+ verifyFormat("int (^foo[kNumEntries + 10])(char, float);");
+ verifyFormat("int (^foo[(kNumEntries + 10)])(char, float);");
+}
+
TEST_F(FormatTestObjC, ObjCSnippets) {
verifyFormat("@autoreleasepool {\n"
" foo();\n"
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12128,6 +12128,22 @@
EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "[[foo::bar, ...]]"));
}
+TEST_F(FormatTest, GuessLanguageWithCaret) {
+ EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "FOO(^);"));
+ EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "FOO(^, Bar);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h",
[PATCH] D43906: [clang-format] Improve detection of Objective-C block types
benhamilton added a comment.
> Right. So the difference is that for blocks, there is always a "(" after the
> "(^.)". That should be easy to parse, no?
Yep, I've updated this diff to implement that as you recommended. I just had to
handle nested parens (I couldn't find an existing way to deal with these so I
wrote my own—let me know if I missed something.)
Repository:
rC Clang
https://reviews.llvm.org/D43906
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43906: [clang-format] Improve detection of Objective-C block types
benhamilton updated this revision to Diff 137319.
benhamilton added a comment.
- Simplify logic and assume we have an ObjC block whenever we see rparen at the
end of `(^...)(`.
Repository:
rC Clang
https://reviews.llvm.org/D43906
Files:
lib/Format/TokenAnnotator.cpp
unittests/Format/FormatTest.cpp
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -845,6 +845,15 @@
verifyFormat("@ /*foo*/ interface");
}
+TEST_F(FormatTestObjC, ObjCBlockTypesAndVariables) {
+ verifyFormat("void DoStuffWithBlockType(int (^)(char));");
+ verifyFormat("int (^foo)(char, float);");
+ verifyFormat("int (^foo[10])(char, float);");
+ verifyFormat("int (^foo[kNumEntries])(char, float);");
+ verifyFormat("int (^foo[kNumEntries + 10])(char, float);");
+ verifyFormat("int (^foo[(kNumEntries + 10)])(char, float);");
+}
+
TEST_F(FormatTestObjC, ObjCSnippets) {
verifyFormat("@autoreleasepool {\n"
" foo();\n"
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12128,6 +12128,22 @@
EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "[[foo::bar, ...]]"));
}
+TEST_F(FormatTest, GuessLanguageWithCaret) {
+ EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "FOO(^);"));
+ EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "FOO(^, Bar);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^)(char, float);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^foo)(char, float);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^foo[10])(char, float);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^foo[kNumEntries])(char, float);"));
+ EXPECT_EQ(
+ FormatStyle::LK_ObjC,
+ guessLanguage("foo.h", "int(^foo[(kNumEntries + 10)])(char, float);"));
+}
+
} // end namespace
} // end namespace format
} // end namespace clang
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -142,8 +142,32 @@
bool StartsObjCMethodExpr = false;
if (CurrentToken->is(tok::caret)) {
- // (^ can start a block type.
- Left->Type = TT_ObjCBlockLParen;
+ // The following are valid block ObjC types and block variables:
+ //
+ // int (^)(char, float)
+ // int (^block)(char, float)
+ // int (^arrayOfTenBlocks[10])(char, float)
+ // int (^arrayOfManyBlocks[(kLen + 42)])(char, float)
+ const FormatToken *Next = CurrentToken->getNextNonComment();
+ int ParenDepth = 1;
+ // Handle nested parens in case we have an array of blocks with
+ // a parenthesized length.
+ while (Next) {
+if (Next->is(tok::l_paren))
+ ++ParenDepth;
+else if (Next->is(tok::r_paren))
+ --ParenDepth;
+if (ParenDepth == 0)
+ break;
+Next = Next->getNextNonComment();
+ }
+ if (Next) {
+const FormatToken *NextNext = Next->getNextNonComment();
+// If we have an opening paren at the end of (^...)( we assume
+// we have an Objective-C block.
+if (NextNext && NextNext->is(tok::l_paren))
+ Left->Type = TT_ObjCBlockLParen;
+ }
} else if (FormatToken *MaybeSel = Left->Previous) {
// @selector( starts a selector.
if (MaybeSel->isObjCAtKeyword(tok::objc_selector) && MaybeSel->Previous
&&
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -845,6 +845,15 @@
verifyFormat("@ /*foo*/ interface");
}
+TEST_F(FormatTestObjC, ObjCBlockTypesAndVariables) {
+ verifyFormat("void DoStuffWithBlockType(int (^)(char));");
+ verifyFormat("int (^foo)(char, float);");
+ verifyFormat("int (^foo[10])(char, float);");
+ verifyFormat("int (^foo[kNumEntries])(char, float);");
+ verifyFormat("int (^foo[kNumEntries + 10])(char, float);");
+ verifyFormat("int (^foo[(kNumEntries + 10)])(char, float);");
+}
+
TEST_F(FormatTestObjC, ObjCSnippets) {
verifyFormat("@autoreleasepool {\n"
" foo();\n"
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12128,6 +12128,22 @@
EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "[[foo::bar, ...]]"));
}
+TEST_F(FormatTest, GuessLanguageWithCaret) {
+ EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "FOO(^);"));
+ EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h",
[PATCH] D43906: [clang-format] Improve detection of Objective-C block types
djasper added a comment.
Right. So the difference is that for blocks, there is always a "(" after the
"(^.)". That should be easy to parse, no?
Repository:
rC Clang
https://reviews.llvm.org/D43906
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43906: [clang-format] Improve detection of Objective-C block types
benhamilton added inline comments.
Comment at: lib/Format/TokenAnnotator.cpp:155
+ Next->startsSequence(tok::identifier, tok::l_square,
+tok::numeric_constant, tok::r_square,
+tok::r_paren, tok::l_paren))) {
djasper wrote:
> benhamilton wrote:
> > benhamilton wrote:
> > > djasper wrote:
> > > > benhamilton wrote:
> > > > > djasper wrote:
> > > > > > This seems suspect. Does it have to be a numeric_constant?
> > > > > Probably not, any constexpr would do, I suspect. What's the best way
> > > > > to parse that?
> > > > I think this is the same answer for both of your questions. If what you
> > > > are trying to prevent "FOO(^)" to be parsed as a block, wouldn't it be
> > > > enough to look for whether there is a "(" after the ")" or even only
> > > > after "(^)", everything else is already correct IIUC? That would get
> > > > you out of need to parse the specifics here, which will be hard.
> > > >
> > > > Or thinking about it another way. Previously, every "(^" would be
> > > > parsed as an ObjC block. There seems to be only a really rare corner
> > > > case in which it isn't (macros). Thus, I'd just try to detect that
> > > > corner case. Instead you are completely inverting the defaults
> > > > (defaulting to "^" is not a block) and then try to exactly parse ObjC
> > > > where there might be many cases and edge cases that you won't even
> > > > think of now.
> > > Hmm. Well, it's not just `FOO(^);` that isn't a block:
> > >
> > > ```
> > > #define FOO(X) operator X
> > >
> > > SomeType FOO(^)(int x, const SomeType& y) { ... }
> > > ```
> > >
> > > Obviously we can't get this perfect without a pre-processor, but it seems
> > > like our best bet is to only assign mark `TT_ObjCBlockLParen` when we are
> > > sure the syntax is a valid block type or block variable.
> > I tried the suggestion to only treat `(^)(` as a block type, but it appears
> > this is the primary place where we set `TT_ObjCBlockLParen`, so I think we
> > really do need to handle the other cases here.
> I don't follow your logic. I'd like you to slowly change this as opposed to
> completely going the opposite way.
>
> So currently, the only know real-live problem is "FOO(^);". So address this
> somehow, but still default/error to recognizing too much stuff as a block.
>
> Have you actually seen
>
> SomeType FOO(^)(int x, const SomeType& y) { ... }
>
> in real code?
I haven't seen that example, but I have seen `FOO(^, OtherParam);`.
I'm trying to change the parser to handle this now without explicitly parsing
all block types, but it's tricky. The following should be ObjC block types:
```
(^)(int, char);
(^foo)(int, char);
(^foo[10])(int, char);
(^foo[kNum])(int, char);
(^foo[(kNum + kOtherNum)])(int, char);
```
but the following should not:
```
FOO(^);
FOO(^, int, char);
```
I'll make it work, it's just easy to make a mistake.
Repository:
rC Clang
https://reviews.llvm.org/D43906
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43906: [clang-format] Improve detection of Objective-C block types
djasper added inline comments.
Comment at: lib/Format/TokenAnnotator.cpp:155
+ Next->startsSequence(tok::identifier, tok::l_square,
+tok::numeric_constant, tok::r_square,
+tok::r_paren, tok::l_paren))) {
benhamilton wrote:
> benhamilton wrote:
> > djasper wrote:
> > > benhamilton wrote:
> > > > djasper wrote:
> > > > > This seems suspect. Does it have to be a numeric_constant?
> > > > Probably not, any constexpr would do, I suspect. What's the best way to
> > > > parse that?
> > > I think this is the same answer for both of your questions. If what you
> > > are trying to prevent "FOO(^)" to be parsed as a block, wouldn't it be
> > > enough to look for whether there is a "(" after the ")" or even only
> > > after "(^)", everything else is already correct IIUC? That would get you
> > > out of need to parse the specifics here, which will be hard.
> > >
> > > Or thinking about it another way. Previously, every "(^" would be parsed
> > > as an ObjC block. There seems to be only a really rare corner case in
> > > which it isn't (macros). Thus, I'd just try to detect that corner case.
> > > Instead you are completely inverting the defaults (defaulting to "^" is
> > > not a block) and then try to exactly parse ObjC where there might be many
> > > cases and edge cases that you won't even think of now.
> > Hmm. Well, it's not just `FOO(^);` that isn't a block:
> >
> > ```
> > #define FOO(X) operator X
> >
> > SomeType FOO(^)(int x, const SomeType& y) { ... }
> > ```
> >
> > Obviously we can't get this perfect without a pre-processor, but it seems
> > like our best bet is to only assign mark `TT_ObjCBlockLParen` when we are
> > sure the syntax is a valid block type or block variable.
> I tried the suggestion to only treat `(^)(` as a block type, but it appears
> this is the primary place where we set `TT_ObjCBlockLParen`, so I think we
> really do need to handle the other cases here.
I don't follow your logic. I'd like you to slowly change this as opposed to
completely going the opposite way.
So currently, the only know real-live problem is "FOO(^);". So address this
somehow, but still default/error to recognizing too much stuff as a block.
Have you actually seen
SomeType FOO(^)(int x, const SomeType& y) { ... }
in real code?
Repository:
rC Clang
https://reviews.llvm.org/D43906
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43906: [clang-format] Improve detection of Objective-C block types
benhamilton added inline comments.
Comment at: lib/Format/TokenAnnotator.cpp:155
+ Next->startsSequence(tok::identifier, tok::l_square,
+tok::numeric_constant, tok::r_square,
+tok::r_paren, tok::l_paren))) {
benhamilton wrote:
> djasper wrote:
> > benhamilton wrote:
> > > djasper wrote:
> > > > This seems suspect. Does it have to be a numeric_constant?
> > > Probably not, any constexpr would do, I suspect. What's the best way to
> > > parse that?
> > I think this is the same answer for both of your questions. If what you are
> > trying to prevent "FOO(^)" to be parsed as a block, wouldn't it be enough
> > to look for whether there is a "(" after the ")" or even only after "(^)",
> > everything else is already correct IIUC? That would get you out of need to
> > parse the specifics here, which will be hard.
> >
> > Or thinking about it another way. Previously, every "(^" would be parsed as
> > an ObjC block. There seems to be only a really rare corner case in which it
> > isn't (macros). Thus, I'd just try to detect that corner case. Instead you
> > are completely inverting the defaults (defaulting to "^" is not a block)
> > and then try to exactly parse ObjC where there might be many cases and edge
> > cases that you won't even think of now.
> Hmm. Well, it's not just `FOO(^);` that isn't a block:
>
> ```
> #define FOO(X) operator X
>
> SomeType FOO(^)(int x, const SomeType& y) { ... }
> ```
>
> Obviously we can't get this perfect without a pre-processor, but it seems
> like our best bet is to only assign mark `TT_ObjCBlockLParen` when we are
> sure the syntax is a valid block type or block variable.
I tried the suggestion to only treat `(^)(` as a block type, but it appears
this is the primary place where we set `TT_ObjCBlockLParen`, so I think we
really do need to handle the other cases here.
Repository:
rC Clang
https://reviews.llvm.org/D43906
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43906: [clang-format] Improve detection of Objective-C block types
benhamilton added inline comments.
Comment at: lib/Format/TokenAnnotator.cpp:155
+ Next->startsSequence(tok::identifier, tok::l_square,
+tok::numeric_constant, tok::r_square,
+tok::r_paren, tok::l_paren))) {
djasper wrote:
> benhamilton wrote:
> > djasper wrote:
> > > This seems suspect. Does it have to be a numeric_constant?
> > Probably not, any constexpr would do, I suspect. What's the best way to
> > parse that?
> I think this is the same answer for both of your questions. If what you are
> trying to prevent "FOO(^)" to be parsed as a block, wouldn't it be enough to
> look for whether there is a "(" after the ")" or even only after "(^)",
> everything else is already correct IIUC? That would get you out of need to
> parse the specifics here, which will be hard.
>
> Or thinking about it another way. Previously, every "(^" would be parsed as
> an ObjC block. There seems to be only a really rare corner case in which it
> isn't (macros). Thus, I'd just try to detect that corner case. Instead you
> are completely inverting the defaults (defaulting to "^" is not a block) and
> then try to exactly parse ObjC where there might be many cases and edge cases
> that you won't even think of now.
Hmm. Well, it's not just `FOO(^);` that isn't a block:
```
#define FOO(X) operator X
SomeType FOO(^)(int x, const SomeType& y) { ... }
```
Obviously we can't get this perfect without a pre-processor, but it seems like
our best bet is to only assign mark `TT_ObjCBlockLParen` when we are sure the
syntax is a valid block type or block variable.
Repository:
rC Clang
https://reviews.llvm.org/D43906
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43906: [clang-format] Improve detection of Objective-C block types
djasper added inline comments.
Comment at: lib/Format/TokenAnnotator.cpp:155
+ Next->startsSequence(tok::identifier, tok::l_square,
+tok::numeric_constant, tok::r_square,
+tok::r_paren, tok::l_paren))) {
benhamilton wrote:
> djasper wrote:
> > This seems suspect. Does it have to be a numeric_constant?
> Probably not, any constexpr would do, I suspect. What's the best way to parse
> that?
I think this is the same answer for both of your questions. If what you are
trying to prevent "FOO(^)" to be parsed as a block, wouldn't it be enough to
look for whether there is a "(" after the ")" or even only after "(^)",
everything else is already correct IIUC? That would get you out of need to
parse the specifics here, which will be hard.
Or thinking about it another way. Previously, every "(^" would be parsed as an
ObjC block. There seems to be only a really rare corner case in which it isn't
(macros). Thus, I'd just try to detect that corner case. Instead you are
completely inverting the defaults (defaulting to "^" is not a block) and then
try to exactly parse ObjC where there might be many cases and edge cases that
you won't even think of now.
Repository:
rC Clang
https://reviews.llvm.org/D43906
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43906: [clang-format] Improve detection of Objective-C block types
krasimir added inline comments.
Comment at: unittests/Format/FormatTest.cpp:7698
+TEST_F(FormatTest, ObjCBlockTypesAndVariables) {
+ verifyFormat("void DoStuffWithBlockType(int (^)(char));");
This should be in `FormatTestObjC.cpp`.
Repository:
rC Clang
https://reviews.llvm.org/D43906
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43906: [clang-format] Improve detection of Objective-C block types
benhamilton added a comment.
> Would it be enough to only add the block type case? With the macro false
> positive, there won't be an open paren after the closing paren, right?
Are you asking to remove the block variable cases? I was concerned those would
no longer be handled correctly if we don't explicitly check for them.
What change are you suggesting if we remove the block variable cases to handle
those?
Comment at: lib/Format/TokenAnnotator.cpp:155
+ Next->startsSequence(tok::identifier, tok::l_square,
+tok::numeric_constant, tok::r_square,
+tok::r_paren, tok::l_paren))) {
djasper wrote:
> This seems suspect. Does it have to be a numeric_constant?
Probably not, any constexpr would do, I suspect. What's the best way to parse
that?
Comment at: unittests/Format/FormatTest.cpp:12027
+TEST_F(FormatTest, GuessLanguageWithCaret) {
+ EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "FOO(^);"));
krasimir wrote:
> Please also add formatting tests. This might require changes to the
> formatting logic since I'd intuitively expect ``int(^)(char)`` to be
> formatted as ``int (^)(char)``.
Added formatting tests. Tests passed with no changes to the formatting logic,
and your intuition was correct.
Comment at: unittests/Format/FormatTest.cpp:12029
+ EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "FOO(^);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^)(char, float);"));
djasper wrote:
> I am somewhat skeptical about all these tests now being solely about
> guessLanguage. It might be the best choice for some of them, but also, there
> might be other things we do to detect ObjC at some point and then all of
> these tests become meaningless.
>
> Does your change create a different formatting here?
I hear you. I just want to make sure the false positive case doesn't regress,
and we don't lose the functionality of detecting ObjC for block types.
Added formatting tests.
Repository:
rC Clang
https://reviews.llvm.org/D43906
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43906: [clang-format] Improve detection of Objective-C block types
benhamilton updated this revision to Diff 137095.
benhamilton marked 2 inline comments as done.
benhamilton added a comment.
- Fix comments from @djasper and @krasimir.
Repository:
rC Clang
https://reviews.llvm.org/D43906
Files:
lib/Format/TokenAnnotator.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -7695,6 +7695,11 @@
verifyGoogleFormat("- foo:(int)foo;");
}
+TEST_F(FormatTest, ObjCBlockTypesAndVariables) {
+ verifyFormat("void DoStuffWithBlockType(int (^)(char));");
+ verifyFormat("int (^foo)(char, float);");
+ verifyFormat("int (^foo[10])(char, float);");
+}
TEST_F(FormatTest, BreaksStringLiterals) {
EXPECT_EQ("\"some text \"\n"
@@ -12159,6 +12164,16 @@
"for (const Foo& baz = in.value(); !baz.at_end(); ++baz) {}"));
}
+TEST_F(FormatTest, GuessLanguageWithCaret) {
+ EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "FOO(^);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^)(char, float);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^foo)(char, float);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^foo[10])(char, float);"));
+}
+
} // end namespace
} // end namespace format
} // end namespace clang
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -142,8 +142,20 @@
bool StartsObjCMethodExpr = false;
if (CurrentToken->is(tok::caret)) {
- // (^ can start a block type.
- Left->Type = TT_ObjCBlockLParen;
+ const FormatToken *Next = CurrentToken->getNextNonComment();
+ if (Next &&
+ // int (^)(char, float)
+ (Next->startsSequence(tok::r_paren, tok::l_paren) ||
+ // int (^blockReturningIntWithCharAndFloatArguments)(char, float)
+ Next->startsSequence(tok::identifier, tok::r_paren, tok::l_paren) ||
+ // int
+ //
(^arrayOfTenBlocksReturningIntWithCharAndFloatArguments[10])(char,
+ // float)
+ Next->startsSequence(tok::identifier, tok::l_square,
+tok::numeric_constant, tok::r_square,
+tok::r_paren, tok::l_paren))) {
+Left->Type = TT_ObjCBlockLParen;
+ }
} else if (FormatToken *MaybeSel = Left->Previous) {
// @selector( starts a selector.
if (MaybeSel->isObjCAtKeyword(tok::objc_selector) && MaybeSel->Previous
&&
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -7695,6 +7695,11 @@
verifyGoogleFormat("- foo:(int)foo;");
}
+TEST_F(FormatTest, ObjCBlockTypesAndVariables) {
+ verifyFormat("void DoStuffWithBlockType(int (^)(char));");
+ verifyFormat("int (^foo)(char, float);");
+ verifyFormat("int (^foo[10])(char, float);");
+}
TEST_F(FormatTest, BreaksStringLiterals) {
EXPECT_EQ("\"some text \"\n"
@@ -12159,6 +12164,16 @@
"for (const Foo& baz = in.value(); !baz.at_end(); ++baz) {}"));
}
+TEST_F(FormatTest, GuessLanguageWithCaret) {
+ EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "FOO(^);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^)(char, float);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^foo)(char, float);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^foo[10])(char, float);"));
+}
+
} // end namespace
} // end namespace format
} // end namespace clang
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -142,8 +142,20 @@
bool StartsObjCMethodExpr = false;
if (CurrentToken->is(tok::caret)) {
- // (^ can start a block type.
- Left->Type = TT_ObjCBlockLParen;
+ const FormatToken *Next = CurrentToken->getNextNonComment();
+ if (Next &&
+ // int (^)(char, float)
+ (Next->startsSequence(tok::r_paren, tok::l_paren) ||
+ // int (^blockReturningIntWithCharAndFloatArguments)(char, float)
+ Next->startsSequence(tok::identifier, tok::r_paren, tok::l_paren) ||
+ // int
+ // (^arrayOfTenBlocksReturningIntWithCharAndFloatArguments[10])(char,
+ // float)
+ Next->startsSequence(tok::identifier, tok::l_square,
+tok::numeric_constant, tok::r_square,
+tok::r_paren, tok::l_paren))) {
+Left->Type = TT_ObjCBlockLParen;
+ }
} else if (FormatToken *MaybeSel = Left->Previous) {
// @selector( starts a
[PATCH] D43906: [clang-format] Improve detection of Objective-C block types
djasper added a comment.
Would it be enough to only add the block type case? With the macro false
positive, there won't be an open paren after the closing paren, right?
Comment at: lib/Format/TokenAnnotator.cpp:155
+ Next->startsSequence(tok::identifier, tok::l_square,
+tok::numeric_constant, tok::r_square,
+tok::r_paren, tok::l_paren))) {
This seems suspect. Does it have to be a numeric_constant?
Comment at: unittests/Format/FormatTest.cpp:12029
+ EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "FOO(^);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^)(char, float);"));
I am somewhat skeptical about all these tests now being solely about
guessLanguage. It might be the best choice for some of them, but also, there
might be other things we do to detect ObjC at some point and then all of these
tests become meaningless.
Does your change create a different formatting here?
Repository:
rC Clang
https://reviews.llvm.org/D43906
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43906: [clang-format] Improve detection of Objective-C block types
krasimir added inline comments.
Comment at: unittests/Format/FormatTest.cpp:12027
+TEST_F(FormatTest, GuessLanguageWithCaret) {
+ EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "FOO(^);"));
Please also add formatting tests. This might require changes to the formatting
logic since I'd intuitively expect ``int(^)(char)`` to be formatted as ``int
(^)(char)``.
Repository:
rC Clang
https://reviews.llvm.org/D43906
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43906: [clang-format] Improve detection of Objective-C block types
benhamilton created this revision.
benhamilton added reviewers: krasimir, jolesiak.
Herald added a subscriber: cfe-commits.
Previously, clang-format would detect the following as an
Objective-C block type:
FOO(^);
when it actually must be a C or C++ macro dealing with an XOR
statement or an XOR operator overload.
According to the Clang Block Language Spec:
https://clang.llvm.org/docs/BlockLanguageSpec.html
block types are of the form:
int (^)(char, float)
and block variables of block type are of the form:
void (^blockReturningVoidWithVoidArgument)(void);
int (^blockReturningIntWithIntAndCharArguments)(int, char);
void (^arrayOfTenBlocksReturningVoidWithIntArgument[10])(int);
This tightens up the detection so we don't unnecessarily detect
C macros which pass in the XOR operator.
Depends On https://reviews.llvm.org/D43904
Test Plan: New tests added. Ran tests with:
make -j12 FormatTests &&
./tools/clang/unittests/Format/FormatTests
Repository:
rC Clang
https://reviews.llvm.org/D43906
Files:
lib/Format/TokenAnnotator.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12024,6 +12024,16 @@
"for (const Foo& baz = in.value(); !baz.at_end(); ++baz) {}"));
}
+TEST_F(FormatTest, GuessLanguageWithCaret) {
+ EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "FOO(^);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^)(char, float);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^foo)(char, float);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^foo[10])(char, float);"));
+}
+
} // end namespace
} // end namespace format
} // end namespace clang
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -142,8 +142,20 @@
bool StartsObjCMethodExpr = false;
if (CurrentToken->is(tok::caret)) {
- // (^ can start a block type.
- Left->Type = TT_ObjCBlockLParen;
+ const FormatToken *Next = CurrentToken->getNextNonComment();
+ if (Next &&
+ // int (^)(char, float)
+ (Next->startsSequence(tok::r_paren, tok::l_paren) ||
+ // int (^blockReturningIntWithCharAndFloatArguments)(char, float)
+ Next->startsSequence(tok::identifier, tok::r_paren, tok::l_paren) ||
+ // int
+ //
(^arrayOfTenBlocksReturningIntWithCharAndFloatArguments[10])(char,
+ // float)
+ Next->startsSequence(tok::identifier, tok::l_square,
+tok::numeric_constant, tok::r_square,
+tok::r_paren, tok::l_paren))) {
+Left->Type = TT_ObjCBlockLParen;
+ }
} else if (FormatToken *MaybeSel = Left->Previous) {
// @selector( starts a selector.
if (MaybeSel->isObjCAtKeyword(tok::objc_selector) && MaybeSel->Previous
&&
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12024,6 +12024,16 @@
"for (const Foo& baz = in.value(); !baz.at_end(); ++baz) {}"));
}
+TEST_F(FormatTest, GuessLanguageWithCaret) {
+ EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "FOO(^);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^)(char, float);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^foo)(char, float);"));
+ EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "int(^foo[10])(char, float);"));
+}
+
} // end namespace
} // end namespace format
} // end namespace clang
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -142,8 +142,20 @@
bool StartsObjCMethodExpr = false;
if (CurrentToken->is(tok::caret)) {
- // (^ can start a block type.
- Left->Type = TT_ObjCBlockLParen;
+ const FormatToken *Next = CurrentToken->getNextNonComment();
+ if (Next &&
+ // int (^)(char, float)
+ (Next->startsSequence(tok::r_paren, tok::l_paren) ||
+ // int (^blockReturningIntWithCharAndFloatArguments)(char, float)
+ Next->startsSequence(tok::identifier, tok::r_paren, tok::l_paren) ||
+ // int
+ // (^arrayOfTenBlocksReturningIntWithCharAndFloatArguments[10])(char,
+ // float)
+ Next->startsSequence(tok::identifier, tok::l_square,
+tok::numeric_constant, tok::r_square,
+tok::r_paren, tok::l_paren))) {
+Left->Type = TT_ObjCBlockLParen;
+ }
