[PATCH][review request] Use ArrayRef in SemaInit.cpp. Try 2

[Robert Wilhelm]

Hello David,

 This patch switches to ArrayRef in SemaInit.cpp.

 It should address all comments except the  ArrayRef implicit ctor from
"None" which as addressed by still unreviewed
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20130429/078934.html

 No functionality change.

Please check.

 Thanks,
	 Robert

On Tue, 2013-04-30 at 10:55 -0700, David Blaikie wrote:
> A few comments:
> 
> * use "empty()" instead of "size() == 0"
> * use "foo" instead of "ArrayRef<T>(&foo, 1)" (ArrayRef has an
> implicit ctor from T)
> * 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)).
> You could, maybe, even remove the default ctor for ArrayRef entirely
> (though there may be cases where we still want/need it to resolve
> ambiguities, I'm not sure - but temporarily removing it would help you
> track down all the places that could use "None" instead). This may
> need some consideration as to whether it improves or hinders
> readability.
> 
> -      Expr **Args = InitList ? &InitListAsExpr : 0;
> -      unsigned NumArgs = InitList ? 1 : 0;
> +  ArrayRef <Expr *> Args ( InitList ? &InitListAsExpr : 0 , InitList ? 1 : 0);
> 
> You can drop the conditional in the first parameter:
> 
> ArrayRef <Expr *> Args (&InitListAsExpr, InitList ? 1 : 0);
> 
> Or possibly write this as:
> 
> ArrayRef<Expr *> Args = InitList ? InitListAsExpr : None;
> 
> If you take up the "None" suggestion above. That might then be simple
> enough to skip the local variable & just use the expression directly
> in the TryConstructorInitialization call below.
> 
> * in some cases you have "ArrayRef<Expr *>(x, y)" or similar which
> could be replaced with "makeArrayRef(x, y)" & avoid having to name the
> Expr (or whatever other element) type
> *
> 
> 
> 
> On Tue, Apr 30, 2013 at 5:23 AM, Robert Wilhelm <robert.wilhelm at gmx.net> wrote:
> >
> > This patch switches to ArrayRef in SemaInit.cpp.
> > No functionality change.
> > Passes make test on x86_64-unknown-linux-gnu
> >
> > Please commit.
> >
> > Thanks,
> > Robert
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> >

-------------- next part --------------
A non-text attachment was scrubbed...
Name: SemaInit_ArrayRef4.patch
Type: text/x-patch
Size: 38719 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130503/c7749a53/attachment.bin>