Made auto ref work for non template functions. #1428

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
5 participants
Contributor

jkrempus commented Jan 2, 2013

This implements auto ref parameters for non template functions so that local variables are created when the function is called with rvalue arguments, as discussed in this newsgroup thread.

Member

yebblies commented Jan 2, 2013

Expression::semantic returns a new expression. The usual pattern is e = e->semantic(sc);.

Semantic only needs to be called once, on the root of the expression tree.

Taking the address of the argument can be done in the glue layer, and I suspect this would be preferable. Not sure though. @WalterBright ?

Needs test cases.

Contributor

jkrempus commented Jan 2, 2013

Expression::semantic returns a new expression. The usual pattern is e = e->semantic(sc);.

Semantic only needs to be called once, on the root of the expression tree.

Fixed.

Needs test cases.

I've added compilable/auto_ref.d and runnable/auto_ref.d now.

@MartinNowak MartinNowak commented on an outdated diff Jan 2, 2013

src/expression.c
@@ -1154,6 +1154,19 @@ Type *functionParameters(Loc loc, Scope *sc, TypeFunction *tf,
}
if (p->storageClass & STCref)
{
+ if(p->storageClass & STCauto && !arg->isLvalue())
+ {
+ VarDeclaration* vd = new VarDeclaration(
+ loc, p->type, Identifier::generateId("rvalueRefParam"),
@MartinNowak

MartinNowak Jan 2, 2013

Member

The id should start with __.

Member

yebblies commented Jan 3, 2013

Looking at this it's possible the glue code will 'just work' if you remove the must-be-lvalue check for non-template auto ref parameters.

Member

9rnsr commented Jan 3, 2013

I've reopened similar pull request #1019. The difference is to remove current template auto ref completely.

Contributor

jkrempus commented Jan 3, 2013

I've reopened similar pull request #1019. The difference is to remove current template auto ref completely

I see. I'm closing this one then. No point in having two of them.

jkrempus closed this Jan 3, 2013

Dgame commented Jan 19, 2013

Maybe we should reopen this pull. It's ready to merge right now and solves the problem also.

@ghost

ghost commented Jan 19, 2013

#1019 is more thoroughly implemented and tested than this pull.

Dgame commented Jan 20, 2013

Could be. But do you think that #1019 is ready in the near future? I am not so sure because it's far more complex than this pull.

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