[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:
> > > > 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?
> > 
> With just the lvalue change in the current patch there should be no additional changes needed in clang CodeGen on top of the current patch
Since we already get the codeGen working with this patch,
it would be nice to have a codeGen test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135721