Make this more consistently a reference type (microsoft#4112)

* Make `this` more consistently a reference type Prior to this change explicit `this` access inside a struct was treated as a reference, but implicit `this` was still a pointer type. With this change `this` is always a reference type. This has no impact on code generation since references and pointers all digress to addresses, but it does make the AST more accurately reflect the types and it resolves microsoft#3732. * Updating test case to handle incorrect operator
llvm-beanz · Dec 3, 2021 · fb7b443 · fb7b443
1 parent 374044c
commit fb7b443
Show file tree

Hide file tree

Showing 6 changed files with 128 additions and 9 deletions.
diff --git a/tools/clang/include/clang/Sema/Sema.h b/tools/clang/include/clang/Sema/Sema.h
@@ -195,6 +195,8 @@ namespace clang {
   struct DeductionFailureInfo;
   class TemplateSpecCandidateSet;
 
+  class CXXThisExpr; // HLSL Change
+
 namespace sema {
   class AccessedEntity;
   class BlockScopeInfo;
@@ -9077,6 +9079,11 @@ class Sema {
       return NumArgs + 1 > NumParams; // If so, we view as an extra argument.
     return NumArgs > NumParams;
   }
+
+  // HLSL Change Begin - adjust this from T* to T&-like
+  CXXThisExpr *genereateHLSLThis(SourceLocation Loc, QualType ThisType,
+                                bool isImplicit);
+  // HLSL Change End - adjust this from T* to T&-like
 };
 
 /// \brief RAII object that enters a new expression evaluation context.

diff --git a/tools/clang/lib/Sema/SemaExprCXX.cpp b/tools/clang/lib/Sema/SemaExprCXX.cpp
@@ -971,17 +971,24 @@ ExprResult Sema::ActOnCXXThis(SourceLocation Loc) {
   CheckCXXThisCapture(Loc);
   // HLSL Change Starts - adjust this from T* to T&-like
   if (getLangOpts().HLSL && ThisTy.getTypePtr()->isPointerType()) {
-    // Expressions cannot be of reference type - instead, they yield
-    // an lvalue on the underlying type.
-    CXXThisExpr* ResultExpr = new (Context)CXXThisExpr(
-      Loc, ThisTy.getTypePtr()->getPointeeType(), /*isImplicit=*/false);
-    ResultExpr->setValueKind(ExprValueKind::VK_LValue);
-    return ResultExpr;
+    return genereateHLSLThis(Loc, ThisTy, /*isImplicit=*/false);
   }
   // HLSL Change Ends
   return new (Context) CXXThisExpr(Loc, ThisTy, /*isImplicit=*/false);
 }
 
+// HLSL Change Begin - adjust this from T* to T&-like
+CXXThisExpr *Sema::genereateHLSLThis(SourceLocation Loc, QualType ThisType,
+                                   bool isImplicit) {
+  // Expressions cannot be of reference type - instead, they yield
+  // an lvalue on the underlying type.
+  CXXThisExpr *ResultExpr = new (Context)
+      CXXThisExpr(Loc, ThisType.getTypePtr()->getPointeeType(), isImplicit);
+  ResultExpr->setValueKind(ExprValueKind::VK_LValue);
+  return ResultExpr;
+}
+// HLSL Change End - adjust this from T* to T&-like
+
 bool Sema::isThisOutsideMemberFunctionBody(QualType BaseType) {
   // If we're outside the body of a member function, then we'll have a specified
   // type for 'this'.

diff --git a/tools/clang/lib/Sema/SemaExprMember.cpp b/tools/clang/lib/Sema/SemaExprMember.cpp
@@ -1049,7 +1049,13 @@ Sema::BuildMemberReferenceExpr(Expr *BaseExpr, QualType BaseExprType,
     if (SS.getRange().isValid())
       Loc = SS.getRange().getBegin();
     CheckCXXThisCapture(Loc);
-    BaseExpr = new (Context) CXXThisExpr(Loc, BaseExprType,/*isImplicit=*/true);
+
+    // HLSL Change Starts - adjust this from T* to T&-like
+    if (getLangOpts().HLSL && BaseExprType->isPointerType())
+      BaseExpr = genereateHLSLThis(Loc, BaseExprType, /*isImplicit=*/true);
+    else
+      BaseExpr = new (Context) CXXThisExpr(Loc, BaseExprType,/*isImplicit=*/true);
+    // HLSL Change Ends
   }
 
   bool ShouldCheckUse = true;
@@ -1762,12 +1768,17 @@ Sema::BuildImplicitMemberExpr(const CXXScopeSpec &SS,
     if (SS.getRange().isValid())
       Loc = SS.getRange().getBegin();
     CheckCXXThisCapture(Loc);
-    baseExpr = new (Context) CXXThisExpr(loc, ThisTy, /*isImplicit=*/true);
+    if (getLangOpts().HLSL && ThisTy->isPointerType()) {
+      baseExpr = genereateHLSLThis(Loc, ThisTy, /*isImplicit=*/true);
+      ThisTy = ThisTy->getAs<PointerType>()->getPointeeType();
+    } else
+      baseExpr = new (Context) CXXThisExpr(loc, ThisTy, /*isImplicit=*/true);
   }
 
   return BuildMemberReferenceExpr(baseExpr, ThisTy,
                                   /*OpLoc*/ SourceLocation(),
-                                  /*IsArrow*/ true,
+                                  // HLSL Change - this is a reference
+                                  /*IsArrow*/ !getLangOpts().HLSL,
                                   SS, TemplateKWLoc,
                                   /*FirstQualifierInScope*/ nullptr,
                                   R, TemplateArgs);

diff --git a/tools/clang/lib/Sema/TreeTransform.h b/tools/clang/lib/Sema/TreeTransform.h
@@ -2343,6 +2343,10 @@ class TreeTransform {
                                 QualType ThisType,
                                 bool isImplicit) {
     getSema().CheckCXXThisCapture(ThisLoc);
+    // HLSL Change Begin - adjust this from T* to T&-like
+    if (getSema().getLangOpts().HLSL && ThisType.getTypePtr()->isPointerType())
+      return getSema().genereateHLSLThis(ThisLoc, ThisType, isImplicit);
+    // HLSL Change End - adjust this from T* to T&-like
     return new (getSema().Context) CXXThisExpr(ThisLoc, ThisType, isImplicit);
   }
 

diff --git a/tools/clang/test/HLSLFileCheck/hlsl/classes/ThisReference.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/classes/ThisReference.hlsl
@@ -0,0 +1,42 @@
+// RUN: %dxc -T cs_6_6 -E main -ast-dump %s | FileCheck %s
+// RUN: %dxc -T cs_6_6 -E main -DERROR_CASE 2>&1 %s | FileCheck -check-prefix=FAIL %s
+
+struct pair {
+  int First;
+  float Second;
+
+  int first() {
+    return this.First;
+  }
+
+  float second() {
+    return Second;
+  }
+
+#if ERROR_CASE
+  float third() {
+    // FAIL: error: operator is not supported
+    return this->First;
+  }
+#endif
+};
+
+[numthreads(1,1,1)]
+void main() {
+  pair Vals = {1, 2.0};
+  Vals.First = Vals.first();
+  Vals.Second = Vals.second();
+}
+
+// CHECK:      CXXMethodDecl 0x{{[0-9a-zA-Z]+}} <line:8:3, line:10:3> line:8:7 used first 'int ()'
+// CHECK-NEXT: `-CompoundStmt 0x{{[0-9a-zA-Z]+}} <col:15, line:10:3>
+// CHECK-NEXT: `-ReturnStmt 0x{{[0-9a-zA-Z]+}} <line:9:5, col:17>
+// CHECK-NEXT: `-ImplicitCastExpr 0x{{[0-9a-zA-Z]+}} <col:12, col:17> 'int' <LValueToRValue>
+// CHECK-NEXT: `-MemberExpr 0x{{[0-9a-zA-Z]+}} <col:12, col:17> 'int' lvalue .First 0x{{[0-9a-zA-Z]+}}
+// CHECK-NEXT: `-CXXThisExpr 0x{{[0-9a-zA-Z]+}} <col:12> 'pair' lvalue this
+// CHECK-NEXT: `-CXXMethodDecl 0x{{[0-9a-zA-Z]+}} <line:12:3, line:14:3> line:12:9 used second 'float ()'
+// CHECK-NEXT: `-CompoundStmt 0x{{[0-9a-zA-Z]+}} <col:18, line:14:3>
+// CHECK-NEXT: `-ReturnStmt 0x{{[0-9a-zA-Z]+}} <line:13:5, col:12>
+// CHECK-NEXT: `-ImplicitCastExpr 0x{{[0-9a-zA-Z]+}} <col:12> 'float' <LValueToRValue>
+// CHECK-NEXT: `-MemberExpr 0x{{[0-9a-zA-Z]+}} <col:12> 'float' lvalue .Second 0x{{[0-9a-zA-Z]+}}
+// CHECK-NEXT: `-CXXThisExpr 0x{{[0-9a-zA-Z]+}} <col:12> 'pair' lvalue this
diff --git a/tools/clang/test/HLSLFileCheck/hlsl/template/ThisAccess.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/template/ThisAccess.hlsl
@@ -0,0 +1,48 @@
+// RUN: %dxc -T cs_6_6 -E main -enable-templates -ast-dump %s | FileCheck %s
+// RUN: %dxc -T cs_6_6 -E main -HV 2021 -ast-dump %s | FileCheck %s
+
+template<typename K, typename V>
+struct pair {
+  K First;
+  V Second;
+
+  K first() {
+    return this.First;
+  }
+
+  V second() {
+    return Second;
+  }
+};
+
+[numthreads(1,1,1)]
+void main() {
+  pair<int, float> Vals = {1, 2.0};
+  Vals.First = Vals.first();
+  Vals.Second = Vals.second();
+}
+
+// CHECK:      CXXMethodDecl 0x{{[0-9a-zA-Z]+}} <line:9:3, line:11:3> line:9:5 first 'K ()'
+// CHECK-NEXT: `-CompoundStmt 0x{{[0-9a-zA-Z]+}} <col:13, line:11:3>
+// CHECK-NEXT: `-ReturnStmt 0x{{[0-9a-zA-Z]+}} <line:10:5, col:17>
+// CHECK-NEXT: `-CXXDependentScopeMemberExpr 0x{{[0-9a-zA-Z]+}} <col:12, col:17> '<dependent type>' lvalue
+// CHECK-NEXT: `-CXXThisExpr 0x{{[0-9a-zA-Z]+}} <col:12> 'pair<K, V>' lvalue this
+// CHECK-NEXT: CXXMethodDecl 0x{{[0-9a-zA-Z]+}} <line:13:3, line:15:3> line:13:5 second 'V ()'
+// CHECK-NEXT: `-CompoundStmt 0x{{[0-9a-zA-Z]+}} <col:14, line:15:3>
+// CHECK-NEXT: `-ReturnStmt 0x{{[0-9a-zA-Z]+}} <line:14:5, col:12>
+// CHECK-NEXT: `-MemberExpr 0x{{[0-9a-zA-Z]+}} <col:12> 'V' lvalue .Second 0x{{[0-9a-zA-Z]+}}
+// CHECK-NEXT: `-CXXThisExpr 0x{{[0-9a-zA-Z]+}} <col:12> 'pair<K, V>' lvalue this
+
+
+// CHECK:      CXXMethodDecl 0x{{[0-9a-zA-Z]+}} <line:9:3, line:11:3> line:9:5 used first 'int ()'
+// CHECK-NEXT: `-CompoundStmt 0x{{[0-9a-zA-Z]+}} <col:13, line:11:3>
+// CHECK-NEXT: `-ReturnStmt 0x{{[0-9a-zA-Z]+}} <line:10:5, col:17>
+// CHECK-NEXT: `-ImplicitCastExpr 0x{{[0-9a-zA-Z]+}} <col:12, col:17> 'int':'int' <LValueToRValue>
+// CHECK-NEXT: `-MemberExpr 0x{{[0-9a-zA-Z]+}} <col:12, col:17> 'int':'int' lvalue .First 0x{{[0-9a-zA-Z]+}}
+// CHECK-NEXT: `-CXXThisExpr 0x{{[0-9a-zA-Z]+}} <col:12> 'pair<int, float>' lvalue this
+// CHECK-NEXT: `-CXXMethodDecl 0x{{[0-9a-zA-Z]+}} <line:13:3, line:15:3> line:13:5 used second 'float ()'
+// CHECK-NEXT: `-CompoundStmt 0x{{[0-9a-zA-Z]+}} <col:14, line:15:3>
+// CHECK-NEXT: `-ReturnStmt 0x{{[0-9a-zA-Z]+}} <line:14:5, col:12>
+// CHECK-NEXT: `-ImplicitCastExpr 0x{{[0-9a-zA-Z]+}} <col:12> 'float':'float' <LValueToRValue>
+// CHECK-NEXT: `-MemberExpr 0x{{[0-9a-zA-Z]+}} <col:12> 'float':'float' lvalue .Second 0x{{[0-9a-zA-Z]+}}
+// CHECK-NEXT: `-CXXThisExpr 0x{{[0-9a-zA-Z]+}} <col:12> 'pair<int, float>' lvalue this