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
