[llvm-commits] [llvm] r52149 - in /llvm/trunk: lib/AsmParser/llvmAsmParser.cpp.cvs lib/AsmParser/llvmAsmParser.h.cvs lib/AsmParser/llvmAsmParser.y lib/AsmParser/llvmAsmParser.y.cvs utils/llvm.grm

[Dan Gohman]

On Jun 23, 2008, at 10:18 AM, Chris Lattner wrote:

> On Jun 9, 2008, at 7:45 AM, Dan Gohman wrote:
>> Author: djg
>> Date: Mon Jun  9 09:45:02 2008
>> New Revision: 52149
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=52149&view=rev
>> Log:
>> AsmParser support for immediate constant aggregate values.
>
> Hey Dan,
>
> General comment: please commit the change of .y file separate from
> the .cvs files.  They are large and noisy.

Ok.

>
>
>>
>> +++ llvm/trunk/lib/AsmParser/llvmAsmParser.y Mon Jun  9 09:45:02 2008
>> @@ -2505,6 +2499,80 @@
>>    delete PTy; delete $2;
>>    CHECK_FOR_ERROR
>>  }
>> +  | '[' ConstVector ']' { // Nonempty unsized arr
>> +    const Type *ETy = (*$2)[0]->getType();
>> +    int NumElements = $2->size();
>
> Please use uint64_t for NumElements.

fixed.

>
>
>>
>> +    ArrayType *ATy = ArrayType::get(ETy, NumElements);
>> +    PATypeHolder* PTy = new PATypeHolder(HandleUpRefs(ATy));
>> +
>> +    // Verify all elements are correct type!
>> +    for (unsigned i = 0; i < $2->size(); i++) {
>
> Here too.

fixed.

>
>
>> +  | '[' ']' {
>> +    $$ = ValID::createUndef();
>> +    CHECK_FOR_ERROR
>> +  }
>
> This should make an empty array not an undef.  I'm not sure if it make
> a difference in practice though.

The problem is that this bit of code doesn't know the element type.
ValID::createUndef lets the array type be inferred later. I judged it
not important enough to justify a significantly more elaborate
solution. I added a comment to explain it now.

>
>
>>
>> +  | 'c' STRINGCONSTANT {
>> +    int NumElements = $2->length();
>
> Please use size_t or uint64_t.

fixed.

Thanks,

Dan