[PATCH] Make .bc en/decoding of AttrKind stable

[Bill Wendling]

On Jul 25, 2013, at 9:21 PM, Tobias Grosser <tobias at grosser.es> wrote:

> On 07/25/2013 05:29 PM, Nick Lewycky wrote:
>> On 17 July 2013 15:54, Tobias Grosser <tobias at grosser.es> wrote:
>> 
>>> Hi,
>>> 
>>> I would like to commit the following patch:
>>> 
>>> ----------------------
>>> Make .bc en/decoding of AttrKind stable
>>> 
>>> The bitcode representation attribute kinds are encoded into / decoded from
>>> should be independent of the current set of LLVM attributes and their
>>> position in the AttrKind enum. This patch explicitly encodes attributes to
>>> fixed bitcode values.
>>> 
>>> With this patch applied, LLVM does not silently misread attributes written
>>> by LLVM 3.3. We also enhance the decoding slightly such that an error
>>> message is printed if an unknown AttrKind encoding was dected.
>>> 
>>> Bonus: Dropping bitcode attributes from AttrKind is now easy, as old
>>> AttrKinds do not need to be kept to support the Bitcode reader.
>>> ----------------------
>>> 
>>> I also considered assigning fixed numbers directly in the AttrKind enum
>>> (or keeping the enum stable), but convinced myself that the bitcode
>>> encoding should really not part of the IR interface.
>>> 
>>> OK to commit?
>>> 
>> 
>> Thanks for working on this.
> 
> Thanks for the review.
> 
>> +  default:
>> +    return Error("Unknown attribute kind");
>> 
>> Any chance we could get the value of the attribute kind in there? I know, I
>> know, but I forsee a future Nick debugging exactly this.
> 
> We now print:
> 
> "Unknown attribute kind (43)"
> 
>> 
>> +; RUN:  llvm-dis < %s.bc| FileCheck %s
>> 
>> Space before pipe please. :)
> 
> Fixed.
> 
> Patch committed in r187186.
> 
Thanks. I missed that bit. :-(

-bw