[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/

[Eli Friedman]

On Sun, Aug 9, 2009 at 10:55 AM, Nate Begeman<natebegeman at mac.com> wrote:
> Author: sampo
> Date: Sun Aug  9 12:55:44 2009
> New Revision: 78535
>
> 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...

> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sun Aug  9 12:55:44 2009
> @@ -2015,6 +2015,9 @@
>   "index for __builtin_shufflevector must be less than the total number "
>   "of vector elements">;
>
> +def err_vector_incorrect_num_initializers : Error<
> +  "%select{too many|too few}0 elements in vector initialization (expected %1 elements, have %2)">;
> +def err_altivec_empty_initializer : Error<"expected initializer">;

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

> @@ -2114,6 +2115,11 @@
>   Expr *BaseExpr = Base.takeAs<Expr>();
>   assert(BaseExpr && "no record expression");
>
> +  // If BaseExpr is a ParenListExpr then convert it into a standard
> +  // paren expr since this is not an altivec initializer.
> +  if (ParenListExpr *PE = dyn_cast<ParenListExpr>(BaseExpr))
> +    BaseExpr = ConvertParenListExpr(S, PE).takeAs<Expr>();
> +
>   // 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.

> +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?  Also, do we want to use this path
unconditionally if Ty is an ext_vector_type?

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

> --- 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?

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

-Eli