[PATCH] ArrayRef ctor from None

Robert Wilhelm robert.wilhelm at gmx.net
Wed May 1 12:48:18 PDT 2013


Thanks for the the hints. I really appreciate them.

I have added ArrayRef ctor from "None" and replaced most  ArrayRef<T>()
calls with "None". Attached are two patches, one for llvm and one for
clang. I had to change two unrelated enums because of scope.

Please review.

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: ArrayRef_None.patch
Type: text/x-patch
Size: 10899 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130501/d357138d/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ArrayRef_None_clang.patch
Type: text/x-patch
Size: 18632 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130501/d357138d/attachment-0001.bin>


More information about the cfe-commits mailing list