Thread overview
[Issue 13506] @safe cartesianProduct.array
[Issue 13506] std.array.array is not @safe in some cases
Jun 07, 2016
Walter Bright
September 21, 2014
https://issues.dlang.org/show_bug.cgi?id=13506

hsteoh@quickfur.ath.cx changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hsteoh@quickfur.ath.cx

--- Comment #1 from hsteoh@quickfur.ath.cx ---
This bug appears unrelated to cartesianProduct; it's ultimately caused by a particular overload of Phobos internal function std.conv.emplaceImpl. It just so happens that other similar range-based calls to std.array.array end up in other, @safe instantiation paths, so the problem isn't (yet) exposed elsewhere (though you can probably find the same problem showing up elsewhere if you knew where to look).

The following Phobos patch fixes this problem, but I'm not ready to submit that as a PR yet because I'm not 100% confident that it's the correct fix (sticking @trusted on that call may be a bit too blunt of an instrument; probably some other static checks should be added to make sure that it is in fact @trust-worthy).

------snip------
diff --git a/std/array.d b/std/array.d
index b9012e4..8ad8cc3 100644
--- a/std/array.d
+++ b/std/array.d
@@ -2567,7 +2567,9 @@ if (isDynamicArray!A)
             else
                 auto ref uitem() @trusted nothrow @property { return
cast(Unqual!T)item; }

-            emplaceRef!(Unqual!T)(bigData[len], uitem);
+            () @trusted {
+                emplaceRef!(Unqual!T)(bigData[len], uitem);
+            }();

             //We do this at the end, in case of exceptions
             _data.arr = bigData;

------snip------

--
September 21, 2014
https://issues.dlang.org/show_bug.cgi?id=13506

hsteoh@quickfur.ath.cx changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |safe

--
September 21, 2014
https://issues.dlang.org/show_bug.cgi?id=13506

--- Comment #2 from hsteoh@quickfur.ath.cx ---
Oh, and just to be clear, this particular instantiation of emplaceRef is un-@safe because it eventually takes the address of an item and calls memcpy on it. It's not caused by something in cartesianProduct range being non-@safe.

--
September 21, 2014
https://issues.dlang.org/show_bug.cgi?id=13506

bearophile_hugs@eml.cc changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|@safe                       |std.array.array is not
                   |cartesianProduct.array      |@safe in some cases

--- Comment #3 from bearophile_hugs@eml.cc ---
(In reply to hsteoh from comment #1)
> This bug appears unrelated to cartesianProduct; it's ultimately caused by a particular overload of Phobos internal function std.conv.emplaceImpl.

Thank you. I change the issue title.

--
September 21, 2014
https://issues.dlang.org/show_bug.cgi?id=13506

--- Comment #4 from bearophile_hugs@eml.cc ---
(In reply to hsteoh from comment #1)

> The following Phobos patch fixes this problem, but I'm not ready to submit that as a PR yet because I'm not 100% confident that it's the correct fix

Currently creating a pull request seems the best way to receive comments and to have a chance to fix your idea. Here in Bugzilla it's less likely to receive comments on the code.

--
September 21, 2014
https://issues.dlang.org/show_bug.cgi?id=13506

--- Comment #5 from hsteoh@quickfur.ath.cx ---
Actually, it looks like it's probably the *wrong* fix, because if appender is used with a type with overloaded assignment operator, and the assign operator was un-@safe, then this patch will cause unsafe code to be called from @trusted code, so it will break @safety in a subtle but fairly horrible way.

--
September 21, 2014
https://issues.dlang.org/show_bug.cgi?id=13506

--- Comment #6 from bearophile_hugs@eml.cc ---
(In reply to hsteoh from comment #5)
> Actually, it looks like it's probably the *wrong* fix, because if appender is used with a type with overloaded assignment operator, and the assign operator was un-@safe, then this patch will cause unsafe code to be called from @trusted code, so it will break @safety in a subtle but fairly horrible way.

Yours was a first try, don't worry. Can you verify at compile time the safety of the assignment, and use a safe or a not safe nested lambda accordingly with a static if?

--
June 07, 2016
https://issues.dlang.org/show_bug.cgi?id=13506

Walter Bright <bugzilla@digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |bugzilla@digitalmars.com
         Resolution|---                         |WORKSFORME

--- Comment #7 from Walter Bright <bugzilla@digitalmars.com> ---
This works with the current compiler.

--