On Thu, 5 Jul 2007, David Greene wrote:>>> We should just keep the existing constructor, so this isn't a problem. >>> These clients don't have the "dereference end" problem. >> >> Duh. >> >> Yep, ok, this sounds good. > > Grr. That's no so easy either, because it conflicts with the one-argument > specialized constructor. > > Either I have to write separate constructors for std::vector<ValType> and > SmallVector<ValType, Size> or I take an alternate approach. > > I've opted for the alternate approach: make all clients pass iterators. That > is, get rid of the specialized one- and two-argument constructors entirely. > > This gets rid of all of these headaches while at the same time reducing code > size and maintenance costs. It does require that clients store their > arguments in an array of some sort, but it's probably not a bad tradeoff. > It at least makes everything consistent.That sounds reasonable. Please keep the "Value* + num elements" ctor as well though. Here's another crazy idea. Instead of overloading the callinst ctor, why not just add a static function to create the call (like binaryoperator and friends): C = CallInst::CreateWithArgRange(X, B, V.begin(), V.end()); That way you can completely avoid the overloading issues. -Chris -- http://nondot.org/sabre/ http://llvm.org/
On Thursday 05 July 2007 11:54, Chris Lattner wrote:> > I've opted for the alternate approach: make all clients pass iterators. > > That is, get rid of the specialized one- and two-argument constructors > > entirely.> That sounds reasonable. Please keep the "Value* + num elements" ctor as > well though.Why? I can't think of a case where a client uses that but can't use the iterator version (just pass Value, Value+num). I'd rather not maintain two interfaces with identical semantics.> Here's another crazy idea. Instead of overloading the callinst ctor, why > not just add a static function to create the call (like binaryoperator and > friends): > > C = CallInst::CreateWithArgRange(X, B, V.begin(), V.end()); > > That way you can completely avoid the overloading issues.That's not a bad idea. In fact, we ought to think about creating factories for everything. Bare new/delete usually comes back to haunt one later on. I'll get what I've got working and checked in. I'm not sure I'm ready to go and change everything to CallInst factories given the tons of other things I've got to work on. The iterator and one-argument constructors appear to cover 99% of the cases and the two-argument clients can often just pass Use::op_iterators directly. I've even found one case where we can eliminate a temporary vector. -Dave
On Thursday 05 July 2007 11:54, Chris Lattner wrote:> Here's another crazy idea. Instead of overloading the callinst ctor, why > not just add a static function to create the call (like binaryoperator and > friends): > > C = CallInst::CreateWithArgRange(X, B, V.begin(), V.end()); > > That way you can completely avoid the overloading issues.What's the relationship of this idea to LLVMBuilder? -Dave
On Thu, 5 Jul 2007, David Greene wrote:> On Thursday 05 July 2007 11:54, Chris Lattner wrote: >>> I've opted for the alternate approach: make all clients pass iterators. >>> That is, get rid of the specialized one- and two-argument constructors >>> entirely.>> That sounds reasonable. Please keep the "Value* + num elements" ctor as >> well though. > > Why? I can't think of a case where a client uses that but can't use the > iterator version (just pass Value, Value+num). I'd rather not maintain two > interfaces with identical semantics.If nothing else, for temporary migration reasons. First step is to get the new interface in, second step is to migrate clients, third step is to remove the old one.>> Here's another crazy idea. Instead of overloading the callinst ctor, why >> not just add a static function to create the call (like binaryoperator and >> friends): >> >> C = CallInst::CreateWithArgRange(X, B, V.begin(), V.end()); >> >> That way you can completely avoid the overloading issues. > > That's not a bad idea. In fact, we ought to think about creating factories > for everything. Bare new/delete usually comes back to haunt one later on.Yep, I agree.> I'll get what I've got working and checked in. I'm not sure I'm ready to go > and change everything to CallInst factories given the tons of other things > I've got to work on.Of course :)> The iterator and one-argument constructors appear to cover 99% of the > cases and the two-argument clients can often just pass Use::op_iterators > directly. I've even found one case where we can eliminate a temporary vector.nice! -Chris -- http://nondot.org/sabre/ http://llvm.org/
On Thu, 5 Jul 2007, David Greene wrote:> On Thursday 05 July 2007 11:54, Chris Lattner wrote: >> Here's another crazy idea. Instead of overloading the callinst ctor, why >> not just add a static function to create the call (like binaryoperator and >> friends): >> >> C = CallInst::CreateWithArgRange(X, B, V.begin(), V.end()); >> >> That way you can completely avoid the overloading issues. > > What's the relationship of this idea to LLVMBuilder?It is orthogonal. -Chris -- http://nondot.org/sabre/ http://llvm.org/