Implement `auto ref` that can receive both lvalue and rvalue, even if the function is template or not. #1019

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
6 participants
@9rnsr
Member

9rnsr commented Jun 20, 2012

For rvalue argument, compiler will assign it to a temporary variable, and get its reference.
Change ref parameter semantics that can receive only pure lvalues, so users cannot pass struct-literals and constructions with ref.
Then opEquals in struct should have the signature:
bool opEquals(const auto ref typeof(this) rhs) const;
instead of:
bool opEquals(const ref typeof(this) rhs) const;

Note:
The current mangling character for auto ref is same as for ref, it is 'K'.
So overloading of auto ref and ref function is disallowed by the linker.

@MartinNowak

This comment has been minimized.

Show comment Hide comment
@MartinNowak

MartinNowak Jun 30, 2012

Member

The current mangling character for auto ref is same as for ref, it is 'K'.

  • It would be better to have a separate letter so that the information can be retrieved from the ABI.
    It also seems more future proof if auto ref semantics evolved.

So overloading of auto ref and ref function is disallowed by the linker.

  • This should be checked in the front end.
  • This pull will collide with void foo(T)(auto ref T t) if (__traits(isRef, t)).

Overall this is a good direction.

Member

MartinNowak commented Jun 30, 2012

The current mangling character for auto ref is same as for ref, it is 'K'.

  • It would be better to have a separate letter so that the information can be retrieved from the ABI.
    It also seems more future proof if auto ref semantics evolved.

So overloading of auto ref and ref function is disallowed by the linker.

  • This should be checked in the front end.
  • This pull will collide with void foo(T)(auto ref T t) if (__traits(isRef, t)).

Overall this is a good direction.

@9rnsr

This comment has been minimized.

Show comment Hide comment
@9rnsr

9rnsr Jun 30, 2012

Member

The current mangling character for auto ref is same as for ref, it is 'K'.

  • It would be better to have a separate letter so that the information can be retrieved from the ABI. It also seems more future proof if auto ref semantics evolved.

So overloading of auto ref and ref function is disallowed by the linker.

  • This should be checked in the front end.

I agree with this definitely.

  • This pull will collide with void foo(T)(auto ref T t) if (__traits(isRef, t)).

Yes. this pull changes the behavior of __traits(isRef), and it would return always true.

Member

9rnsr commented Jun 30, 2012

The current mangling character for auto ref is same as for ref, it is 'K'.

  • It would be better to have a separate letter so that the information can be retrieved from the ABI. It also seems more future proof if auto ref semantics evolved.

So overloading of auto ref and ref function is disallowed by the linker.

  • This should be checked in the front end.

I agree with this definitely.

  • This pull will collide with void foo(T)(auto ref T t) if (__traits(isRef, t)).

Yes. this pull changes the behavior of __traits(isRef), and it would return always true.

@yebblies

This comment has been minimized.

Show comment Hide comment
@yebblies

yebblies Oct 27, 2012

Member

Can an auto-ref function pointer/deltegate implicitly convert to ref?

@andralex Is this the behavior for auto ref you intended?

Member

yebblies commented Oct 27, 2012

Can an auto-ref function pointer/deltegate implicitly convert to ref?

@andralex Is this the behavior for auto ref you intended?

@9rnsr 9rnsr closed this Nov 25, 2012

@9rnsr 9rnsr reopened this Jan 3, 2013

@9rnsr

This comment has been minimized.

Show comment Hide comment
@9rnsr

9rnsr Jan 3, 2013

Member

Updated:

  • To resolve the mangled name collision, I selected KK for auto ref encoding.
  • Now auto ref is the most lesser matching against non-ref and ref.

To @yebblies : I yet not implement it because this is a basic proposal.

Member

9rnsr commented Jan 3, 2013

Updated:

  • To resolve the mangled name collision, I selected KK for auto ref encoding.
  • Now auto ref is the most lesser matching against non-ref and ref.

To @yebblies : I yet not implement it because this is a basic proposal.

@9rnsr

This comment has been minimized.

Show comment Hide comment
@9rnsr

9rnsr Jan 3, 2013

Member

This change will break, as intended, Phobos unittests for std.algorithm.forward.

Member

9rnsr commented Jan 3, 2013

This change will break, as intended, Phobos unittests for std.algorithm.forward.

@alexrp

This comment has been minimized.

Show comment Hide comment
@alexrp

alexrp Jan 3, 2013

Member

To resolve the mangled name collision, I selected KK for auto ref encoding.

Please also remember to send an update to the ABI document on dlang.org. Thanks!

Member

alexrp commented Jan 3, 2013

To resolve the mangled name collision, I selected KK for auto ref encoding.

Please also remember to send an update to the ABI document on dlang.org. Thanks!

+ assert(var(s ) == 1);
+ assert(var(S()) == 3);
+
+ //alias var!S Var; // cannot make alias to overload set

This comment has been minimized.

Show comment Hide comment
@ghost

ghost Jan 19, 2013

These should go into test/fail_compilation, or use !__traits(compiles) rather than just commenting it.

@ghost

ghost Jan 19, 2013

These should go into test/fail_compilation, or use !__traits(compiles) rather than just commenting it.

This comment has been minimized.

Show comment Hide comment
@9rnsr

9rnsr Jan 20, 2013

Member

I thought it is essentially a kind of enhancement, not a fail-compilation case. So this is just a note.

@9rnsr

9rnsr Jan 20, 2013

Member

I thought it is essentially a kind of enhancement, not a fail-compilation case. So this is just a note.

This comment has been minimized.

Show comment Hide comment
@MartinNowak

MartinNowak Jan 24, 2013

Member

So this is not related to auto ref, right?
Can we then get an enhancement request in Bugzilla for this.

@MartinNowak

MartinNowak Jan 24, 2013

Member

So this is not related to auto ref, right?
Can we then get an enhancement request in Bugzilla for this.

@ghost

View changes

src/func.c
- if (p->storageClass & (STCref | STCout))
+ if ((p->storageClass & (STCauto | STCref)) == (STCauto | STCref))
+ {
+ // 'ref' is specialized than 'auto ref'

This comment has been minimized.

Show comment Hide comment
@ghost

ghost Jan 19, 2013

specialized -> more specialized

@ghost

ghost Jan 19, 2013

specialized -> more specialized

This comment has been minimized.

Show comment Hide comment
@9rnsr

9rnsr Jan 20, 2013

Member

Thanks.

@9rnsr

9rnsr Jan 20, 2013

Member

Thanks.

+int var(T)( T s) { return 3; }
+
+// 'auto ref' parameter is mangled as same as 'ref' parameter.
+// So linker will raise "Previous Definition Different" error.

This comment has been minimized.

Show comment Hide comment
@MartinNowak

MartinNowak Jan 24, 2013

Member

plz update comment

@MartinNowak

MartinNowak Jan 24, 2013

Member

plz update comment

+
+ // keep right behavior: rvalues never matches to 'ref'
+ static assert(!__traits(compiles, baz(S1(1)) ));
+ static assert(!__traits(compiles, vaz(S2(1)) ));

This comment has been minimized.

Show comment Hide comment
@MartinNowak

MartinNowak Jan 24, 2013

Member

Those are the test cases for removing rvalue-refs?
Can we get a deprecation for this breaking change?

@MartinNowak

MartinNowak Jan 24, 2013

Member

Those are the test cases for removing rvalue-refs?
Can we get a deprecation for this breaking change?

@@ -108,7 +108,7 @@ int foo6(T...)(auto ref T x)
if (v == 10)
assert(__traits(isRef, x[i]));
else
- assert(!__traits(isRef, x[i]));
+ assert(__traits(isRef, x[i]));
result += v;

This comment has been minimized.

Show comment Hide comment
@MartinNowak

MartinNowak Jan 24, 2013

Member

You could probably remove the whole test case then.

@MartinNowak

MartinNowak Jan 24, 2013

Member

You could probably remove the whole test case then.

@Dgame

This comment has been minimized.

Show comment Hide comment
@Dgame

Dgame Jan 24, 2013

This change will break, as intended, Phobos unittests for std.algorithm.forward.

I believe the breaking code isn't necessary anymore if this pull is merged. Or am I wrong?

Dgame commented Jan 24, 2013

This change will break, as intended, Phobos unittests for std.algorithm.forward.

I believe the breaking code isn't necessary anymore if this pull is merged. Or am I wrong?

9rnsr added some commits Dec 31, 2012

Implement `auto ref` that can receive both lvalue and rvalue, even if…
… the function is template or not.

For rvalue argument, compiler will assign it to a temporary variable, and get its reference.

Then opEquals in struct should have the signature:
`bool opEquals(const auto ref typeof(this) rhs) const;`
instead of:
`bool opEquals(const ref typeof(this) rhs) const;`

For the mangling, `auto ref` is encoded to `KK`.

@9rnsr 9rnsr closed this Apr 4, 2013

@andralex

This comment has been minimized.

Show comment Hide comment
@andralex

andralex Apr 4, 2013

Owner

@9rnsr why did you close this? Was it superseded by other work? P.S. Thanks for the work on translating TDPL!

Owner

andralex commented Apr 4, 2013

@9rnsr why did you close this? Was it superseded by other work? P.S. Thanks for the work on translating TDPL!

@9rnsr

This comment has been minimized.

Show comment Hide comment
@9rnsr

9rnsr Apr 5, 2013

Member

From recent discussion in forum (http://forum.dlang.org/thread/ntsyfhesnywfxvzbemwc@forum.dlang.org), I think that removing current auto ref would not be good.

Thanks for the work on translating TDPL!
It's my pleasure.

Member

9rnsr commented Apr 5, 2013

From recent discussion in forum (http://forum.dlang.org/thread/ntsyfhesnywfxvzbemwc@forum.dlang.org), I think that removing current auto ref would not be good.

Thanks for the work on translating TDPL!
It's my pleasure.

@9rnsr

This comment has been minimized.

Show comment Hide comment
@9rnsr

9rnsr Apr 17, 2013

Member

Based on the forum discussion and DIP36, I opened a new pull request #1903.
I think it is much better than this.

Member

9rnsr commented Apr 17, 2013

Based on the forum discussion and DIP36, I opened a new pull request #1903.
I think it is much better than this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment