Make mmap_allocator allocate @safe#6405
Conversation
|
Thanks for your pull request and interest in making D better, @edi33416! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
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 referencesYour 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 locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub fetch digger
dub run digger -- build "master + phobos#6405" |
|
Looks fine to me |
| nothrow @trusted @nogc | ||
| void[] allocate(size_t bytes) shared | ||
| { | ||
| import core.sys.posix.sys.mman : mmap, MAP_ANON, PROT_READ, |
There was a problem hiding this comment.
On a second look, it might be better to use a @trusted lambda around mmap as @trusted on functions is a really anti-pattern that we soon might automatically check against.
| auto p = alloc.allocate(100); | ||
| assert(p.length == 100); | ||
| () nothrow @nogc { alloc.deallocate(p); }(); | ||
| () @trusted { alloc.deallocate(p); }(); |
There was a problem hiding this comment.
Also need to nullify p in the @trusted section. Else you have a dangling pointer in @safe code.
edi33416
force-pushed
the
mmap_allocator_allocate
branch
from
4add68a to
199bffb
Compare
|
@edi33416 have a look at dlang/druntime#1718 |
| auto p = alloc.allocate(100); | ||
| assert(p.length == 100); | ||
| () nothrow @nogc { alloc.deallocate(p); }(); | ||
| () @trusted { alloc.deallocate(p); p = null; }(); |
6 participants
This is a subset of the old #5879