[PATCH] D44632: [clang-format] Add a few more Core Graphics identifiers to ObjC heuristic
This revision was automatically updated to reflect the committed changes.
Closed by commit rL328175: [clang-format] Add a few more Core Graphics
identifiers to ObjC heuristic (authored by benhamilton, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D44632
Files:
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -1449,6 +1449,19 @@
// Keep this array sorted, since we are binary searching over it.
static constexpr llvm::StringLiteral FoundationIdentifiers[] = {
"CGFloat",
+"CGPoint",
+"CGPointMake",
+"CGPointZero",
+"CGRect",
+"CGRectEdge",
+"CGRectInfinite",
+"CGRectMake",
+"CGRectNull",
+"CGRectZero",
+"CGSize",
+"CGSizeMake",
+"CGVector",
+"CGVectorMake",
"NSAffineTransform",
"NSArray",
"NSAttributedString",
@@ -1497,6 +1510,8 @@
"NSURLQueryItem",
"NSUUID",
"NSValue",
+"UIImage",
+"UIView",
};
for (auto : AnnotatedLines) {
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -12108,6 +12108,12 @@
EXPECT_EQ(FormatStyle::LK_ObjC, guessLanguage("foo.h", "@interface
Foo\n@end\n"));
EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo", ""));
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(
+ FormatStyle::LK_ObjC,
+ guessLanguage("foo.h",
+"#define MY_POINT_MAKE(x, y) CGPointMake((x), (y));\n"));
}
TEST_F(FormatTest, GuessLanguageWithCpp11AttributeSpecifiers) {
Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -1449,6 +1449,19 @@
// Keep this array sorted, since we are binary searching over it.
static constexpr llvm::StringLiteral FoundationIdentifiers[] = {
"CGFloat",
+"CGPoint",
+"CGPointMake",
+"CGPointZero",
+"CGRect",
+"CGRectEdge",
+"CGRectInfinite",
+"CGRectMake",
+"CGRectNull",
+"CGRectZero",
+"CGSize",
+"CGSizeMake",
+"CGVector",
+"CGVectorMake",
"NSAffineTransform",
"NSArray",
"NSAttributedString",
@@ -1497,6 +1510,8 @@
"NSURLQueryItem",
"NSUUID",
"NSValue",
+"UIImage",
+"UIView",
};
for (auto : AnnotatedLines) {
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -12108,6 +12108,12 @@
EXPECT_EQ(FormatStyle::LK_ObjC, guessLanguage("foo.h", "@interface Foo\n@end\n"));
EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo", ""));
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(
+ FormatStyle::LK_ObjC,
+ guessLanguage("foo.h",
+"#define MY_POINT_MAKE(x, y) CGPointMake((x), (y));\n"));
}
TEST_F(FormatTest, GuessLanguageWithCpp11AttributeSpecifiers) {
___
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
benhamilton updated this revision to Diff 139410.
benhamilton marked an inline comment as done.
benhamilton added a comment.
Remove assert
Repository:
rC Clang
https://reviews.llvm.org/D44632
Files:
lib/Format/Format.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12108,6 +12108,12 @@
EXPECT_EQ(FormatStyle::LK_ObjC, guessLanguage("foo.h", "@interface
Foo\n@end\n"));
EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo", ""));
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(
+ FormatStyle::LK_ObjC,
+ guessLanguage("foo.h",
+"#define MY_POINT_MAKE(x, y) CGPointMake((x), (y));\n"));
}
TEST_F(FormatTest, GuessLanguageWithCpp11AttributeSpecifiers) {
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1449,6 +1449,19 @@
// Keep this array sorted, since we are binary searching over it.
static constexpr llvm::StringLiteral FoundationIdentifiers[] = {
"CGFloat",
+"CGPoint",
+"CGPointMake",
+"CGPointZero",
+"CGRect",
+"CGRectEdge",
+"CGRectInfinite",
+"CGRectMake",
+"CGRectNull",
+"CGRectZero",
+"CGSize",
+"CGSizeMake",
+"CGVector",
+"CGVectorMake",
"NSAffineTransform",
"NSArray",
"NSAttributedString",
@@ -1497,6 +1510,8 @@
"NSURLQueryItem",
"NSUUID",
"NSValue",
+"UIImage",
+"UIView",
};
for (auto : AnnotatedLines) {
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12108,6 +12108,12 @@
EXPECT_EQ(FormatStyle::LK_ObjC, guessLanguage("foo.h", "@interface Foo\n@end\n"));
EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo", ""));
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(
+ FormatStyle::LK_ObjC,
+ guessLanguage("foo.h",
+"#define MY_POINT_MAKE(x, y) CGPointMake((x), (y));\n"));
}
TEST_F(FormatTest, GuessLanguageWithCpp11AttributeSpecifiers) {
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1449,6 +1449,19 @@
// Keep this array sorted, since we are binary searching over it.
static constexpr llvm::StringLiteral FoundationIdentifiers[] = {
"CGFloat",
+"CGPoint",
+"CGPointMake",
+"CGPointZero",
+"CGRect",
+"CGRectEdge",
+"CGRectInfinite",
+"CGRectMake",
+"CGRectNull",
+"CGRectZero",
+"CGSize",
+"CGSizeMake",
+"CGVector",
+"CGVectorMake",
"NSAffineTransform",
"NSArray",
"NSAttributedString",
@@ -1497,6 +1510,8 @@
"NSURLQueryItem",
"NSUUID",
"NSValue",
+"UIImage",
+"UIView",
};
for (auto : AnnotatedLines) {
___
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
djasper accepted this revision. djasper added a comment. Looks good. 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",
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] D44632: [clang-format] Add a few more Core Graphics identifiers to ObjC heuristic
krasimir 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",
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.
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] D44632: [clang-format] Add a few more Core Graphics identifiers to ObjC heuristic
benhamilton marked 4 inline comments as done.
benhamilton 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:
> 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.
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
benhamilton updated this revision to Diff 139136.
benhamilton added a comment.
Add assert(std::is_sorted(...)). (We can't static_assert on is_sorted until
C++2x.)
Repository:
rC Clang
https://reviews.llvm.org/D44632
Files:
lib/Format/Format.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12108,6 +12108,12 @@
EXPECT_EQ(FormatStyle::LK_ObjC, guessLanguage("foo.h", "@interface
Foo\n@end\n"));
EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo", ""));
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(
+ FormatStyle::LK_ObjC,
+ guessLanguage("foo.h",
+"#define MY_POINT_MAKE(x, y) CGPointMake((x), (y));\n"));
}
TEST_F(FormatTest, GuessLanguageWithCpp11AttributeSpecifiers) {
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1449,6 +1449,19 @@
// Keep this array sorted, since we are binary searching over it.
static constexpr llvm::StringLiteral FoundationIdentifiers[] = {
"CGFloat",
+"CGPoint",
+"CGPointMake",
+"CGPointZero",
+"CGRect",
+"CGRectEdge",
+"CGRectInfinite",
+"CGRectMake",
+"CGRectNull",
+"CGRectZero",
+"CGSize",
+"CGSizeMake",
+"CGVector",
+"CGVectorMake",
"NSAffineTransform",
"NSArray",
"NSAttributedString",
@@ -1497,7 +1510,11 @@
"NSURLQueryItem",
"NSUUID",
"NSValue",
+"UIImage",
+"UIView",
};
+assert(std::is_sorted(std::begin(FoundationIdentifiers),
+ std::end(FoundationIdentifiers)));
for (auto : AnnotatedLines) {
for (FormatToken *FormatTok = Line->First; FormatTok;
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12108,6 +12108,12 @@
EXPECT_EQ(FormatStyle::LK_ObjC, guessLanguage("foo.h", "@interface Foo\n@end\n"));
EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo", ""));
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(
+ FormatStyle::LK_ObjC,
+ guessLanguage("foo.h",
+"#define MY_POINT_MAKE(x, y) CGPointMake((x), (y));\n"));
}
TEST_F(FormatTest, GuessLanguageWithCpp11AttributeSpecifiers) {
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1449,6 +1449,19 @@
// Keep this array sorted, since we are binary searching over it.
static constexpr llvm::StringLiteral FoundationIdentifiers[] = {
"CGFloat",
+"CGPoint",
+"CGPointMake",
+"CGPointZero",
+"CGRect",
+"CGRectEdge",
+"CGRectInfinite",
+"CGRectMake",
+"CGRectNull",
+"CGRectZero",
+"CGSize",
+"CGSizeMake",
+"CGVector",
+"CGVectorMake",
"NSAffineTransform",
"NSArray",
"NSAttributedString",
@@ -1497,7 +1510,11 @@
"NSURLQueryItem",
"NSUUID",
"NSValue",
+"UIImage",
+"UIView",
};
+assert(std::is_sorted(std::begin(FoundationIdentifiers),
+ std::end(FoundationIdentifiers)));
for (auto : AnnotatedLines) {
for (FormatToken *FormatTok = Line->First; FormatTok;
___
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
djasper 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",
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.
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
benhamilton added a subscriber: krasimir.
benhamilton added a comment.
@krasimir, can you answer @djasper's question about hash set vs. binary search?
Comment at: lib/Format/Format.cpp:1450
// Keep this array sorted, since we are binary searching over it.
static constexpr llvm::StringLiteral FoundationIdentifiers[] = {
"CGFloat",
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/
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(
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?)
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
benhamilton updated this revision to Diff 139125.
benhamilton marked 2 inline comments as done.
benhamilton added a comment.
clang-format
Repository:
rC Clang
https://reviews.llvm.org/D44632
Files:
lib/Format/Format.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12108,6 +12108,12 @@
EXPECT_EQ(FormatStyle::LK_ObjC, guessLanguage("foo.h", "@interface
Foo\n@end\n"));
EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo", ""));
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(
+ FormatStyle::LK_ObjC,
+ guessLanguage("foo.h",
+"#define MY_POINT_MAKE(x, y) CGPointMake((x), (y));\n"));
}
TEST_F(FormatTest, GuessLanguageWithCpp11AttributeSpecifiers) {
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1449,6 +1449,19 @@
// Keep this array sorted, since we are binary searching over it.
static constexpr llvm::StringLiteral FoundationIdentifiers[] = {
"CGFloat",
+"CGPoint",
+"CGPointMake",
+"CGPointZero",
+"CGRect",
+"CGRectEdge",
+"CGRectInfinite",
+"CGRectMake",
+"CGRectNull",
+"CGRectZero",
+"CGSize",
+"CGSizeMake",
+"CGVector",
+"CGVectorMake",
"NSAffineTransform",
"NSArray",
"NSAttributedString",
@@ -1497,6 +1510,8 @@
"NSURLQueryItem",
"NSUUID",
"NSValue",
+"UIImage",
+"UIView",
};
for (auto : AnnotatedLines) {
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12108,6 +12108,12 @@
EXPECT_EQ(FormatStyle::LK_ObjC, guessLanguage("foo.h", "@interface Foo\n@end\n"));
EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo", ""));
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(
+ FormatStyle::LK_ObjC,
+ guessLanguage("foo.h",
+"#define MY_POINT_MAKE(x, y) CGPointMake((x), (y));\n"));
}
TEST_F(FormatTest, GuessLanguageWithCpp11AttributeSpecifiers) {
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1449,6 +1449,19 @@
// Keep this array sorted, since we are binary searching over it.
static constexpr llvm::StringLiteral FoundationIdentifiers[] = {
"CGFloat",
+"CGPoint",
+"CGPointMake",
+"CGPointZero",
+"CGRect",
+"CGRectEdge",
+"CGRectInfinite",
+"CGRectMake",
+"CGRectNull",
+"CGRectZero",
+"CGSize",
+"CGSizeMake",
+"CGVector",
+"CGVectorMake",
"NSAffineTransform",
"NSArray",
"NSAttributedString",
@@ -1497,6 +1510,8 @@
"NSURLQueryItem",
"NSUUID",
"NSValue",
+"UIImage",
+"UIView",
};
for (auto : AnnotatedLines) {
___
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
djasper 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",
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?
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(
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.
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 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
benhamilton created this revision.
benhamilton added reviewers: jolesiak, djasper.
Herald added subscribers: cfe-commits, klimek.
We received reports of the Objective-C style guesser getting a false
negative on header files like:
CGSize SizeOfThing(MyThing thing);
This adds more Core Graphics identifiers to the Objective-C style
guesser.
Test Plan: New tests added. Ran tests with:
% make -j12 FormatTests && ./tools/clang/unittests/Format/FormatTests
Repository:
rC Clang
https://reviews.llvm.org/D44632
Files:
lib/Format/Format.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12096,6 +12096,11 @@
EXPECT_EQ(FormatStyle::LK_ObjC, guessLanguage("foo.h", "@interface
Foo\n@end\n"));
EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo", ""));
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(
+ FormatStyle::LK_ObjC,
+ guessLanguage("foo.h",
+"#define MY_POINT_MAKE(x, y) CGPointMake((x), (y));\n"));
}
TEST_F(FormatTest, GuessLanguageWithCpp11AttributeSpecifiers) {
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1449,6 +1449,19 @@
// Keep this array sorted, since we are binary searching over it.
static constexpr llvm::StringLiteral FoundationIdentifiers[] = {
"CGFloat",
+"CGPoint",
+"CGPointMake",
+"CGPointZero",
+"CGRect",
+"CGRectEdge",
+"CGRectInfinite",
+"CGRectMake",
+"CGRectNull",
+"CGRectZero",
+"CGSize",
+"CGSizeMake",
+"CGVector",
+"CGVectorMake",
"NSAffineTransform",
"NSArray",
"NSAttributedString",
@@ -1497,6 +1510,8 @@
"NSURLQueryItem",
"NSUUID",
"NSValue",
+"UIImage",
+"UIView",
};
for (auto : AnnotatedLines) {
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12096,6 +12096,11 @@
EXPECT_EQ(FormatStyle::LK_ObjC, guessLanguage("foo.h", "@interface Foo\n@end\n"));
EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo", ""));
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(
+ FormatStyle::LK_ObjC,
+ guessLanguage("foo.h",
+"#define MY_POINT_MAKE(x, y) CGPointMake((x), (y));\n"));
}
TEST_F(FormatTest, GuessLanguageWithCpp11AttributeSpecifiers) {
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1449,6 +1449,19 @@
// Keep this array sorted, since we are binary searching over it.
static constexpr llvm::StringLiteral FoundationIdentifiers[] = {
"CGFloat",
+"CGPoint",
+"CGPointMake",
+"CGPointZero",
+"CGRect",
+"CGRectEdge",
+"CGRectInfinite",
+"CGRectMake",
+"CGRectNull",
+"CGRectZero",
+"CGSize",
+"CGSizeMake",
+"CGVector",
+"CGVectorMake",
"NSAffineTransform",
"NSArray",
"NSAttributedString",
@@ -1497,6 +1510,8 @@
"NSURLQueryItem",
"NSUUID",
"NSValue",
+"UIImage",
+"UIView",
};
for (auto : AnnotatedLines) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
