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 7019 - implicit constructors are inconsistently allowed #1213

Merged
merged 1 commit into from
Nov 11, 2012

Conversation

9rnsr
Copy link
Contributor

@9rnsr 9rnsr commented Oct 24, 2012

http://d.puremagic.com/issues/show_bug.cgi?id=7019

From the @andralex's reply , we can sure that it is an expected feature.

}
}

S7019 rt_gs = 2;
Copy link
Member

Choose a reason for hiding this comment

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

unusual indent - vertical align doesn't quite justify it

Copy link
Member

Choose a reason for hiding this comment

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

should also add auto r_gs2 = S7019(1); if not present elsewhere

Copy link
Contributor Author

Choose a reason for hiding this comment

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

unusual indent - vertical align doesn't quite justify it

Will fix.

should also add auto r_gs2 = S7019(1); if not present elsewhere

Auto declaration, which doesn't have an explicit type, is irrelevant with the bug 7019.

@9rnsr
Copy link
Contributor Author

9rnsr commented Oct 24, 2012

Fixed indentation in the test suite code.

@donc
Copy link
Collaborator

donc commented Oct 30, 2012

LGTM

@WalterBright
Copy link
Member

In the 7019 bug report, Andrei points out the reason this enhancement is a bad idea. I think his reasoning is sound and we should go with his judgement.

@ghost
Copy link

ghost commented Nov 10, 2012

I think he only implemented the global assignment syntax A a = 5;, there are no test-cases for implicit function calls.

@andralex
Copy link
Member

Yah, this pull request is actually fine. I'll go ahead and merge it.

andralex added a commit that referenced this pull request Nov 11, 2012
Issue 7019 - implicit constructors are inconsistently allowed
@andralex andralex merged commit 2daf24f into dlang:master Nov 11, 2012
@denis-sh
Copy link
Contributor

It's a major syntax change as previously one can be sure S s = { 5 }; will not call any constructors. And it complicates things because constructor isn't called in this particular case:

struct S
{
    void* p;
    this(const S) { }
}

void main()
{
    const S cs;
    S s = cs; // Error: cannot implicitly convert expression (cs) of type const(S) to S
}

Please, add comments about how compiler worked before and how it works no to changelog for such changes as it isn't obvious at all.

@9rnsr
Copy link
Contributor Author

9rnsr commented Nov 15, 2012

Implicit constructor call works just only when:

  1. The declaration has an explicit type T, and it is a struct type which has one or more user-defined constructors.
  2. The initializer should have a distinct type U from T (in here, distinct means same as !is(Unqual!T == Unqual!U)).

If all conditions satisfied, the declaration T t = u; is translated to T t = T(u); implicitly.


    const S cs;
    S s = cs; // Error: cannot implicitly convert expression (cs) of type const(S) to S

It does not satisfy the condition 2, and it should call a built-in generated postblit constructor.
But, S has a mutable indirection, which can access through the field p, and calling built-in postblit will break type system correctness. Therefore copying const(S) to S is rejected.

@denis-sh
Copy link
Contributor

Implicit constructor call works just only when:

Thanks for the rules.

It does not satisfy the condition 2, and it should call a built-in generated postblit constructor.
But, S has a mutable indirection, which can access through the field p, and calling built-in postblit will break type system correctness. Therefore copying const(S) to S is rejected.

Thanks, but I know that. I just wanted to show that the rules aren't obvious.

The declaration has an explicit type T

The problem is I don't know how do you define explicit. This is how I define it but it doesn't satisfy the compiler:

struct S
{
    this(int) { }
}

struct S2
{
    S s;
}

void f(S s) { }

void main()
{
    S s = 5; // ok, explicit
    static assert(!__traits(compiles, f(5))); // ok, imlicit
    static assert(!__traits(compiles, { S2 s2 = 5; })); // ok, imlicit
    static assert(!__traits(compiles, { S2 s2 = S2(5); })); // ok, imlicit
    static assert(!__traits(compiles, { S2 s2 = { 5 }; })); // fails, but also imlicit
    static assert(!__traits(compiles, { S2 s2 = { s: 5 }; })); // fails, but also imlicit
}

@9rnsr
Copy link
Contributor Author

9rnsr commented Nov 15, 2012

The problem is I don't know how do you define explicit. This is how I define it but it doesn't satisfy the compiler:

Yes, I know such current inconsistencies. On ArrayInitializer and StructInitializer elelemts, the implicit ctor call doesn't work in current.
And I think they should work. Former is already reported as bug 8864 by bearophile, but latter is yet not reported, AFAIK.

@denis-sh
Copy link
Contributor

Yes, I know such current inconsistencies. On ArrayInitializer and StructInitializer elelemts, the implicit ctor call doesn't work in current.
And I think they should work.

Why f(5) shouldn't work but S2(5) should? What's the difference? Only the possible ability to overload f? It's something new for me to rely on possibility of overloading. Is it the only rule relying on it in the language or am I missing something?
IMHO, it doesn't look consistent. And it definitely not explicit so explicit word in that new rules have to be changed to an obviousness list of will/will not work for.

@9rnsr
Copy link
Contributor Author

9rnsr commented Nov 15, 2012

S2(5) is not a StructInitializer, it is a StructLiteralExp. So, Both f(5) and S2(5) should not be allowed IMO.

In my brain, followings should be allowed.

BigInt[] arr = [1,2];  // [1,2] is ArrayInitializer, so be translated to [BigInt(1), BigInt(2)]
struct S { BigInt n; }
S s = { 1 };  // { 1 } is StructInitializer, so will be translated to { BigInt(1) }
S s = { n:1 };  // same as above, and will be translated to { n:BigInt(1) }

@denis-sh
Copy link
Contributor

I don't see why you are making such difference between S(1) and S s = { 1 } as both mention type S but not BigInt.

By the way, in my mind: I like you first explicit rule and explicit for me is that "the tipe for which a constructor will be called must be explicitly mentioned" i.e.:

BigInt[] arr = [1,2]; // explicit
struct S { BigInt n; }
S s = { 1 }; // implicit, no `BigInt` here

@9rnsr
Copy link
Contributor Author

9rnsr commented Nov 15, 2012

S s = { 1 }; // implicit, no `BigInt` here

Hmm, your point out is good. It seems better that StructInitializer don't support it.

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

Successfully merging this pull request may close these issues.

5 participants