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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
