[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

[Chris Lattner]

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.

>
> +++ 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.

>
> +    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.

> +  | '[' ']' {
> +    $$ = 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.

>
> +  | 'c' STRINGCONSTANT {
> +    int NumElements = $2->length();

Please use size_t or uint64_t.

Thanks Dan,

-Chris