[PATCH] D44632: [clang-format] Add a few more Core Graphics identifiers to ObjC heuristic

[Ben Hamilton via Phabricator via cfe-commits]

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