Bug 18755 – std.typecons.Rebindable breaks @safe-ty

Status
RESOLVED
Resolution
FIXED
Severity
critical
Priority
P1
Component
phobos
Product
D
Version
D2
Platform
All
OS
All
Creation time
2018-04-11T23:57:36Z
Last change time
2018-12-24T19:21:59Z
Keywords
bootcamp, safe
Assigned to
Eduard Staniloiu
Creator
David Nadlinger

Comments

Comment #0 by code — 2018-04-11T23:57:36Z
This compiles and runs with DMD 2.079: --- class Foo { auto opCast(T)() @system immutable pure nothrow { *(cast(uint*)0xdeadbeef) = 0xcafebabe; return T.init; } } void main() @safe { import std.typecons; auto r = Rebindable!(immutable Foo)(new Foo); // oops } ---
Comment #1 by johannes.loher — 2018-08-26T08:41:20Z
This also applies to std.typecons.UnqualRef because they use the same implementation which is responsible for this: --- class Foo { auto opCast(T)() @system immutable pure nothrow { *(cast(uint*)0xdeadbeef) = 0xcafebabe; return T.init; } } void main() @safe { import std.typecons; auto r = UnqualRef!(immutable Foo)(new Foo); // oops } --- The reason for the breakage is that they both implement this opAssign, which gets called by the constructor: --- void opAssign(T another) @trusted pure nothrow @nogc { stripped = cast(U) another; } --- Notice how this performs the cast in an @trusted context. The problem with this is that we actually need to do this, because casting is not @safe, but in this case we are only casting away immutability etc. which is actually verified manually to be safe in this case. To get around this, we would need to check, if there is a user defined opCast and if it is provided, check if is safe. If it is not safe, we can either decide to not instanciate Rebindable / UnqualRef, or we can instanciate versions of them, which are not @safe (i.e. we omit the @trusted attributes at the corresponding places). What do you think would be the correct thing to do in this case? Here is a basic implementation, which does the necessary checks: --- template hasElaborateCast(T) { import std.traits : hasMember; enum hasElaborateCast = hasMember!(T, "opCast"); } template hasSafeElaborateCast(T, U) { enum hasSafeElaborateCast = hasElaborateCast!T && __traits(compiles, () @safe{ T.init.opCast!U; }); } private mixin template RebindableCommon(T, U, alias This) if ((is(T == class) || is(T == interface) || isAssociativeArray!T) && (!hasElaborateCast!T || hasSafeElaborateCast!(T, U))) /* ... */ --- In this case, I put in as a template constraint, but as mentioned, we could also put it in static ifs to decide whether the implementation should be @trusted or not.
Comment #2 by edi33416 — 2018-12-17T14:18:36Z
Comment #3 by github-bugzilla — 2018-12-24T19:21:59Z
Commits pushed to master at https://github.com/dlang/phobos https://github.com/dlang/phobos/commit/dbca8521b5164c2ba95401d9bc784ec653e1a157 Fix Issue 18755 - std.typecons.Rebindable breaks @safe-ty https://github.com/dlang/phobos/commit/4435abf27ebe29bceea3a3666a361b64990f8fb7 Merge pull request #6808 from edi33416/issue_18755 Fix Issue 18755 - std.typecons.Rebindable breaks @safe-ty merged-on-behalf-of: Petar Kirov <[email protected]>