[LLVMdev] API design

[Chris Lattner]

On Mon, 2 Jul 2007, David A. Greene wrote:
>>> - Changing the API
>>>  a) Template it to take two iterators. This causes code size bloat.
>
> This seems like the right solution to me.  Unless llvm is running on
> extremely limited memory embedded systems, the extra code space
> shouldn't be an issue.

Code size is always an issue.

What specifically are you proposing?  I assume you want to 
eliminate/supplement this ctor:

CallInst::CallInst(Value *Func, Value* const *Args, unsigned NumArgs,
                    const std::string &Name, BasicBlock *InsertAtEnd)

Turning it into a template would require duplicating it (small), and 
CallInst::init (large).

I don't really think this is workable.

>>> We could also just ignore the problem. I'm not clear on what the effects
>>> of that would be in the long term. Is this something we can expect will
>>> be changed in C++0x?
>>
>> I don't think that this issue causes any problems in practice.  The
>> biggest bad thing is that it makes the extra checking code apparently
>> useless :(
>
> The index operation can imply a dereference on some implementations,
> otherwise the checking code wouldn't have caught it.  In other words, the
> standard requires that the memory being indexed by referenceable.  Violating
> that could cause all sorts of interesting things, like segmentation faults or
> the sudden sprouting of rose bushes on Dan Gohman's head.
>
> I agree that on common architectures it probably won't cause a problem.
> But it still results in undefined behavior.

I agree about undefined behavior.  As I said above, I don't think it 
causes any problems on any implementations we have seen, nor is it likely 
to in the future.  That said, I obviously agree that relying on undefined 
behavior is bad :)

>>> I'd like to get the extra checks working so that they can help find our
>>> more subtle bugs. Any idea what we should do here?
>>
>> I don't really have a good idea, but I also don't want to significantly
>> uglify the sourcebase...
>
> Passing two iterators is in the spirit of the standard library, so I don't
> consider that uglification.  I understand others may disagree.

I totally agree that it is clean, the question is, how do we make it work?

Here's a different suggestion that cloning all the code.  Instead of doing 
that, why not add a new (templated) CallInst ctor, one which is very 
trivial.  Then you could put the conditional code in it (to detect an 
empty range) and call into the non-inline stuff we already have.

It should be fine to only support random access iterators, so you could 
call into the existing stuff that takes a base + size.

-Chris

-- 
http://nondot.org/sabre/
http://llvm.org/