std.uni struct Grapheme mem fun signatures: Replace some @trusted by @safe;...#6104
std.uni struct Grapheme mem fun signatures: Replace some @trusted by @safe;...#6104carblue wants to merge 1 commit intodlang:masterfrom carblue:std_uni_struct_grapheme
Conversation
|
Thanks for your pull request, @carblue! We are looking forward to reviewing it, and you should be hearing from a maintainer soon. Some tips to help speed things up:
Bear in mind that large or tricky changes may require multiple rounds of review and revision. Please see CONTRIBUTING.md for more information. Bugzilla references
|
wilzbach
left a comment
wilzbach
left a comment
There was a problem hiding this comment.
I would suggest to cleanup the code to use stdx.allocator instead of slapping ugly @trusted over it.
| dchar opIndex(size_t index) const pure nothrow @nogc @trusted | ||
| { | ||
| assert(index < length); | ||
| return read24(isBig ? ptr_ : small_.ptr, index); |
There was a problem hiding this comment.
While it doesn't help here, are you aware of https://dlang.org/changelog/pending.html#ptr-safe-end-of-deprecation?
| ref opOpAssign(string op)(dchar ch) @trusted | ||
| { | ||
| static if (op == "~") | ||
| { |
| } | ||
|
|
||
| this(this) pure @nogc nothrow | ||
| this(this) pure @nogc nothrow @trusted |
There was a problem hiding this comment.
This should use stdx.allocator.makeArray - it's @safe
import std.experimental.allocator.mallocator : Mallocator;
import std.experimental.allocator : makeArray;
auto p = Mallocator.instance.makeArray!ubyte(20);
There was a problem hiding this comment.
Can't use it here because it's not pure.
There was a problem hiding this comment.
Use PR #6041 instead (this PR is included there and should be closed but isn't yet).
When I inject behind } on line 7579
pragma(msg, "std.uni.d:Grapheme.this(this):", LINE, " ", typeof(&Grapheme.__postblit));
the compiler emits: std.uni.d:Grapheme.this(this):7579 void function() pure nothrow @nogc @trusted
There was a problem hiding this comment.
pragma(msg, "std.uni.d:Grapheme.this(this):", __LINE__, " ", typeof(&Grapheme.__postblit));
| ~this() pure @nogc nothrow | ||
| ~this() pure @nogc nothrow @trusted | ||
| { | ||
| import core.memory : pureFree; |
There was a problem hiding this comment.
Go with dispose, instead:
import std.experimental.allocator.mallocator : Mallocator;
import std.experimental.allocator : makeArray, dispose;
alias alloc = Mallocator.instance;
auto p = alloc.makeArray!ubyte(20);
alloc.dispose(p);https://run.dlang.io/is/t2IMq2
but it needs to be fixed first (https://issues.dlang.org/show_bug.cgi?id=18347)
| } | ||
|
|
||
| void convertToBig() pure @nogc nothrow | ||
| void convertToBig() pure @nogc nothrow @trusted |
There was a problem hiding this comment.
stdx.allocator to the help!
4 participants
…preparation for -dip1000, issues #17961, #18110.
Changes apply to function signatures only and are trivial except referring to return attribute: Seems to be undocumented in DIP25/DIP1000, similar to https://dlang.org/spec/function.html#return-ref-parameters;
It's what template inference does for opSlice: "return attribute ensures the returned object will not outlive the Grapheme instance".
Without return, there are "-dip1000 errors" like this one: returning sliceOverIndexed(a, b, &this) escapes a reference to parameter this, perhaps annotate with return.