[cfe-commits] [llvm-commits] [PATCH] PR9214: convert InvokeInst API to use ArrayRef

[Chris Lattner]

On Apr 14, 2011, at 4:13 AM, Jay Foad wrote:
> See http://llvm.org/bugs/show_bug.cgi?id=9214
> 
> The bugs says:
> 
>> Various LLVM APIs (such as the Constant*::get APIs, the
>> GetElementPtrInst::Create methods, IRBuilder CreateGEP method, etc) that take
>> multiple input values take std::vectors, and some now also can take a pointer
>> to the start of an array and a count (for use with SmallVector).  The new
>> ArrayRef class subsumes both of these uses and is more convenient to use.
> 
> I had a look at the "IRBuilder CreateGEP method", and noticed that
> there are several Instructions that could take ArrayRefs where they
> currently take a pair of RandomAccessIterators, both in their own
> constructors and Create() methods, and in the corresponding IRBuilder
> API to create them. The Instructions in question are:
> 
> CallInst
> InvokeInst
> GetElementPtrInst
> InsertValueInst
> ExtractValueInst
> 
> This patch just converts InvokeInst, because I thought it would be the
> easiest one to start with.

Great, makes sense.

> I'm concerned about the amount of churn it causes. It touches more or
> less every place that creates an InvokeInst. I expect there to be way
> more churn with more common Instructions like CallInst and
> GetElementPtrInst. Do we really want to cause this pain for all
> clients of the LLVM APIs?

I think that this is clearly the right thing to do for the APIs, and we don't worry about client churn.  In terms of making your life easier, it is probably best to a) introduce a new ArrayRef overload, b) convert clients to use it (including clang etc) then c) remove the old overloads.

Thanks for working on this Jay!

-Chris