[PATCH] D135721: [HLSL] Added HLSL this as a reference

[Xiang Li via Phabricator via cfe-commits]

python3kgae added inline comments.


================
Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:179
+        AST, SourceLocation(),
+        Constructor->getThisType().getTypePtr()->getPointeeType(), true);
+    This->setValueKind(ExprValueKind::VK_LValue);
----------------
gracejennings wrote:
> python3kgae wrote:
> > gracejennings wrote:
> > > python3kgae wrote:
> > > > Should this be a reference type?
> > > Could you expand on the question? I'm not sure I understand what you're asking. The two changes in this file were made to update the previous RWBuffer implementation
> > The current code will create CXXThisExpr with the pointeeType.
> > I thought it should be a reference type of the pointeeType.
> > 
> > Like in the test,
> > CXXThisExpr 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> 'RWBuffer<element_type> *' implicit this
> > 
> > The type is RWBuffer<element_type> * before,
> > I expected this patch will change it to
> > RWBuffer<element_type> &.
> The change that makes it more reference like than c++ from:
> 
> `-MemberExpr 0x{{[0-9A-Fa-f]+}} <col:11, col:17> 'int' lvalue ->First 0x{{[0-9A-Fa-f]+}}`
> `-CXXThisExpr 0x{{[0-9A-Fa-f]+}} <col:11> 'Pair *' this`
> 
> to hlsl with this change
> 
> `-MemberExpr 0x{{[0-9A-Fa-f]+}} <col:11, col:16> 'int' lvalue .First 0x{{[0-9A-Fa-f]+}}`
> `-CXXThisExpr 0x{{[0-9A-Fa-f]+}} <col:11> 'Pair' lvalue this`
I guess we have to change clang codeGen for this anyway.

Not sure which has less impact for codeGen side,  lvalue like what is in the current patch or make it a lvalue reference? My feeling is lvalue reference might be eaiser.

Did you test what needs to change for clang codeGen on top of the current patch?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135721