[PATCH] D159247: [HLSL] Cleanup support for `this` as an l-value

[Chris Bieneman via Phabricator via cfe-commits]

beanz created this revision.
beanz added reviewers: cor3ntin, aaron.ballman, bogner.
Herald added subscribers: Anastasia, ChuanqiXu, martong.
Herald added a reviewer: shafik.
Herald added a reviewer: NoQ.
Herald added a project: All.
beanz requested review of this revision.
Herald added a project: clang.

The goal of this change is to clean up some of the code surrounding
HLSL using CXXThisExpr as a non-pointer l-value. This change cleans up
a bunch of assumptions and inconsistencies around how the type of
`this` is handled through the AST and code generation.

This change is be mostly NFC for HLSL, and completely NFC for other
language modes.

This change introduces a new member to query for the this object's type
and seeks to clarify the normal usages of the this type.

With the introudction of HLSL to clang, CXXThisExpr may now be an
l-value and behave like a reference type rather than C++'s normal
method of it being an r-value of pointer type.

With this change there are now three ways in which a caller might need
to query the type of `this`:

- The type of the `CXXThisExpr`
- The type of the object `this` referrs to
- The type of the implicit (or explicit) `this` argument

This change codifies those three ways you may need to query
respectively as:

- CXXMethodDecl::getThisType()
- CXXMethodDecl::getThisObjectType()
- CXXMethodDecl::getThisArgType()

This change then revisits all uses of `getThisType()`, and in cases
where the only use was to resolve the pointee type, it replaces the
call with `getThisObjectType()`. In other cases it evaluates whether
the desired returned type is the type of the `this` expr, or the type
of the `this` function argument. The `this` expr type is used for
creating additional expr AST nodes and for member lookup, while the
argument type is used mostly for code generation.

Additionally some cases that used `getThisType` in simple queries could
be substituted for `getThisObjectType`. Since `getThisType` is
implemented in terms of `getThisObjectType` calling the later should be
more efficient if the former isn't needed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D159247

Files:
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/AST/ExprCXX.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/Analysis/Consumed.cpp
  clang/lib/CodeGen/CGBlocks.cpp
  clang/lib/CodeGen/CGCXXABI.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/lib/Sema/HLSLExternalSemaSource.cpp
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaExprMember.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/test/CodeGenHLSL/this-reference.hlsl

Index: clang/test/CodeGenHLSL/this-reference.hlsl
===
--- clang/test/CodeGenHLSL/this-reference.hlsl
+++ clang/test/CodeGenHLSL/this-reference.hlsl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -emit-llvm -disable-llvm-passes -o - -hlsl-entry main %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -emit-llvm -disable-llvm-passes -o - -hlsl-entry main %s -debug-info-kind=standalone -dwarf-version=4 | FileCheck %s
 
 struct Pair {
   int First;
@@ -26,3 +26,9 @@
   // CHECK-NEXT:  store i32 %call, ptr %First, align 4
   // CHECK-NEXT:  %call1 = call noundef float @"?getSecond@Pair@@QAAMXZ"(ptr noundef nonnull align 4 dereferenceable(8) %Vals)
   // CHECK-NEXT:  %Second = getelementptr inbounds %struct.Pair, ptr %Vals, i32 0, i32 1
+
+// CHECK: [[Pair:![0-9]+]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Pair"
+// CHECK: [[getFirst:![0-9]+]] = distinct !DISubprogram(name: "getFirst"
+// CHECK-SAME: scope: [[Pair]]
+// CHECK: [[FirstThis:![0-9]+]] = !DILocalVariable(name: "this", arg: 1, scope: [[getFirst]], type: [[thisType:![0-9]+]]
+// CHECK: [[thisType]] = !DIDerivedType(tag: DW_TAG_reference_type, baseType: [[Pair]], size: 32)
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -3880,7 +3880,7 @@
   break;
 
 case EXPR_CXX_THIS:
-  S = new (Context) CXXThisExpr(Empty);
+  S = CXXThisExpr::CreateEmpty(Context);
   break;
 
 case EXPR_CXX_THROW:
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ 

[PATCH] D159247: [HLSL] Cleanup support for `this` as an l-value

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

Full disclosure here, I need to write some more tests for this. It should be 
NFC for all languages other than HLSL, but I want to spend some time making 
sure that the tests adequately exercise the HLSL paths (which should also be 
mostly NFC).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159247/new/

https://reviews.llvm.org/D159247

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159247: [HLSL] Cleanup support for `this` as an l-value

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

Thanks for working on this.
This is going to conflict with deducing this (https://reviews.llvm.org/D140828) 
in interesting (ie, horrible) ways, so i guess I'll have to deal with that.
For people following along we are trying to fix nasty lambda regression 
https://reviews.llvm.org/D159126, for which the present patch is needed so we 
probably needs to land the current patch before `deducing this`, which is less 
than ideal.

I do wonder if we do really need 3 functions though.

We ought to have `this` being either a pointer (C++), or a reference (HLSL), 
and the type of the object it points to.
It's unclear to me that we could not  just have 2 methods.

- getThisObjectType
- getThisType (as implemented in getThisArgType)

What would happen if we would:

- Make `getThisType` return a reference type (for HLSL)
- Explicitely remove the reference  where we need to

If that would be too verbose, maybe renaming would add some clarity
`getThisReferenceType` and `getThisType` - this would mirror what we do for 
deducing this (`getFunctionObjectParameterReferenceType` and 
`getFunctionObjectParameterType`)

Ultimately we can have:

- This being a pointer in C++
- This a reference in HLSL
- The object parameter being a reference or a pointer depending on whether it 
is explicit or not in C++23, or depending on whether we are using HLSL

Which is... not confusing at all !


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159247/new/

https://reviews.llvm.org/D159247

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159247: [HLSL] Cleanup support for `this` as an l-value

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

Yea, we could do that approach. It will mean filtering about a few 
`getNonReferenceType()` calls, which is what I was trying to avoid. That said, 
it might be the better solution since it can be unconditional and will work for 
both deducing `this` and HLSL.

I'll test it out and post an update shortly.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159247/new/

https://reviews.llvm.org/D159247

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159247: [HLSL] Cleanup support for `this` as an l-value

[Chris Bieneman via Phabricator via cfe-commits]

beanz updated this revision to Diff 555118.
beanz added a comment.

Updating based on review feedback from @core3ntin. Thank you!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159247/new/

https://reviews.llvm.org/D159247

Files:
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/AST/ExprCXX.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/Analysis/Consumed.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/Sema/HLSLExternalSemaSource.cpp
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaExprMember.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/test/CodeGenHLSL/this-reference.hlsl

Index: clang/test/CodeGenHLSL/this-reference.hlsl
===
--- clang/test/CodeGenHLSL/this-reference.hlsl
+++ clang/test/CodeGenHLSL/this-reference.hlsl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -emit-llvm -disable-llvm-passes -o - -hlsl-entry main %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -emit-llvm -disable-llvm-passes -o - -hlsl-entry main %s -debug-info-kind=standalone -dwarf-version=4 | FileCheck %s
 
 struct Pair {
   int First;
@@ -26,3 +26,9 @@
   // CHECK-NEXT:  store i32 %call, ptr %First, align 4
   // CHECK-NEXT:  %call1 = call noundef float @"?getSecond@Pair@@QAAMXZ"(ptr noundef nonnull align 4 dereferenceable(8) %Vals)
   // CHECK-NEXT:  %Second = getelementptr inbounds %struct.Pair, ptr %Vals, i32 0, i32 1
+
+// CHECK: [[Pair:![0-9]+]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Pair"
+// CHECK: [[getFirst:![0-9]+]] = distinct !DISubprogram(name: "getFirst"
+// CHECK-SAME: scope: [[Pair]]
+// CHECK: [[FirstThis:![0-9]+]] = !DILocalVariable(name: "this", arg: 1, scope: [[getFirst]], type: [[thisType:![0-9]+]]
+// CHECK: [[thisType]] = !DIDerivedType(tag: DW_TAG_reference_type, baseType: [[Pair]], size: 32)
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -3880,7 +3880,7 @@
   break;
 
 case EXPR_CXX_THIS:
-  S = new (Context) CXXThisExpr(Empty);
+  S = CXXThisExpr::CreateEmpty(Context);
   break;
 
 case EXPR_CXX_THROW:
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -753,14 +753,15 @@
 
   if (!MightBeCxx11UnevalField && !isAddressOfOperand && !IsEnum &&
   isa(DC) && cast(DC)->isInstance()) {
-QualType ThisType = cast(DC)->getThisType();
+QualType ThisType = cast(DC)->getThisType().getNonReferenceType();
 
 // Since the 'this' expression is synthesized, we don't need to
 // perform the double-lookup check.
 NamedDecl *FirstQualifierInScope = nullptr;
 
 return CXXDependentScopeMemberExpr::Create(
-Context, /*This*/ nullptr, ThisType, /*IsArrow*/ true,
+Context, /*This*/ nullptr, ThisType,
+/*IsArrow*/ !Context.getLangOpts().HLSL,
 /*Op*/ SourceLocation(), SS.getWithLocInContext(Context), TemplateKWLoc,
 FirstQualifierInScope, NameInfo, TemplateArgs);
   }
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -689,7 +689,7 @@
 if (CMD->isStatic())
   Type.MemberType = FuncType::ft_static_member;
 else {
-  Type.This = CMD->getThisType()->getPointeeType();
+  Type.This = CMD->getThisObjectType();
   Type.MemberType = FuncType::ft_non_static_member;
 }
 Type.Func = CMD->getType()->castAs();
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -3532,14 +3532,14 @@
   case OR_Success: {
 // Record the standard conversion we used and the conversion function.
 CXXConstructorDecl *Constructor = cast(Best->Function);
-QualType ThisType = Constructor->getThisType();
+QualType ThisType = Constructor->getThisObjectType();
 // Initializer lists don't have conversions as such.
 User.Before.setAsIdentityConversion();
 User.HadMultipleCandidates = HadMultipleCandidates;
 User.ConversionFunction = Constructor;
 User.FoundConversionFunction = Best->FoundDecl;
 User.After.setAsIdentityConversion();
-User.After.setFromType(ThisType->ca

[PATCH] D159247: [HLSL] Cleanup support for `this` as an l-value

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

I would like @aaron.ballman to weight in, (he might not get back to you until 
next week), but overall, i think I'm pretty happy with this new direction.




Comment at: clang/include/clang/AST/DeclCXX.h:2174
 
+
   static QualType getThisType(const FunctionProtoType *FPT,

Whitespace only change


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159247/new/

https://reviews.llvm.org/D159247

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159247: [HLSL] Cleanup support for `this` as an l-value

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/include/clang/AST/DeclCXX.h:2174
 
+
   static QualType getThisType(const FunctionProtoType *FPT,

cor3ntin wrote:
> Whitespace only change
Spurious change.



Comment at: clang/lib/Sema/SemaExprMember.cpp:1903-1906
   /*OpLoc*/ SourceLocation(),
-  /*IsArrow*/ true,
+  /*IsArrow*/ !getLangOpts().HLSL,
   SS, TemplateKWLoc,
   /*FirstQualifierInScope*/ nullptr,

To be consistent with 
[bugprone-argument-comment](https://clang.llvm.org/extra/clang-tidy/checks/bugprone/argument-comment.html)



Comment at: clang/lib/Sema/SemaTemplate.cpp:763-765
+Context, /*This*/ nullptr, ThisType,
+/*IsArrow*/ !Context.getLangOpts().HLSL,
 /*Op*/ SourceLocation(), SS.getWithLocInContext(Context), 
TemplateKWLoc,




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159247/new/

https://reviews.llvm.org/D159247

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159247: [HLSL] Cleanup support for `this` as an l-value

[Chris Bieneman via Phabricator via cfe-commits]

beanz updated this revision to Diff 555380.
beanz added a comment.

Updating based on PR feedback.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159247/new/

https://reviews.llvm.org/D159247

Files:
  clang/include/clang/AST/ExprCXX.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/Analysis/Consumed.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/Sema/HLSLExternalSemaSource.cpp
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaExprMember.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/test/CodeGenHLSL/this-reference.hlsl

Index: clang/test/CodeGenHLSL/this-reference.hlsl
===
--- clang/test/CodeGenHLSL/this-reference.hlsl
+++ clang/test/CodeGenHLSL/this-reference.hlsl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -emit-llvm -disable-llvm-passes -o - -hlsl-entry main %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -emit-llvm -disable-llvm-passes -o - -hlsl-entry main %s -debug-info-kind=standalone -dwarf-version=4 | FileCheck %s
 
 struct Pair {
   int First;
@@ -26,3 +26,9 @@
   // CHECK-NEXT:  store i32 %call, ptr %First, align 4
   // CHECK-NEXT:  %call1 = call noundef float @"?getSecond@Pair@@QAAMXZ"(ptr noundef nonnull align 4 dereferenceable(8) %Vals)
   // CHECK-NEXT:  %Second = getelementptr inbounds %struct.Pair, ptr %Vals, i32 0, i32 1
+
+// CHECK: [[Pair:![0-9]+]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Pair"
+// CHECK: [[getFirst:![0-9]+]] = distinct !DISubprogram(name: "getFirst"
+// CHECK-SAME: scope: [[Pair]]
+// CHECK: [[FirstThis:![0-9]+]] = !DILocalVariable(name: "this", arg: 1, scope: [[getFirst]], type: [[thisType:![0-9]+]]
+// CHECK: [[thisType]] = !DIDerivedType(tag: DW_TAG_reference_type, baseType: [[Pair]], size: 32)
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -3880,7 +3880,7 @@
   break;
 
 case EXPR_CXX_THIS:
-  S = new (Context) CXXThisExpr(Empty);
+  S = CXXThisExpr::CreateEmpty(Context);
   break;
 
 case EXPR_CXX_THROW:
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -753,15 +753,16 @@
 
   if (!MightBeCxx11UnevalField && !isAddressOfOperand && !IsEnum &&
   isa(DC) && cast(DC)->isInstance()) {
-QualType ThisType = cast(DC)->getThisType();
+QualType ThisType = cast(DC)->getThisType().getNonReferenceType();
 
 // Since the 'this' expression is synthesized, we don't need to
 // perform the double-lookup check.
 NamedDecl *FirstQualifierInScope = nullptr;
 
 return CXXDependentScopeMemberExpr::Create(
-Context, /*This*/ nullptr, ThisType, /*IsArrow*/ true,
-/*Op*/ SourceLocation(), SS.getWithLocInContext(Context), TemplateKWLoc,
+Context, /*This=*/nullptr, ThisType,
+/*IsArrow=*/!Context.getLangOpts().HLSL,
+/*Op=*/SourceLocation(), SS.getWithLocInContext(Context), TemplateKWLoc,
 FirstQualifierInScope, NameInfo, TemplateArgs);
   }
 
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -689,7 +689,7 @@
 if (CMD->isStatic())
   Type.MemberType = FuncType::ft_static_member;
 else {
-  Type.This = CMD->getThisType()->getPointeeType();
+  Type.This = CMD->getThisObjectType();
   Type.MemberType = FuncType::ft_non_static_member;
 }
 Type.Func = CMD->getType()->castAs();
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -3532,14 +3532,14 @@
   case OR_Success: {
 // Record the standard conversion we used and the conversion function.
 CXXConstructorDecl *Constructor = cast(Best->Function);
-QualType ThisType = Constructor->getThisType();
+QualType ThisType = Constructor->getThisObjectType();
 // Initializer lists don't have conversions as such.
 User.Before.setAsIdentityConversion();
 User.HadMultipleCandidates = HadMultipleCandidates;
 User.ConversionFunction = Constructor;
 User.FoundConversionFunction = Best->FoundDecl;
 User.After.setAsIdentityConversion();
-User.After.setFrom

[PATCH] D159247: [HLSL] Cleanup support for `this` as an l-value

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM, I like this as a simplification, too!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159247/new/

https://reviews.llvm.org/D159247

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159247: [HLSL] Cleanup support for `this` as an l-value

[Chris Bieneman via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG400d3261a0da: [HLSL] Cleanup support for `this` as an 
l-value (authored by beanz).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159247/new/

https://reviews.llvm.org/D159247

Files:
  clang/include/clang/AST/ExprCXX.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/Analysis/Consumed.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/Sema/HLSLExternalSemaSource.cpp
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaExprMember.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/test/CodeGenHLSL/this-reference.hlsl

Index: clang/test/CodeGenHLSL/this-reference.hlsl
===
--- clang/test/CodeGenHLSL/this-reference.hlsl
+++ clang/test/CodeGenHLSL/this-reference.hlsl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -emit-llvm -disable-llvm-passes -o - -hlsl-entry main %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -emit-llvm -disable-llvm-passes -o - -hlsl-entry main %s -debug-info-kind=standalone -dwarf-version=4 | FileCheck %s
 
 struct Pair {
   int First;
@@ -26,3 +26,9 @@
   // CHECK-NEXT:  store i32 %call, ptr %First, align 4
   // CHECK-NEXT:  %call1 = call noundef float @"?getSecond@Pair@@QAAMXZ"(ptr noundef nonnull align 4 dereferenceable(8) %Vals)
   // CHECK-NEXT:  %Second = getelementptr inbounds %struct.Pair, ptr %Vals, i32 0, i32 1
+
+// CHECK: [[Pair:![0-9]+]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Pair"
+// CHECK: [[getFirst:![0-9]+]] = distinct !DISubprogram(name: "getFirst"
+// CHECK-SAME: scope: [[Pair]]
+// CHECK: [[FirstThis:![0-9]+]] = !DILocalVariable(name: "this", arg: 1, scope: [[getFirst]], type: [[thisType:![0-9]+]]
+// CHECK: [[thisType]] = !DIDerivedType(tag: DW_TAG_reference_type, baseType: [[Pair]], size: 32)
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -3876,7 +3876,7 @@
   break;
 
 case EXPR_CXX_THIS:
-  S = new (Context) CXXThisExpr(Empty);
+  S = CXXThisExpr::CreateEmpty(Context);
   break;
 
 case EXPR_CXX_THROW:
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -754,15 +754,16 @@
 
   if (!MightBeCxx11UnevalField && !isAddressOfOperand && !IsEnum &&
   isa(DC) && cast(DC)->isInstance()) {
-QualType ThisType = cast(DC)->getThisType();
+QualType ThisType = cast(DC)->getThisType().getNonReferenceType();
 
 // Since the 'this' expression is synthesized, we don't need to
 // perform the double-lookup check.
 NamedDecl *FirstQualifierInScope = nullptr;
 
 return CXXDependentScopeMemberExpr::Create(
-Context, /*This*/ nullptr, ThisType, /*IsArrow*/ true,
-/*Op*/ SourceLocation(), SS.getWithLocInContext(Context), TemplateKWLoc,
+Context, /*This=*/nullptr, ThisType,
+/*IsArrow=*/!Context.getLangOpts().HLSL,
+/*Op=*/SourceLocation(), SS.getWithLocInContext(Context), TemplateKWLoc,
 FirstQualifierInScope, NameInfo, TemplateArgs);
   }
 
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -689,7 +689,7 @@
 if (CMD->isStatic())
   Type.MemberType = FuncType::ft_static_member;
 else {
-  Type.This = CMD->getThisType()->getPointeeType();
+  Type.This = CMD->getThisObjectType();
   Type.MemberType = FuncType::ft_non_static_member;
 }
 Type.Func = CMD->getType()->castAs();
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -3532,14 +3532,14 @@
   case OR_Success: {
 // Record the standard conversion we used and the conversion function.
 CXXConstructorDecl *Constructor = cast(Best->Function);
-QualType ThisType = Constructor->getThisType();
+QualType ThisType = Constructor->getThisObjectType();
 // Initializer lists don't have conversions as such.
 User.Before.setAsIdentityConversion();
 User.HadMultipleCandidates = HadMultipleCandidates;
 User.ConversionFunction =