D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 15782 - [Reg 2.071-devel] Alias no longer strips qualifiers from user defined types
Summary: [Reg 2.071-devel] Alias no longer strips qualifiers from user defined types
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: phobos (show other issues)
Version: D2
Hardware: All All
: P1 regression
Assignee: No Owner
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-03-09 12:12 UTC by Martin Nowak
Modified: 2016-03-24 02:10 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Martin Nowak 2016-03-09 12:12:17 UTC
Since a week or so vibe.d's test suite is no longer compilable w/ the dmd-master.
A meta-programming test fails with
    static assert  ("(A, const(B), const(A))" == "(A, B)")
indicating that some compiler semantics changed.
https://github.com/rejectedsoftware/vibe.d/blob/0b4f51ef7a7bc880a7ce37356fcf0c0a5f4b19b5/source/vibe/internal/meta/codegen.d#L95
Comment 1 Kenji Hara 2016-03-11 11:40:55 UTC
Introduced in:
https://github.com/D-Programming-Language/phobos/pull/3985

Now the Alias template properly holds the aliased type qualifier. It would affect to staticIndexOf, Erase, EraseAll, NoDuplicates, Replace, ReplaceAll, and DerivedToFront behaviors at least.
Comment 2 John Colvin 2016-03-11 12:22:18 UTC
If that test had used builtin types it would never have passed, it was a bug in Alias when dealing with qualified user-defined types.

I understand that it's frustrating to have old code break, but having templates like Alias & NoDuplicates strip qualifiers for user-defined types but not builtins is definitely not an option. So we have 2 choices: it always strips or it never strips. The second seems vastly preferable to me.
Comment 3 John Colvin 2016-03-11 12:48:27 UTC
https://issues.dlang.org/show_bug.cgi?id=11577
Comment 4 Martin Nowak 2016-03-15 18:41:27 UTC
I think it would be OK to make the change if it were only for Alias (which was package protected and undocumented anyhow), but I don't like that those breakages just happen, and it's definitely not OK to change all the other functions in std.meta w/o a proper deprecation.

Also see issue 11480
Comment 5 John Colvin 2016-03-16 09:36:44 UTC
Apologies for not noticing the changes, I didn't notice the bug in the old implementation.

What is the way forward here? The old behaviour was unexpected (why remove qualifiers?), inconsistent (didn't happen for builtins) and undocumented. I would argue that it's a straightforward bug and we should just say it's now fixed, perhaps adding a warning and a special changelog entry.
Comment 6 Martin Nowak 2016-03-16 16:41:39 UTC
(In reply to John Colvin from comment #5)
> Apologies for not noticing the changes, I didn't notice the bug in the old
> implementation.

No problem, it took me 20 minutes to find the change (b/c it's more or less a compiler bug, see issue 11480).

> What is the way forward here? The old behaviour was unexpected (why remove
> qualifiers?), inconsistent (didn't happen for builtins) and undocumented. I
> would argue that it's a straightforward bug and we should just say it's now
> fixed, perhaps adding a warning and a special changelog entry.

But it's been like that since ages and might now subtly break metagen code (e.g. serialization frameworks). The amount of time necessary to debug that some binary protocol no longer works b/c the semantics of Replace were fixed justifies that we put some more effort into this.

I'd try sth. along this line, though it might raise too many errors.
----
deprecated("message about Alias")
template Alias(T) if (isAffected!T && !is(T == Unqual!T))
{
    alias Alias = Unqual!T;
}

template Alias(T) if (!isAffected!T || is(T == Unqual!T))
{
    alias Alias = T;
}
----
Comment 7 github-bugzilla 2016-03-20 18:47:30 UTC
Commits pushed to stable at https://github.com/D-Programming-Language/phobos

https://github.com/D-Programming-Language/phobos/commit/5fb53481fccbe800f083aeaa867d0dd541efbe67
fix Issue 15782 - Alias no longer strips qualifiers from user defined types

- introduce OldAlias for internal usage
- deprecate unqualifying of UDTs
- unfortunately deprecation message isn't shown in many use-cases

https://github.com/D-Programming-Language/phobos/commit/fed12920a6bfc6cbb37da8e7568cf555005632f1
Merge pull request #4094 from MartinNowak/fix15782

fix Issue 15782 - Alias no longer strips qualifiers from user defined types
Comment 8 github-bugzilla 2016-03-24 02:10:22 UTC
Commits pushed to master at https://github.com/D-Programming-Language/phobos

https://github.com/D-Programming-Language/phobos/commit/5fb53481fccbe800f083aeaa867d0dd541efbe67
fix Issue 15782 - Alias no longer strips qualifiers from user defined types

https://github.com/D-Programming-Language/phobos/commit/fed12920a6bfc6cbb37da8e7568cf555005632f1
Merge pull request #4094 from MartinNowak/fix15782