Skip to content

Avoid calling a qualified constructor in Cycle.#1852

Merged
andralex merged 1 commit intodlang:masterfrom
dnadlinger:cycle-qualified-constructor
Jan 13, 2014
Merged

Avoid calling a qualified constructor in Cycle.#1852
andralex merged 1 commit intodlang:masterfrom
dnadlinger:cycle-qualified-constructor

Conversation

@dnadlinger
Copy link
Contributor

This is actually invalid code, as typeof(this) == inout(this),
but Cycle has no inout constructor. This wasn't previously
detected, as the constructor was mistakenly flagged as creating
an unique object where qualifiers can be disregarded. (The
reference parameter is leaked into the Cycle instance, so
this is obviously not the case.)

This is actually invalid code, as typeof(this) == inout(this),
but Cycle has no inout constructor. This wasn't previously
detected, as the constructor was mistakenly flagged as creating
an unique object where qualifiers can be disregarded. (The
reference parameter is leaked into the Cycle instance, so
this is obviously not the case.)
@andralex
Copy link
Member

Auto-merge toggled on

@andralex
Copy link
Member

thanks!

andralex added a commit that referenced this pull request Jan 13, 2014
Avoid calling a qualified constructor in Cycle.
@andralex andralex merged commit 1f17f63 into dlang:master Jan 13, 2014
@monarchdodra
Copy link
Collaborator

I suppose that the fact that this used to compile was wrong?

void main()
{
    int[3] arr; 
    immutable c = cycle(arr);
}

Because here, immutable c references a pointer to mutable data... Will the new qualified constructors address this?

Also, I'm wondering if the new code is actually correct. The intent was to preserve the returned type's const-ness; the new code strips it. Looking at it a bit deeper, I realize the reason it currently works is thanks (because?) of a cast I put in here. It may be better to strip that inout entirely, I introduced it something like last week. No point of keeping it if its wrong.

@dnadlinger dnadlinger deleted the cycle-qualified-constructor branch January 13, 2014 12:00
@dnadlinger
Copy link
Contributor Author

@monarchdodra: Your example is addressed by my DMD pull request. The qualified constructor improvements would only make it easier to write a constructor that does work in the immutable case.

And thanks for pointing out the bug – I guess that's what one deserves for not writing a unit test…

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.

3 participants