phobos
phobos copied to clipboard
Make it possible to use RefCounted from @safe code
Reviews definitely required here.
I thought it through and I think this is memory safe. I tried to do nasty things with -preview=dip1000
and the compiler prevented me, so that's nice.
This is currently preventing std.file.dirEntries
from being @safe
.
Thanks for your pull request and interest in making D better, @atilaneves! We are looking forward to reviewing it, and you should be hearing from a maintainer soon. Please verify that your PR follows this checklist:
- My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
- My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
- I have provided a detailed rationale explaining my changes
- New or modified functions have Ddoc comments (with
Params:
andReturns:
)
Please see CONTRIBUTING.md for more information.
If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.
Bugzilla references
Your PR doesn't reference any Bugzilla issue.
If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.
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 + phobos#8101"
@safe:
import std.stdio;
import std.typecons;
struct Container
{
int[] data;
}
void main ()
{
auto ptr = getPtr();
writeln(ptr);
const save = ptr.dup;
auto other = getPtr();
assert(save == ptr);
}
int[] getPtr ()
{
int[42] local;
return getPtr2(Container(local));
}
int[] getPtr2 (scope Container local)
{
RefCounted!Container rc = local;
return rc.refCountedPayload().data;
}
Compiles, print garbage, fails the assert.
Compiled with ./generated/osx/release/64/dmd -preview=dip1000 -run atila.d
and this PR.
@safe: import std.stdio; import std.typecons; struct Container { int[] data; } void main () { auto ptr = getPtr(); writeln(ptr); const save = ptr.dup; auto other = getPtr(); assert(save == ptr); } int[] getPtr () { int[42] local; return getPtr2(Container(local)); } int[] getPtr2 (scope Container local) { RefCounted!Container rc = local; return rc.refCountedPayload().data; }
Compiles, print garbage, fails the assert.
Compiled with
./generated/osx/release/64/dmd -preview=dip1000 -run atila.d
and this PR.
This is what reviews are for. Good work and thanks! Now to try and fix...
Now to try and fix...
As I mentioned in our previous discussion, given the nature of DIP1000, I have little hope that we can end up with a (truly) @safe
container that is usable. I would love nothing more than to be proven wrong though.
Now that SafeRefCounted
(#8368) has been merged, we can probably close this.