Handle implicit operator overloading (#4063)

Currently, for a binary operator, we check whether the RHS expr defines
an operator using the operator overloading or not regardless the binary
operator to handle. If the RHS expr defines such operator, we build the
operator using the defined one. However, it can result in trying to
build an operator that is not actually defined by the RHS expr.
This commit lets it decide whether it has to build a new operator or use
some pre-defined one based on the type of the binary operator to handle
and the operator defined by the RHS expr.

Author: jaebaek

tools/clang/include/clang/AST/HlslTypes.h

@@ -431,6 +431,11 @@ bool GetIntrinsicOp(const clang::FunctionDecl *FD, unsigned &opcode,
                     llvm::StringRef &group);
 bool GetIntrinsicLowering(const clang::FunctionDecl *FD, llvm::StringRef &S);
 
+bool IsUserDefinedRecordType(clang::QualType type);
+bool DoesTypeDefineOverloadedOperator(clang::QualType typeWithOperator,
+                                      clang::OverloadedOperatorKind opc,
+                                      clang::QualType paramType);
+
 /// <summary>Adds a function declaration to the specified class record.</summary>
 /// <param name="context">ASTContext that owns declarations.</param>
 /// <param name="recordDecl">Record declaration in which to add function.</param>

tools/clang/lib/AST/HlslTypes.cpp

@@ -580,6 +580,49 @@ bool IsHLSLSubobjectType(clang::QualType type) {
   return GetHLSLSubobjectKind(type, kind, hgType);
 }
 
+bool IsUserDefinedRecordType(clang::QualType type) {
+  if (const auto *rt = type->getAs<RecordType>()) {
+    // HLSL specific types
+    if (hlsl::IsHLSLResourceType(type) || hlsl::IsHLSLVecMatType(type) ||
+        isa<ExtVectorType>(type.getTypePtr()) || type->isBuiltinType() ||
+        type->isArrayType()) {
+      return false;
+    }
+
+    // SubpassInput or SubpassInputMS type
+    if (rt->getDecl()->getName() == "SubpassInput" ||
+        rt->getDecl()->getName() == "SubpassInputMS") {
+      return false;
+    }
+    return true;
+  }
+
+  return false;
+}
+
+bool DoesTypeDefineOverloadedOperator(clang::QualType typeWithOperator,
+                                      clang::OverloadedOperatorKind opc,
+                                      clang::QualType paramType) {
+  if (const RecordType *recordType = typeWithOperator->getAs<RecordType>()) {
+    if (const CXXRecordDecl *cxxRecordDecl =
+            dyn_cast<CXXRecordDecl>(recordType->getDecl())) {
+      for (const auto *method : cxxRecordDecl->methods()) {
+        if (!method->isUserProvided() || method->getNumParams() != 1)
+          continue;
+        // It must be an implicit assignment.
+        if (opc == OO_Equal &&
+            typeWithOperator != method->getParamDecl(0)->getOriginalType() &&
+            typeWithOperator == paramType) {
+          continue;
+        }
+        if (method->getOverloadedOperator() == opc)
+          return true;
+      }
+    }
+  }
+  return false;
+}
+
 bool GetHLSLSubobjectKind(clang::QualType type, DXIL::SubobjectKind &subobjectKind, DXIL::HitGroupType &hgType) {
   hgType = (DXIL::HitGroupType)(-1);
   type = type.getCanonicalType();

tools/clang/lib/Sema/SemaExpr.cpp

@@ -1553,49 +1553,6 @@ static ExprResult BuildCookedLiteralOperatorCall(Sema &S, Scope *Scope,
   return S.BuildLiteralOperatorCall(R, OpNameInfo, Args, LitEndLoc);
 }
 
-// HLSL Change Starts
-static bool IsUserDefinedRecordType(QualType type) {
-  if (const auto *rt = type->getAs<RecordType>()) {
-    // HLSL specific types
-    if (hlsl::IsHLSLResourceType(type) || hlsl::IsHLSLVecMatType(type) ||
-        isa<ExtVectorType>(type.getTypePtr()) || type->isBuiltinType() ||
-        type->isArrayType()) {
-      return false;
-    }
-
-    // SubpassInput or SubpassInputMS type
-    if (rt->getDecl()->getName() == "SubpassInput" ||
-        rt->getDecl()->getName() == "SubpassInputMS") {
-      return false;
-    }
-    return true;
-  }
-
-  return false;
-}
-
-static bool DoesTypeDefineOverloadedOperator(QualType type) {
-  if (const RecordType *recordType = type->getAs<RecordType>()) {
-    if (const CXXRecordDecl *cxxRecordDecl =
-            dyn_cast<CXXRecordDecl>(recordType->getDecl())) {
-      bool found_overloaded_operator = false;
-      for (const auto *method : cxxRecordDecl->methods()) {
-        if (method->getOverloadedOperator() != OO_None)
-          found_overloaded_operator = true;
-      }
-      return found_overloaded_operator;
-    }
-  }
-  return false;
-}
-
-static bool IsUserDefinedRecordTypeWithOverloadedOperator(QualType type) {
-  if (!IsUserDefinedRecordType(type))
-    return false;
-  return DoesTypeDefineOverloadedOperator(type);
-}
-// HLSL Change Ends
-
 /// ActOnStringLiteral - The specified tokens were lexed as pasted string
 /// fragments (e.g. "foo" "bar" L"baz").  The result string has to handle string
 /// concatenation ([C99 5.1.1.2, translation phase #6]), so it may come from
@@ -10435,7 +10392,12 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc,
 
   // HLSL Change Starts
   // Handle HLSL binary operands differently
-  if (getLangOpts().HLSL) {
+  if (getLangOpts().HLSL &&
+          (!getLangOpts().EnableOperatorOverloading ||
+           !hlsl::IsUserDefinedRecordType(LHSExpr->getType())) ||
+      !hlsl::DoesTypeDefineOverloadedOperator(
+          LHSExpr->getType(), clang::BinaryOperator::getOverloadedOperator(Opc),
+          RHSExpr->getType())) {
     hlsl::CheckBinOpForHLSL(*this, OpLoc, Opc, LHS, RHS, ResultTy, CompLHSTy, CompResultTy);
     if (!ResultTy.isNull() && Opc == BO_Comma) {
       // In C/C++, the RHS value kind should propagate. In HLSL, it should yield an r-value.
@@ -10921,8 +10883,10 @@ ExprResult Sema::BuildBinOp(Scope *S, SourceLocation OpLoc,
   // methods or not.
   if (getLangOpts().CPlusPlus &&
       (!getLangOpts().HLSL || getLangOpts().EnableOperatorOverloading) &&
-      IsUserDefinedRecordTypeWithOverloadedOperator(LHSExpr->getType()) &&
-      IsUserDefinedRecordType(RHSExpr->getType())) {
+      hlsl::IsUserDefinedRecordType(LHSExpr->getType()) &&
+      hlsl::DoesTypeDefineOverloadedOperator(
+          LHSExpr->getType(), clang::BinaryOperator::getOverloadedOperator(Opc),
+          RHSExpr->getType())) {
     // If either expression is type-dependent, always build an
     // overloaded op.
     if (LHSExpr->isTypeDependent() || RHSExpr->isTypeDependent())

tools/clang/test/HLSL/use-undefined-overloaded-operator.hlsl

@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -fsyntax-only -Wno-unused-value -ffreestanding -verify -enable-operator-overloading %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-unused-value -ffreestanding -verify -HV 2021 %s
+
+// This test checks that when we use undefined overloaded operator
+// dxcompiler generates error and no crashes are observed.
+
+struct S1 {
+    float a;
+
+    float operator+(float x) {
+        return a + x;
+    }
+};
+
+struct S2 {
+    S1 s1;
+};
+
+void main(float4 pos: SV_Position) {
+    S1 s1;
+    S2 s2;
+    pos.x = s2.s1 + 0.1;
+    pos.x = s2.s1 + s1; // expected-error {{invalid operands to binary expression ('S1' and 'S1')}}
+}

tools/clang/test/HLSLFileCheck/hlsl/operator_overloading/operator.overloading.implicit-assign1.hlsl

@@ -0,0 +1,29 @@
+// RUN: %dxc -E main -T ps_6_0 -fcgl -enable-operator-overloading %s | FileCheck %s
+
+// CHECK: define internal void {{.*}}(%struct.MyArray* %this, %struct.MyArray* noalias sret %agg.result, %struct.MyArray* %RHS)
+
+#define MAX_SIZE 100
+
+struct MyArray {
+  float4 A[MAX_SIZE];
+
+  void splat(float f) {
+    for (int i = 0; i < MAX_SIZE; i++)
+      A[i] = f;
+  };
+
+  MyArray operator+(MyArray RHS) {
+    MyArray OutArray;
+    for (int i = 0; i < MAX_SIZE; i++)
+      OutArray.A[i] = A[i] + RHS.A[i];
+    return OutArray;
+  };
+};
+
+
+float4 main(float4 col1: COLOR0, float4 col2: COLOR2, int ix : I) : SV_Target {
+  MyArray A, B;
+  B.splat(col1);
+  A = A + B;
+  return A.A[ix];
+}

tools/clang/test/HLSLFileCheck/hlsl/operator_overloading/operator.overloading.implicit-assign2.hlsl

@@ -0,0 +1,18 @@
+// RUN: %dxc -E main -T ps_6_0 -HV 2021 %s | FileCheck %s
+
+// CHECK: define void @main()
+
+struct S1 {
+    float a;
+};
+
+struct S2 {
+    S1 s1;
+    Texture2D<float> tex;
+};
+
+void main(float4 pos: SV_Position) {
+    S1 s1;
+    S2 s2;
+    s2.s1 = s1;
+}

tools/clang/test/HLSLFileCheck/hlsl/operator_overloading/operator.overloading.lookup.definition.hlsl

@@ -0,0 +1,36 @@
+// RUN: %dxc -E main -T ps_6_0 -HV 2021 %s | FileCheck %s
+
+// CHECK: define void @main()
+
+struct S {
+    float a;
+
+    void operator=(float x) {
+        a = x;
+    }
+
+    float operator+(float x) {
+        return a + x;
+    }
+};
+
+struct Number {
+    int n;
+
+    void operator=(float x) {
+        n = x;
+    }
+};
+
+int main(float4 pos: SV_Position) : SV_Target {
+    S s1;
+    S s2;
+    s1 = s2;
+    s1 = 0.2;
+    s1 = s1 + 0.1;
+
+    Number a = {pos.x};
+    Number b = {pos.y};
+    a = pos.x;
+    return a.n;
+}

tools/clang/unittests/HLSL/VerifierTest.cpp

@@ -47,6 +47,7 @@ class VerifierTest TEST_CLASS_DERIVATION {
   TEST_METHOD(RunCppErrors)
   TEST_METHOD(RunCppErrorsHV2015)
   TEST_METHOD(RunOperatorOverloadingForNewDelete)
+  TEST_METHOD(RunOperatorOverloadingNotDefinedBinaryOp)
   TEST_METHOD(RunCXX11Attributes)
   TEST_METHOD(RunEnums)
   TEST_METHOD(RunFunctions)
@@ -198,6 +199,10 @@ TEST_F(VerifierTest, RunOperatorOverloadingForNewDelete) {
   CheckVerifiesHLSL(L"overloading-new-delete-errors.hlsl");
 }
 
+TEST_F(VerifierTest, RunOperatorOverloadingNotDefinedBinaryOp) {
+  CheckVerifiesHLSL(L"use-undefined-overloaded-operator.hlsl");
+}
+
 TEST_F(VerifierTest, RunCXX11Attributes) {
   CheckVerifiesHLSL(L"cxx11-attributes.hlsl");
 }