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

fix issue 8737 Unqual for AA #822

Merged
merged 1 commit into from Oct 4, 2012
Merged

fix issue 8737 Unqual for AA #822

merged 1 commit into from Oct 4, 2012

Conversation

monarchdodra
Copy link
Collaborator

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

Unqual did not work with AAs.

This is mostly because an AA with a const KeyType is considered non-mutable, even if the AA's general type is not const. This fixes this by Unqualing the Key Type directly.

Examples in the unittests. Non of these unit tests worked without the fix.

Pre-required for 8709 http://d.puremagic.com/issues/show_bug.cgi?id=8709, in pull request...

823 fix issue 8709, from documentation
#823

andralex added a commit that referenced this pull request Oct 4, 2012
fix issue 8737 Unqual for AA
@andralex andralex merged commit 4bd3601 into dlang:master Oct 4, 2012
@andralex
Copy link
Member

andralex commented Oct 4, 2012

thanks

@9rnsr
Copy link
Contributor

9rnsr commented Oct 5, 2012

Wait, why is this merged!?
Unqual template removes just only top qualifier. Then, the top qualifier of associative array type and the qualifier of its value type is different.
Unqual!(const(V[K])) should be const(V)[K], should not be V[K].

@monarchdodra
Copy link
Collaborator Author

Looking into it.

@9rnsr
Copy link
Contributor

9rnsr commented Oct 5, 2012

I've seen the unittest and changed Unqual implementation.
Now your change breaks the behavior of Unqual which was consistent.

@9rnsr
Copy link
Contributor

9rnsr commented Oct 5, 2012

And more, you should not have fixed bug 8705 by changing Unqual behavior. For the purpose, you should have made private template.

@monarchdodra
Copy link
Collaborator Author

Yes, you are correct actually.

I figured out I did not actually need to change unqual. Give me 5 minutes?

@monarchdodra
Copy link
Collaborator Author

#840

still waiting for D2 tester, but that should fix 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
3 participants