[PATCH] Use ArrayRef in SemaInit.cpp.

John McCall rjmccall at apple.com
Tue Apr 30 13:53:19 PDT 2013


On Apr 30, 2013, at 1:42 PM, Jordan Rose <jordan_rose at apple.com> wrote:
> On Apr 30, 2013, at 12:45 , Sean Silva <silvas at purdue.edu> wrote:
>> On Tue, Apr 30, 2013 at 1:55 PM, David Blaikie <dblaikie at gmail.com> wrote:
>> * should we consider adding an ArrayRef implicit ctor from "None"?
>> (see llvm/ADT/None.h and its use in llvm/ADT/Optional.h) Then you
>> could replace all the "ArrayRef<T>()" calls with "None" (you might
>> even want to do a sed replace on existing instances of this as a
>> separate patch (a purely mechanical patch is easy to review/apply)).
>> 
>> What's wrong with `ArrayRef<T>()`? It's explicit and clear. What is the benefit of writing `None` instead? IMHO if I saw "None" that would just confuse me and send me on a wild goose chase that would eventually terminate on finding the implicit ArrayRef ctor and then saying to myself "why the heck didn't they just write `ArrayRef<T>`"?
> 
> I like None. In argument lists with possibly empty arrays, specifying the empty array by type is weird and brittle. Normally, in an argument list you have values:
> 
> invalidate(ConstArgs, NonConstArgs);
> 
> and if one of those arrays is empty, you start mixing values and types when you're reading:
> 
> invalidate(ArrayRef<SVal>(), NonConstArgs);
> 
> Not to mention that if you decide to change the type of this thing, and it's threaded through multiple functions, you'd normally only have to change the function headers...but now you have to change some (but not all) of the call sites as well:
> 
> invalidate(ArrayRef<const MemRegion *>(), NonConstArgs);
> 
> even though that particular caller is still semantically doing the same thing:
> 
> invalidate(None, NonConstArgs);
> 
> It's really the same argument as "why not Optional<T>"? I personally find None much nicer than Optional<T>.

I agree;  we should be able to construct an ArrayRef with None.

It'd be nice if whoever made this change also added a C++11-only std::initializer_list constructor while they're at it.

John.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130430/a3cc6367/attachment.html>


More information about the cfe-commits mailing list