llvm dev - Sep 2009 - [LLVMdev] [llvm-commits] [llvm] r81845 - in /llvm/trunk: lib/VMCore/ConstantFold.cpp test/Assembler/insertextractvalue.ll

Chris Lattner wrote:
> URL: http://llvm.org/viewvc/llvm-project?rev=81845&view=rev
> Log:
> fix PR4963: folding insertvalue would sometimes turn a packed struct into
> an unpacked one.
About this bug -- it happened because ConstantStruct::get has a default 
parameter for whether the struct should be packed or not, which defaults 
to not packed.

This is strikingly error prone. We even list struct and packed struct as 
two different types in the LangRef. We don't need to go so far as to 
offer separate ConstantStruct and ConstantPackedStruct but maybe the 
isPacked parameter should at least be mandatory?

Any objections?

Nick

On Sep 14, 2009, at 11:33 PM, Nick Lewycky wrote:
> Chris Lattner wrote:
>> URL: http://llvm.org/viewvc/llvm-project?rev=81845&view=rev
>> Log:
>> fix PR4963: folding insertvalue would sometimes turn a packed  
>> struct into
>> an unpacked one.
>
> About this bug -- it happened because ConstantStruct::get has a  
> default
> parameter for whether the struct should be packed or not, which  
> defaults
> to not packed.
>
> This is strikingly error prone. We even list struct and packed  
> struct as
> two different types in the LangRef. We don't need to go so far as to
> offer separate ConstantStruct and ConstantPackedStruct but maybe the
> isPacked parameter should at least be mandatory?
Please do make it mandatory, thanks!

-Chris