[cfe-commits] r78535 - in /cfe/trunk: include/clang/AST/ include/clang/Basic/ include/clang/Parse/ lib/AST/ lib/CodeGen/ lib/Frontend/ lib/Parse/ lib/Sema/ test/Sema/ tools/clang-cc/

[Nate Begeman]

On Aug 9, 2009, at 12:36 PM, Eli Friedman wrote:

> On Sun, Aug 9, 2009 at 10:55 AM, Nate Begeman<natebegeman at mac.com>  
> wrote:
>> URL: http://llvm.org/viewvc/llvm-project?rev=78535&view=rev
>> Log:
>> AltiVec-style vector initializer syntax, vec4 a = (vec4)(a, b, c, d);
>
> This is generally nasty, but I don't think there's anything we can do
> about that...

Short of going back in time, no.  Also, it does have its advantages.

> You never actually use this warning; did you forget to remove it?

It is used, SemaExpr.cpp:3146

>>   // Perform default conversions.
>>   DefaultFunctionArrayConversion(BaseExpr);
>
> Do we end up running into this issue with all postfix operators?  If
> so, is there some easy way to make the parser deal with it?  I don't
> like scattering these checks all over Sema, especially considering
> that you missed some places.

I'm not sure;  I'm not a huge fan of it either but no better solution  
came to mind.

>
>> +Action::OwningExprResult
>> +Sema::ActOnCastOfParenListExpr(Scope *S, SourceLocation LParenLoc,
>> +                               SourceLocation RParenLoc,
>> +                               ParenListExpr *E, QualType Ty) {
>> +  // If this is an altivec initializer, '(' type ')' '('  
>> init, ..., init ')'
>> +  // then handle it as such.
>> +  if (getLangOptions().AltiVec && Ty->isVectorType()) {
>
> What if Ty is dependent?

I'm not sure what this means for this code, can you explain further?

> Also, do we want to use this path
> unconditionally if Ty is an ext_vector_type?

I don't think so, there's no requirement that ext vector types be used  
with AltiVec.

>
>> +    if (E->getNumExprs() == 0) {
>> +      Diag(E->getExprLoc(), diag::err_altivec_empty_initializer);
>> +      return ExprError();
>> +    }
>> +
>> +    llvm::SmallVector<Expr *, 8> initExprs;
>> +    for (unsigned i = 0, e = E->getNumExprs(); i != e; ++i)
>> +      initExprs.push_back(E->getExpr(i));
>> +
>> +    // FIXME: This means that pretty-printing the final AST will  
>> produce curly
>> +    // braces instead of the original commas.
>> +    InitListExpr *E = new (Context) InitListExpr(LParenLoc,  
>> &initExprs[0],
>> +                                                 initExprs.size(),  
>> RParenLoc);
>> +    E->setType(Ty);
>> +    return ActOnCompoundLiteral(LParenLoc, Ty.getAsOpaquePtr(),  
>> RParenLoc,
>> +                                Owned(E));
>> +  } else {
>> +    // This is not an AltiVec-style cast, so turn the  
>> ParenListExpr into a
>> +    // sequence of BinOp comma operators.
>> +    OwningExprResult Result = ConvertParenListExpr(S, E);
>> +    Expr *castExpr = (Expr *)Result.get();
>> +    CastExpr::CastKind Kind = CastExpr::CK_Unknown;
>> +
>> +    if (CheckCastTypes(SourceRange(LParenLoc, RParenLoc), Ty,  
>> castExpr, Kind))
>> +      return ExprError();
>> +
>> +    return Owned(new (Context) CStyleCastExpr 
>> (Ty.getNonReferenceType(),
>> +                                              CastExpr::CK_Unknown,
>> +                                              Result.takeAs<Expr> 
>> (), Ty,
>> +                                              LParenLoc,  
>> RParenLoc));
>
> This is extremely ugly... could you possibly make it call into the
> usual Sema path for casts?

I'll look into it.

>
>> --- cfe/trunk/lib/Sema/SemaInit.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaInit.cpp Sun Aug  9 12:55:44 2009
>> @@ -805,16 +805,47 @@
>>                                       unsigned &StructuredIndex) {
>>   if (Index < IList->getNumInits()) {
>>     const VectorType *VT = DeclType->getAsVectorType();
>> -    int maxElements = VT->getNumElements();
>> +    unsigned maxElements = VT->getNumElements();
>> +    unsigned numEltsInit = 0;
>>     QualType elementType = VT->getElementType();
>>
>> -    for (int i = 0; i < maxElements; ++i) {
>> -      // Don't attempt to go past the end of the init list
>> -      if (Index >= IList->getNumInits())
>> -        break;
>> -      CheckSubElementType(IList, elementType, Index,
>> -                          StructuredList, StructuredIndex);
>> +    if (!SemaRef.getLangOptions().OpenCL) {
>
> Perhaps this should check for an ext_vector_type rather than OpenCL?

It's technically a requirement of OpenCL not ext_vectors, but since  
the two are largely the same thing, I can make that change.

>
>> +    // OpenCL & AltiVec require all elements to be initialized.
>> +    if (numEltsInit != maxElements)
>> +      if (SemaRef.getLangOptions().OpenCL || SemaRef.getLangOptions 
>> ().AltiVec)
>> +        SemaRef.Diag(IList->getSourceRange().getBegin(),
>> +                     diag::err_vector_incorrect_num_initializers)
>> +          << (numEltsInit < maxElements) << maxElements <<  
>> numEltsInit;
>>   }
>>  }
>
> So turning on Altivec makes something we normally accept into an
> error?  That strikes me as strange at best.  I'm not sure if I have
> any better ideas, though.

This will be turned into a warning, gcc doesn't complain about it and  
the AltiVec PIM is vague about whether it is strictly required.

Nate