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

std.conv: Fix ambiguity when target has ctor and source has opCast #1864

Merged
1 commit merged into from Feb 16, 2014

Conversation

CyberShadow
Copy link
Member

{
struct T
{
this(S s) @safe pure { }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Place an assert in here. We're supposed to call opCast, right?

@monarchdodra
Copy link
Collaborator

I have just 1 question: There's an ambiguity between opCast and T().

You chose to default to opCast (I think). Apparently, dmd favors calling construction in this case. Shouldn't we default to calling construction too?

@CyberShadow
Copy link
Member Author

Apparently, dmd favors calling construction in this case.

For assignment? I think this is something from C++.

Shouldn't we default to calling construction too?

I chose to prefer opCast because its meaning seems closer to the idea of converting one type to another, thus better suitable for conv. But since they'll probably be synonymous in most cases I don't have a strong opinion. Do you think I should change it?

@monarchdodra
Copy link
Collaborator

For assignment? I think this is something from C++.

I'm thinking:

    S s;
    T t = s; //Calls constructor

For assignement, you need an explict cast or contruction anyways (unlike C++).

T t;
t = s; //Fails
T = cast(T)s; //OK
T = T(s); //OK

Do you think I should change it?

Given that conv does "whatever it takes to build a T from an S", I think it would make more to do the same as the compiler. IMO.

@CyberShadow
Copy link
Member Author

Right, I meant initialization, not assignment.

Updated.

@monarchdodra
Copy link
Collaborator

LGTM. Anyone else?

@ghost
Copy link

ghost commented Feb 15, 2014

Auto-merge toggled on

@ghost
Copy link

ghost commented Feb 15, 2014

LG.

ghost pushed a commit that referenced this pull request Feb 16, 2014
std.conv: Fix ambiguity when target has ctor and source has opCast
@ghost ghost merged commit 5a5f899 into dlang:master Feb 16, 2014
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants