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
