Fix Issue 21889 - __traits(isSame, Object, const Object) yields true

[BorisCarvajal]

[dlang-bot]

Thanks for your pull request, @BorisCarvajal!

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	21889	major	__traits(isSame, Object, const Object) yields true

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#12497"

[BorisCarvajal]

automem fails but a pull is ready. https://github.com/atilaneves/automem/pull/61

[thewilsonator]

as this is a major issue, feel free to retarget this to stable and then open a PR to merge stable back into master

[RazvanN7]

Since code out there depends on traits(isSame) ignoring qualifiers, this requires a deprecation cycle. We can suggest in the deprecation message to use Unqual for the types if the previous behavior is desired.

[RazvanN7]

ping @BorisCarvajal

[BorisCarvajal]

Since code out there depends on traits(isSame) ignoring qualifiers, this requires a deprecation cycle. We can suggest in the deprecation message to use Unqual for the types if the previous behavior is desired.

I think that code is depending on undefined behavior because I don't see any info in the spec about using a type inside isSame, the closest thing is comparing tuples.

[RazvanN7]

It's not exactly undefined behavior. It just doesn't appear in the spec, but the behavior is that it compares types without taking into account qualifiers. The last thing we want is to break people's code. If automem was not included in the testing pipeline and the author upgraded the compiler, the project would have failed to compile without any indication of what the reason is. We don't know how many projects are in this state out in the wild. It could be 1 or 2 or it could be dozens. We don't know and that is why we should (1) issue a deprecation warning any time 2 types are compared with isSame and (2) add a changelog entry that offers more information about this.

Edit: this should also be accompanied with a spec PR clarifying the behavior in the presence of types.

[RazvanN7]

cc @WalterBright @atilaneves

[atilaneves]

I defer to @WalterBright

[nordlow]

Can we have this merged, @WalterBright?

FYI, @andralex.

[RazvanN7]

@dkorpel What are your thoughts on this?

[dkorpel]

I don't use __traits(isSame), but it appears to be mostly used for comparing functions (e.g. "is this template parameter an identity function?") Using it for type comparison is unspecified, and we already have is(T == U) for that, so unless including it under isSame make sense in generic code, I would deprecate type comparisons in isSame.

@andralex I'm curious how the case in the bug report static assert(!__traits(isSame, Object, const Object)); came about, what was isSame being used for?

[nordlow]

I don't use __traits(isSame), but it appears to be mostly used for comparing functions (e.g. "is this template parameter an identity function?") Using it for type comparison is unspecified, and we already have is(T == U) for that, so unless including it under isSame make sense in generic code, I would deprecate type comparisons in isSame.

I would rather deprecate __traits(isSame) in favor of a generalized is(T == U) that covers all the sane cases of __traits(isSame, T, U).

[WalterBright]

I marked this as WONTFIX in https://issues.dlang.org/show_bug.cgi?id=21889 for reasons given there. Needs a strong compelling reason to reopen it, especially since this change breaks existing code.