Check Syntax errors for matrix/vector subscript operators (#123)

Check Syntax errors for matrix/vector subscript operators
 - Throw errors for subscript operators with index out of bounds
 - Throw errors for cases where matrix/vectors are modified using subscript operators
 - There was an issue on the release build with the vector/matrix variables
    picking invalid candidate for overloaded array subscript operators. This was because
    implicit conversion sequence was not initialized correctly.

tools/clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -7454,6 +7454,8 @@ def err_hlsl_matrix_member_mixing_refs: Error<
   "matrix subscript '%0' mixes one-based and zero-based references">;
 def err_hlsl_matrix_member_out_of_bounds: Error<
   "matrix subscript '%0' is out of bounds">;
+def err_hlsl_matrix_row_index_out_of_bounds: Error<
+  "matrix row index '%0' is out of bounds">;
 def err_hlsl_matrix_member_too_many_positions: Error<
   "more than four positions are referenced in '%0'">;
 def err_hlsl_matrix_member_zero_in_one_based: Error<
@@ -7464,6 +7466,8 @@ def err_hlsl_vector_member_empty: Error<
   "vector swizzle is empty '%0'">;
 def err_hlsl_vector_member_out_of_bounds: Error<
   "vector swizzle '%0' is out of bounds">;
+def err_hlsl_vector_element_index_out_of_bounds: Error<
+  "vector element index '%0' is out of bounds">;
 def err_hlsl_vector_member_too_many_positions: Error<
   "more than four positions are referenced in '%0'">;
 def err_hlsl_missing_type_specifier : Error< // Patterened after err_missing_type_specifier

tools/clang/include/clang/Sema/Sema.h

@@ -8781,6 +8781,9 @@ private:
   void CheckArrayAccess(const Expr *BaseExpr, const Expr *IndexExpr,
                         const ArraySubscriptExpr *ASE=nullptr,
                         bool AllowOnePastEnd=true, bool IndexNegated=false);
+  // HLSL Change Starts - checking array subscript access to vector or matrix member
+  void CheckHLSLArrayAccess(const Expr *expr);
+  // HLSL Change ends
   void CheckArrayAccess(const Expr *E);
   // Used to grab the relevant information from a FormatAttr and a
   // FunctionDeclaration.

tools/clang/lib/Sema/SemaChecking.cpp

@@ -8378,6 +8378,7 @@ static bool IsTailPaddedMemberArray(Sema &S, llvm::APInt Size,
   return true;
 }
 
+
 void Sema::CheckArrayAccess(const Expr *BaseExpr, const Expr *IndexExpr,
                             const ArraySubscriptExpr *ASE,
                             bool AllowOnePastEnd, bool IndexNegated) {
@@ -8531,6 +8532,14 @@ void Sema::CheckArrayAccess(const Expr *expr) {
           CheckArrayAccess(rhs);
         return;
       }
+      // HLSL Change Starts : Access checking for HLSL vector and matrix array subscript
+      case Stmt::CXXOperatorCallExprClass : {
+        if (getLangOpts().HLSL) {
+            CheckHLSLArrayAccess(expr);
+        }
+        return;
+      }
+      // HLSL Change Ends
       default:
         return;
     }

tools/clang/lib/Sema/SemaExpr.cpp

@@ -9345,10 +9345,38 @@ static void DiagnoseConstAssignment(Sema &S, const Expr *E,
   S.Diag(Loc, diag::err_typecheck_assign_const) << ExprRange << ConstUnknown;
 }
 
+static bool HLSLCheckForModifiableLValue(
+    Expr *E,
+    SourceLocation Loc,
+    Sema &S
+) {
+    assert(isa<CXXOperatorCallExpr>(E));
+    const CXXOperatorCallExpr *expr = cast<CXXOperatorCallExpr>(E);
+    const Expr *LHS = expr->getArg(0);
+    QualType qt = LHS->getType();
+
+    // Check modifying const matrix with double subscript operator calls
+    if (isa<CXXOperatorCallExpr>(expr->getArg(0)))
+        return HLSLCheckForModifiableLValue(const_cast<Expr *>(expr->getArg(0)), Loc, S);
+
+    if (qt.isConstQualified() && (hlsl::IsMatrixType(&S, qt) || hlsl::IsVectorType(&S, qt))) {
+      DiagnoseConstAssignment(S, LHS, Loc);
+      return true;
+    }
+    return false;
+}
+
 /// CheckForModifiableLvalue - Verify that E is a modifiable lvalue.  If not,
 /// emit an error and return true.  If so, return false.
 bool CheckForModifiableLvalue(Expr *E, SourceLocation Loc, Sema &S) { // HLSL Change: export this function
   assert(!E->hasPlaceholderType(BuiltinType::PseudoObject));
+  // HLSL Change Starts - check const for array subscript operator for HLSL vector/matrix
+  if (S.Context.getLangOpts().HLSL && E->getStmtClass() == Stmt::CXXOperatorCallExprClass) {
+      // check if it's a vector or matrix
+      return HLSLCheckForModifiableLValue(E, Loc, S);
+  }
+  // HLSL Change Ends
+
   SourceLocation OrigLoc = Loc;
   Expr::isModifiableLvalueResult IsLV = E->isModifiableLvalue(S.Context,
                                                               &Loc);

tools/clang/lib/Sema/SemaHLSL.cpp

@@ -10812,6 +10812,40 @@ clang::QualType hlsl::CheckVectorConditional(
   return HLSLExternalSource::FromSema(self)->CheckVectorConditional(Cond, LHS, RHS, QuestionLoc);
 }
 
+void Sema::CheckHLSLArrayAccess(const Expr *expr) {
+  DXASSERT_NOMSG(isa<CXXOperatorCallExpr>(expr));
+  const CXXOperatorCallExpr *OperatorCallExpr = cast<CXXOperatorCallExpr>(expr);
+  DXASSERT_NOMSG(OperatorCallExpr->getOperator() == OverloadedOperatorKind::OO_Subscript);
+
+  const Expr *RHS = OperatorCallExpr->getArg(1); // first subscript expression
+  llvm::APSInt index;
+  if (RHS->EvaluateAsInt(index, Context)) {
+      int64_t intIndex = index.getLimitedValue();
+      const QualType LHSQualType = OperatorCallExpr->getArg(0)->getType();
+      if (IsVectorType(this, LHSQualType)) {
+          uint32_t vectorSize = GetHLSLVecSize(LHSQualType);
+          // If expression is a double two subscript operator for matrix (e.g x[0][1])
+          // we also have to check the first subscript oprator by recursively calling
+          // this funciton for the first CXXOperatorCallExpr
+          if (isa<CXXOperatorCallExpr>(OperatorCallExpr->getArg(0))) {
+              CheckHLSLArrayAccess(cast<CXXOperatorCallExpr>(OperatorCallExpr->getArg(0)));
+          }
+          if (intIndex < 0 || (uint32_t)intIndex >= vectorSize) {
+              Diag(RHS->getExprLoc(),
+                  diag::err_hlsl_vector_element_index_out_of_bounds)
+                  << (int)intIndex;
+          }
+      }
+      else if (IsMatrixType(this, LHSQualType)) {
+          uint32_t rowCount, colCount;
+          GetHLSLMatRowColCount(LHSQualType, rowCount, colCount);
+          if (intIndex < 0 || (uint32_t)intIndex >= rowCount) {
+              Diag(RHS->getExprLoc(), diag::err_hlsl_matrix_row_index_out_of_bounds)
+                  << (int)intIndex;
+          }
+      }
+  }
+}
 clang::QualType ApplyTypeSpecSignToParsedType(
     _In_ clang::Sema* self,
     _In_ clang::QualType &type,
@@ -10820,4 +10854,4 @@ clang::QualType ApplyTypeSpecSignToParsedType(
 )
 {
     return HLSLExternalSource::FromSema(self)->ApplyTypeSpecSignToParsedType(type, TSS, Loc);
-}
+}

tools/clang/lib/Sema/SemaOverload.cpp

@@ -144,6 +144,7 @@ ImplicitConversionRank clang::GetConversionRank(ImplicitConversionKind Kind) {
   };
   static_assert(_countof(Rank) == ICK_Num_Conversion_Kinds,
       "Otherwise, GetConversionRank is out of sync with ImplicitConversionKind"); // HLSL Change
+  assert((int)Kind < (int)ICK_Num_Conversion_Kinds); // HLSL Change
   return Rank[(int)Kind];
 }
 
@@ -4927,6 +4928,7 @@ TryObjectArgumentInitialization(Sema &S, QualType FromType,
   ICS.Standard.BindsToRvalue = FromClassification.isRValue();
   ICS.Standard.BindsImplicitObjectArgumentWithoutRefQualifier
     = (Method->getRefQualifier() == RQ_None);
+  ICS.Standard.ComponentConversion = ICK_Identity;
   return ICS;
 }
 

tools/clang/test/HLSL/matrix-syntax.hlsl

@@ -82,6 +82,7 @@ void main() {
     matrix<float, 4, 4> mymatrix;
     matrix<float, 4, 4> absMatrix = abs(mymatrix);
     matrix<float, 4, 4> absMatrix2 = abs(absMatrix);
+    const matrix<float, 4, 4> myConstMatrix = mymatrix;                /* expected-note {{variable 'myConstMatrix' declared const here}} expected-note {{variable 'myConstMatrix' declared const here}} fxc-pass {{}} */
 
     matrix<float, 2, 4> f24;
     float f;
@@ -137,4 +138,23 @@ void main() {
       `-ExtMatrixElementExpr <col:10, col:19> 'vector<float, 4>':'vector<float, 4>' _11_11_44_44
         `-DeclRefExpr <col:10> 'matrix<float, 4, 4>':'matrix<float, 4, 4>' lvalue Var 'mymatrix' 'matrix<float, 4, 4>':'matrix<float, 4, 4>'
     */
+    // member assignment using subscript syntax
+    f = mymatrix[0][0];
+    f = mymatrix[1][1];
+    f2 = mymatrix[1].xx;
+    f4 = mymatrix[2];
+
+    f = mymatrix[0][4];                                     /* expected-error {{vector element index '4' is out of bounds}} fxc-pass {{}} */
+    f = mymatrix[-1][3];                                    /* expected-error {{matrix row index '-1' is out of bounds}} fxc-pass {{}} */
+    f4 = mymatrix[10];                                      /* expected-error {{matrix row index '10' is out of bounds}} fxc-pass {{}} */
+
+    // accessing const member
+    f = myConstMatrix[0][0];
+    f = myConstMatrix[1][1];
+    f2 = myConstMatrix[1].xx;
+    f4 = myConstMatrix[2];
+
+    myConstMatrix[0][0] = 3;                                /* expected-error {{cannot assign to variable 'myConstMatrix' with const-qualified type 'const matrix<float, 4, 4>'}} fxc-error {{X3025: l-value specifies const object}} */
+    myConstMatrix[3] = float4(1,2,3,4);                     /* expected-error {{cannot assign to variable 'myConstMatrix' with const-qualified type 'const matrix<float, 4, 4>'}} fxc-error {{X3025: l-value specifies const object}} */
+
 }

tools/clang/test/HLSL/vector-syntax.hlsl

@@ -111,6 +111,25 @@ float fn() {
     myvar.y = 1.0f;
     myvar.z = 1.0f;
     myvar.w = 1.0f;
+    myvar[0] = 1.0f;
+    myvar[1]= 1.0f;
+    myvar[2] = 1.0f;
+    myvar[3] = 1.0f;
+    myvar[-10] = 1.0f;            /* expected-error {{vector element index '-10' is out of bounds}} fxc-pass {{}} */
+    myvar[4] = 1.0f;              /* expected-error {{vector element index '4' is out of bounds}} fxc-pass {{}} */
+
+    float3 myvar3 = float3(1,2,3);
+    myvar3[3] = 1.0f;             /* expected-error {{vector element index '3' is out of bounds}} fxc-pass {{}} */
+
+    float2 myvar2 = float2(1,2);
+    myvar2[2] = 1.0f;             /* expected-error {{vector element index '2' is out of bounds}} fxc-pass {{}} */
+
+    float1 myvar1 = float1(1);
+    myvar1[1] = 1.0f;             /* expected-error {{vector element index '1' is out of bounds}} fxc-pass {{}} */
+
+    const float4 constMyVar = float4(1,2,3,4);              /* expected-note {{variable 'constMyVar' declared const here}} expected-note {{variable 'constMyVar' declared const here}} fxc-pass {{}} */
+    constMyVar = float4(1,1,1,1); /* expected-error {{cannot assign to variable 'constMyVar' with const-qualified type 'const float4'}} fxc-error {{X3025: l-value specifies const object}} */
+    constMyVar[0] = 2.0f;         /* expected-error {{cannot assign to variable 'constMyVar' with const-qualified type 'const float4'}} fxc-error {{X3025: l-value specifies const object}} */
 
     float4 myothervar;
     myothervar.rgba = myvar.xyzw;
@@ -132,25 +151,25 @@ float fn() {
     uint3 u3;
     u3.xyz = f.xxx;
     /*verify-ast
-      BinaryOperator <col:5, col:16> 'vector<uint, 3>':'vector<uint, 3>' '='
-      |-HLSLVectorElementExpr <col:5, col:8> 'vector<uint, 3>':'vector<uint, 3>' lvalue vectorcomponent xyz
-      | `-DeclRefExpr <col:5> 'uint3':'vector<uint, 3>' lvalue Var 'u3' 'uint3':'vector<uint, 3>'
-      `-ImplicitCastExpr <col:14, col:16> 'vector<uint, 3>' <HLSLCC_FloatingToIntegral>
+      BinaryOperator <col:5, col:16> 'vector<uint, 3>':'vector<unsigned int, 3>' '='
+      |-HLSLVectorElementExpr <col:5, col:8> 'vector<uint, 3>':'vector<unsigned int, 3>' lvalue vectorcomponent xyz
+      | `-DeclRefExpr <col:5> 'uint3':'vector<unsigned int, 3>' lvalue Var 'u3' 'uint3':'vector<unsigned int, 3>'
+      `-ImplicitCastExpr <col:14, col:16> 'vector<unsigned int, 3>' <HLSLCC_FloatingToIntegral>
         `-HLSLVectorElementExpr <col:14, col:16> 'vector<float, 3>':'vector<float, 3>' xxx
           `-ImplicitCastExpr <col:14> 'vector<float, 1>':'vector<float, 1>' lvalue <HLSLVectorSplat>
             `-DeclRefExpr <col:14> 'float' lvalue Var 'f' 'float'
     */
     u3 = (!u3).zyx;
     /*verify-ast
-      BinaryOperator <col:5, col:16> 'uint3':'vector<uint, 3>' '='
-      |-DeclRefExpr <col:5> 'uint3':'vector<uint, 3>' lvalue Var 'u3' 'uint3':'vector<uint, 3>'
-      `-ImplicitCastExpr <col:10, col:16> 'vector<uint, 3>' <HLSLCC_IntegralCast>
+      BinaryOperator <col:5, col:16> 'uint3':'vector<unsigned int, 3>' '='
+      |-DeclRefExpr <col:5> 'uint3':'vector<unsigned int, 3>' lvalue Var 'u3' 'uint3':'vector<unsigned int, 3>'
+      `-ImplicitCastExpr <col:10, col:16> 'vector<unsigned int, 3>' <HLSLCC_IntegralCast>
         `-HLSLVectorElementExpr <col:10, col:16> 'vector<bool, 3>':'vector<bool, 3>' zyx
           `-ParenExpr <col:10, col:14> 'vector<bool, 3>':'vector<bool, 3>'
             `-UnaryOperator <col:11, col:12> 'vector<bool, 3>':'vector<bool, 3>' prefix '!'
               `-ImplicitCastExpr <col:12> 'vector<bool, 3>' <HLSLCC_IntegralToBoolean>
-                `-ImplicitCastExpr <col:12> 'uint3':'vector<uint, 3>' <LValueToRValue>
-                  `-DeclRefExpr <col:12> 'uint3':'vector<uint, 3>' lvalue Var 'u3' 'uint3':'vector<uint, 3>'
+                `-ImplicitCastExpr <col:12> 'uint3':'vector<unsigned int, 3>' <LValueToRValue>
+                  `-DeclRefExpr <col:12> 'uint3':'vector<unsigned int, 3>' lvalue Var 'u3' 'uint3':'vector<unsigned int, 3>'
     */
     f.xx = 2; // expected-error {{vector is not assignable (contains duplicate components)}} fxc-error {{X3025: l-value specifies const object}}
     u3.x = u3.w;                                            /* expected-error {{vector swizzle 'w' is out of bounds}} fxc-error {{X3018: invalid subscript 'w'}} */