Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 2803 - template + default argument = doesn't work #4261

Merged
merged 2 commits into from
Apr 18, 2015

Conversation

9rnsr
Copy link
Contributor

@9rnsr 9rnsr commented Jan 6, 2015

@ghost
Copy link

ghost commented Jan 6, 2015

I had a gut feeling you were going to work on this! Very very nice. :)

@WalterBright
Copy link
Member

This makes me uneasy. First, there's the (deleted) comment that default parameters do not participate in type deduction. I don't remember the reason for that decision. The next issue it the comment about a workaround for existing code - doesn't this make the language more complex than necessary?

/* Bugzilla 2803: Before the starting of type deduction from the function
* default arguments, lock the already deduced parameters into paramscope.
* It's necessary to avoid breaking some existing code. See the regression cases
* in `runnable/template9.d`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please show the cases being worked around here, rather than a reference to something in template9.d which is a 4000+ line file and would surely get out of sync with this comment regardless.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had just specified the issue 2803 test case. It's only 500 lines so finding it would be easy.
But OK, I added acceptable cases in the comment.

@9rnsr
Copy link
Contributor Author

9rnsr commented Jan 7, 2015

First, there's the (deleted) comment that default parameters do not participate in type deduction. I don't remember the reason for that decision.

I know the comment, but I couldn't understand the reason. As far as I see, issue 2803 is possible.

The next issue it the comment about a workaround for existing code - doesn't this make the language more complex than necessary?

This PR will add just only one more IFTI rule - if a default argument is used for the type deduction, it can refer already deduced types. For example:

auto foo(A, B)(A a, B b = A.stringof) {}
foo(1);

At the function parameter B b = A.stringof, B depends on the deduced type A (it's already deduced to int).
The example case was actually existing in earlier std.conv code.

@9rnsr 9rnsr force-pushed the fix2803 branch 3 times, most recently from 4c069ea to 66baefd Compare January 14, 2015 03:27
@MartinNowak
Copy link
Member

The next issue it the comment about a workaround for existing code

Seems to me, that existing code here means the newly added test cases.

doesn't this make the language more complex than necessary?

I'd say it makes the language simpler because default arguments should behave like normal arguments.

void foo(T)(T t = 0) {}

void main() {
    foo();
    foo(0);
}

And the possibility to refer to already deduced types matches default template arguments.

void foo(T, U = T.U)(T t, U u) {}

struct Foo {
    alias U = size_t;
}

void main() {
    foo(Foo(), 12);
}

@yebblies
Copy link
Member

yebblies commented Apr 8, 2015

This is failing with ddmd because of a goto-skips-init error.

@9rnsr
Copy link
Contributor Author

9rnsr commented Apr 8, 2015

@yebblies Thanks, updated.

@WalterBright
Copy link
Member

Auto-merge toggled on

WalterBright added a commit that referenced this pull request Apr 18, 2015
Issue 2803 - template + default argument = doesn't work
@WalterBright WalterBright merged commit 596aef6 into dlang:master Apr 18, 2015
@9rnsr 9rnsr deleted the fix2803 branch April 18, 2015 10:08
@MartinNowak
Copy link
Member

@WalterBright
Copy link
Member

Another regression: https://issues.dlang.org/show_bug.cgi?id=15668

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants