[OpenCL] Added parsing for OpenCL vector types.

[Anastasia]

Although nothing wrong with the implementation apart from it complicates a bit the understanding of conventional C parse flow by adding extra corner case, I don't see anything in the spec that states explicitly how the vector components should be parsed. I guess with extra parenthesis the parsing problem can easily be avoided:

((int3)(32, 32, 32)).x

test/Parser/vector-cast-define.cl

12

I would prefer to simply the test and avoid using macros... it will be easier for us to maintain it in the future.

[yaxunl]

I think this is a good feature for the convenience of user. I've seen usage like this.

[echuraev]

In D31183#708833, @yaxunl wrote:

I think this is a good feature for the convenience of user. I've seen usage like this.

I agree. I don't see any reasons why this case doesn't have the right to exist. I don't think that using extra parenthesis is a good solution for solving this problem.

[Anastasia]

In D31183#709566, @echuraev wrote:

In D31183#708833, @yaxunl wrote:

I think this is a good feature for the convenience of user. I've seen usage like this.

I agree. I don't see any reasons why this case doesn't have the right to exist. I don't think that using extra parenthesis is a good solution for solving this problem.

I am just saying that I don't see a big use case for this. I am guessing it can largely come from the macro expansions, but those are generally good style to parenthesize.

[echuraev]

In D31183#710202, @Anastasia wrote:

In D31183#709566, @echuraev wrote:

In D31183#708833, @yaxunl wrote:

I think this is a good feature for the convenience of user. I've seen usage like this.

I agree. I don't see any reasons why this case doesn't have the right to exist. I don't think that using extra parenthesis is a good solution for solving this problem.

I am just saying that I don't see a big use case for this. I am guessing it can largely come from the macro expansions, but those are generally good style to parenthesize.

Ok. But in current implementation if I forget to parenthesize the defined expression (as in the test) I will get the following message: "error: member reference base type 'int' is not a structure or union". I don't think that the message is clear to understand that you just forgot to add parenthesis.

So, should we change the diagnostic message to do it more understandable or push this patch because it can be more convenience for users?

[Anastasia]

I don't think that diagnostics can always be very clear. This is not the case neither for C nor C++.

As I said I don't see any issue to continue with this patch. I would just like to see the test simplified a bit.

[Anastasia]

LGTM!

include/clang/Parse/Parser.h

private:
 
   ExprResult ParseExpressionWithLeadingExtension(SourceLocation ExtLoc);
 
   ExprResult ParseRHSOfBinaryExpression(ExprResult LHS,
                                         prec::Level MinPrec);
   ExprResult ParseCastExpression(bool isUnaryExpression,
                                  bool isAddressOfOperand,
                                  bool &NotCastExpr,
-                                 TypeCastState isTypeCast);
+                                 TypeCastState isTypeCast,
+                                 bool isVectorLiteral = false);
   ExprResult ParseCastExpression(bool isUnaryExpression,
                                  bool isAddressOfOperand = false,
-                                 TypeCastState isTypeCast = NotTypeCast);
+                                 TypeCastState isTypeCast = NotTypeCast,
+                                 bool isVectorLiteral = false);
 
   /// Returns true if the next token cannot start an expression.
   bool isNotExpressionStart();
 
   /// Returns true if the next token would start a postfix-expression
   /// suffix.
   bool isPostfixExpressionSuffixStart() {
     tok::TokenKind K = Tok.getKind();

lib/Parse/ParseExpr.cpp

 /// \brief Parse a cast-expression, or, if \p isUnaryExpression is true,
 /// parse a unary-expression.
 ///
 /// \p isAddressOfOperand exists because an id-expression that is the
 /// operand of address-of gets special treatment due to member pointers.
 ///
 ExprResult Parser::ParseCastExpression(bool isUnaryExpression,
                                        bool isAddressOfOperand,
-                                       TypeCastState isTypeCast) {
+                                       TypeCastState isTypeCast,
+                                       bool isVectorLiteral) {
   bool NotCastExpr;
   ExprResult Res = ParseCastExpression(isUnaryExpression,
                                        isAddressOfOperand,
                                        NotCastExpr,
-                                       isTypeCast);
+                                       isTypeCast,
+                                       isVectorLiteral);
   if (NotCastExpr)
     Diag(Tok, diag::err_expected_expression);
   return Res;
 }
 
 namespace {
 class CastExpressionIdValidator : public CorrectionCandidateCallback {
  public:

 /// [Embarcadero] expression-trait:
 ///                   '__is_lvalue_expr'
 ///                   '__is_rvalue_expr'
 /// \endverbatim
 ///
 ExprResult Parser::ParseCastExpression(bool isUnaryExpression,
                                        bool isAddressOfOperand,
                                        bool &NotCastExpr,
-                                       TypeCastState isTypeCast) {
+                                       TypeCastState isTypeCast,
+                                       bool isVectorLiteral) {
   ExprResult Res;
   tok::TokenKind SavedKind = Tok.getKind();
   NotCastExpr = false;
 
   // This handles all of cast-expression, unary-expression, postfix-expression,
   // and primary-expression.  We handle them together like this for efficiency
   // and to simplify handling of an expression starting with a '(' token: which
   // may be one of a parenthesized expression, cast-expression, compound literal
case tok::l_paren: {
     ParenParseOption ParenExprType =
         (isUnaryExpression && !getLangOpts().CPlusPlus) ? CompoundLiteral
                                                         : CastExpr;
     ParsedType CastTy;
     SourceLocation RParenLoc;
     Res = ParseParenExpression(ParenExprType, false/*stopIfCastExr*/,
                                isTypeCast == IsTypeCast, CastTy, RParenLoc);
 
+    if (isVectorLiteral)
+        return Res;
+
     switch (ParenExprType) {
     case SimpleExpr:   break;    // Nothing else to do.
     case CompoundStmt: break;  // Nothing else to do.
     case CompoundLiteral:
       // We parsed '(' type-name ')' '{' ... '}'.  If any suffixes of
       // postfix-expression exist, parse them now.
       break;
     case CastExpr:
if (!DeclaratorInfo.isInvalidType() && Tok.is(tok::identifier) &&
         TypeResult Ty;
         {
           InMessageExpressionRAIIObject InMessage(*this, false);
           Ty = Actions.ActOnTypeName(getCurScope(), DeclaratorInfo);
         }
         return ParseCompoundLiteralExpression(Ty.get(), OpenLoc, RParenLoc);
       }
 
+      if (Tok.is(tok::l_paren)) {
+        // This could be OpenCL vector Literals
+        if (getLangOpts().OpenCL)
+        {
+          TypeResult Ty;
+          {
+            InMessageExpressionRAIIObject InMessage(*this, false);
+            Ty = Actions.ActOnTypeName(getCurScope(), DeclaratorInfo);
+          }
+          if(Ty.isInvalid())
+          {
+             return ExprError();
+          }
+          QualType QT = Ty.get().get().getCanonicalType();
+          if (QT->isVectorType())
+          {
+            // We parsed '(' vector-type-name ')' followed by '('
+
+            // Parse the cast-expression that follows it next.
+            // isVectorLiteral = true will make sure we don't parse any
+            // Postfix expression yet
+            Result = ParseCastExpression(/*isUnaryExpression=*/false,
+                                         /*isAddressOfOperand=*/false,
+                                         /*isTypeCast=*/IsTypeCast,
+                                         /*isVectorLiteral=*/true);
+
+            if (!Result.isInvalid()) {
+              Result = Actions.ActOnCastExpr(getCurScope(), OpenLoc,
+                                             DeclaratorInfo, CastTy,
+                                             RParenLoc, Result.get());
+            }
+
+            // After we performed the cast we can check for postfix-expr pieces.
+            if (!Result.isInvalid()) {
+              Result = ParsePostfixExpressionSuffix(Result);
+            }
+
+            return Result;
+          }
+        }
+      }
+
       if (ExprType == CastExpr) {
         // We parsed '(' type-name ')' and the thing after it wasn't a '{'.
 
         if (DeclaratorInfo.isInvalidType())
           return ExprError();
 
         // Note that this doesn't parse the subsequent cast-expression, it just
         // returns the parsed type to the callee.
if (!DeclaratorInfo.isInvalidType() && Tok.is(tok::identifier) &&
             getCurScope()->isInObjcMethodScope() &&
             GetLookAheadToken(1).isNot(tok::period)) {
           Diag(Tok.getLocation(), diag::err_illegal_super_cast)
             << SourceRange(OpenLoc, RParenLoc);
           return ExprError();
         }
 
         // Parse the cast-expression that follows it next.
+        // isVectorLiteral = true will make sure we don't parse any
+        // Postfix expression yet
         // TODO: For cast expression with CastTy.
         Result = ParseCastExpression(/*isUnaryExpression=*/false,
                                      /*isAddressOfOperand=*/false,
-                                     /*isTypeCast=*/IsTypeCast);
+                                     /*isTypeCast=*/IsTypeCast,
+                                     /*isVectorLiteral=*/true);
         if (!Result.isInvalid()) {
           Result = Actions.ActOnCastExpr(getCurScope(), OpenLoc,
                                          DeclaratorInfo, CastTy, 
                                          RParenLoc, Result.get());
         }
         return Result;
       }
 

test/Parser/vector-cast-define.cl

+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
+// expected-no-diagnostics
+
+typedef int int3 __attribute__((ext_vector_type(3)));
+
+void test()
+{
+    int index = (int3)(1, 2, 3).x * (int3)(3, 2, 1).y;
+}
+