[PATCH] rvalue reference implementation for C++0x

Russell Yanofsky russ@yanofsky.org
Sun Mar 11 02:58:00 GMT 2007


On Fri, 2007-03-09 at 16:26 -0500, Jason Merrill wrote:
> Mostly looks good.  A few comments:
> 
> Need a space before the ( in many of the uses of TYPE_REF_IS_RVALUE and 
> some other places.

Ok. There's a new patch attached which fixes this and some of the other
issues you mention.

> I'd rather not expose the concept of an rvalue reference to the back 
> end, its semantics should be encapsulated by the front end.  I think the 
> right answer is to have a wrapper for build_reference_type_* in the 
> front end to handle the extra semantics.
> 
> I'm not sure what the best coding strategy is for this.  One option 
> would be to establish a convention of putting the rvalue ref after the 
> non-rvalue ref in the TYPE_NEXT_REF_TO chain, or in some other field if 
> there are others free, so that the C++ front end function can call the 
> back end function and then use the result to look up the rvalue version.

It seems to me like everything you said is already implemented. Rvalue
reference types become part of the TYPE_NEXT_REF_TO chain just like
other variants of reference types. And build_reference_type() is the
front-end wrapper for the build_reference_type_for_mode() function.

That being said, I think there is an important simplification that can
be made that might address your concerns. The current
build_reference_type_* functions provided are:

  tree build_rval_reference_type_for_mode
    (tree to_type, enum machine_mode mode, bool can_alias_all,
     bool rval);

  tree build_reference_type_for_mode
    (tree to_type, enum machine_mode mode, bool can_alias_all);

  tree build_rval_reference_type
    (tree to_type, bool rval);

  tree build_reference_type
    (tree to_type);

I think it would be a very good idea to get rid of two of these
functions and just provide:

  tree build_reference_type_for_mode
    (tree to_type, bool rval, 
     enum machine_mode mode, bool can_alias_all);

  tree build_reference_type
    (tree to_type, bool rval);

The only reason I didn't do this already is that it would require
updating invocations of build_reference_type_for_mode and
build_reference_type all over the GCC code and substantially increase
the length of the patch, making it harder to review and maintain. If you
want, I can go back and make this change, either incorporating it into a
new version of the rvalue reference patch, or as a seperate follow up
patch.

> Please quote the reference collapsing rules where you implement them in 
> grokdeclarator and tsubst.

This is done in the attached patch. The comments that were in those
places before already referenced the relevant section and paragraph
numbers in the C++ standard, and I would argue that including those long
quotations increases, rather than decreases, the amount of time a person
needs to spend to read and understand that code. But it's a small issue.

> I'd change "valuedness" to "rvalueness" for clarity, as "valuedness" 
> could mean "the state of having a value".

Done in the attached patch.

> The test in compare_ics seems to check for cases where one conversion 
> has matching rvalueness and the other doesn't, but that isn't all the 
> rule in the working paper talks about: it also requires that for one ICS 
> to be better than the other, the two references must differ in 
> rvalueness, one converting to lvalue ref and the other to rvalue ref or 
> vice versa.

I don't understand what the difference is. Can you give an example of a
case compare_ics might not handle correctly?

> In lvalue_p_1, I'd clarify the INDIRECT_REF comment by noting that the 
> INDIRECT_REFs are just internal compiler representation, not part of the 
> language, so we have to look through them.

Done in the attached patch.

> Please send mail to cxx-abi-dev@codesourcery.com about allocating a new 
> type encoding for rvalue references.  Since this is part of C++0x, it 
> shouldn't be mangled using a vendor extension, it should be added to the 
> standard ABI.  Also, I'd expect it to be used instead of the "R" for an 
> lvalue reference, not in addition.

I'll go ahead and do this. Rvalue references have also been implemented
in the Metrowerks Codewarrior compiler, so it might be worth looking at
how that compiler does the mangling also. In the meantime do you want me
to change the patch? Could using a new character code interfere with
debugging tools? I don't even know what character code would make sense
to use for rvalue references..

> About the deduce_ref stuff: why not just add the REFERENCE_TYPE to arg 
> in type_unification_real rather than add another argument to unify?

I think you were looking at the Feb 21 patch instead of the Mar 9 patch.
The newer patch doesn't add an argument to unify(), it handles the case
in maybe_adjust_types_for_deduction().

-- 
-  Russell Yanofsky (PGP ID: 0x5FAA0216)
-  http://russ.yanofsky.org/
--
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ChangeLog
Type: text/x-changelog
Size: 2586 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20070311/5a3576d3/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch.txt
Type: text/x-patch
Size: 42004 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20070311/5a3576d3/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: tests.tar.bz2
Type: application/x-bzip-compressed-tar
Size: 18182 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20070311/5a3576d3/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20070311/5a3576d3/attachment.sig>


More information about the Gcc-patches mailing list