[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 12:53 PM, Nate Begeman<natebegeman at mac.com> wrote:
>> You never actually use this warning; did you forget to remove it?
>
> It is used, SemaExpr.cpp:3146

Ah, I missed that.

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

Take something like the following:

template<typename T> T f() { return (T)(1,2,3,4); }

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

I was more considering the OpenCL case; I think we want to make
ext_vector_type follow the OpenCL rules in all modes.

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

Yes, please do; for consistency, I think we want to make ext_vectors
follow the OpenCL rules in all modes.

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

Okay, that's fine.

-Eli