[PATCH] D49511: [Sema/Attribute] Check for noderef attribute

Leonard Chan via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 21 11:49:32 PDT 2018


leonardchan added a comment.

In https://reviews.llvm.org/D49511#1206267, @rsmith wrote:

> In https://reviews.llvm.org/D49511#1206265, @rsmith wrote:
>
> > In https://reviews.llvm.org/D49511#1194716, @leonardchan wrote:
> >
> > > @rsmith any more feedback on this current version? If it still looks incorrect to use the record this way, I don't mind simplifying it to work on lvalue to rvalue conversions without checking for a leading address space operation.
> >
> >
> > I've been thinking more about cleaner ways to implement this (and in particular, approaches that will provide more reasonable semantics in C++ -- allowing references to `noderef`, for example). We want to disallow operands of type `noderef T` to all operations by default, and only allow very specific operations on lvalues of type `noderef T` -- taking the address, performing member accesses, lvalue casts. The most natural way to get that effect would be to add a new form of placeholder type for a "dereferenced noderef" expression, that `CheckPlaceholderExpr` rejects, and that we add explicit support for in the contexts where such a construct is valid. (This is similar to how we handle overloaded function names and bound member function expressions in C++, for example.) (When we reach a context that "consumes" a dereferenced noderef expression, we'd need to go back and patch up its type, but I think that can be handled in a straightforward way.)
> >
> > I think we should also treat `noderef` more like a type qualifier: as an important example, if we have a pointer or reference to `noderef struct X`, then member access for a member of type `T` should give an lvalue of type `noderef T`.
>
>
> Sorry, I forgot to say: I do not think we need to do this for the initial version of this functionality. This is mostly about cleanly extending the functionality to cover more C++ constructs.


No problem. After this, I will make another patch that does this.


Repository:
  rC Clang

https://reviews.llvm.org/D49511





More information about the cfe-commits mailing list