Skip to content

std.uni struct Grapheme mem fun signatures: Replace some @trusted by @safe;... #6104

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
carblue wants to merge 1 commit into from
Closed

std.uni struct Grapheme mem fun signatures: Replace some @trusted by @safe;... #6104

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions std/uni.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7395,7 +7395,7 @@ if (isInputRange!Range && is(Unqual!(ElementType!Range) == dchar))

See_Also: $(LREF decodeGrapheme), $(LREF graphemeStride)
+/
@trusted struct Grapheme
@safe struct Grapheme
{
import std.exception : enforce;
import std.traits : isDynamicArray;
Expand All @@ -7417,7 +7417,7 @@ public:
}

/// Gets a $(CODEPOINT) at the given index in this cluster.
dchar opIndex(size_t index) const pure nothrow @nogc
dchar opIndex(size_t index) const pure nothrow @nogc @trusted
{
assert(index < length);
return read24(isBig ? ptr_ : small_.ptr, index);
Copy link
Copy Markdown
Contributor

@wilzbach wilzbach Feb 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While it doesn't help here, are you aware of https://dlang.org/changelog/pending.html#ptr-safe-end-of-deprecation?

Copy link
Copy Markdown
Contributor Author

@carblue carblue Feb 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep!

Expand All @@ -7430,7 +7430,7 @@ public:
Use of this facility may invalidate grapheme cluster,
see also $(LREF Grapheme.valid).
+/
void opIndexAssign(dchar ch, size_t index) pure nothrow @nogc
void opIndexAssign(dchar ch, size_t index) pure nothrow @nogc @trusted
{
assert(index < length);
write24(isBig ? ptr_ : small_.ptr, ch, index);
Expand All @@ -7453,13 +7453,13 @@ public:
Warning: Invalidates when this Grapheme leaves the scope,
attempts to use it then would lead to memory corruption.
+/
SliceOverIndexed!Grapheme opSlice(size_t a, size_t b) pure nothrow @nogc
SliceOverIndexed!Grapheme opSlice(size_t a, size_t b) pure nothrow @nogc return
{
return sliceOverIndexed(a, b, &this);
}

/// ditto
SliceOverIndexed!Grapheme opSlice() pure nothrow @nogc
SliceOverIndexed!Grapheme opSlice() pure nothrow @nogc return
{
return sliceOverIndexed(0, length, &this);
}
Expand All @@ -7478,7 +7478,7 @@ public:

See_Also: $(LREF Grapheme.valid)
+/
ref opOpAssign(string op)(dchar ch)
ref opOpAssign(string op)(dchar ch) @trusted
{
static if (op == "~")
{
Copy link
Copy Markdown
Contributor

@wilzbach wilzbach Feb 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It we can use stdx.checked in general - it's just not @safe by default atm, but there's #5928

Expand Down Expand Up @@ -7560,7 +7560,7 @@ public:
return r.length == 0;
}

this(this) pure @nogc nothrow
this(this) pure @nogc nothrow @trusted
Copy link
Copy Markdown
Contributor

@wilzbach wilzbach Feb 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);

https://run.dlang.io/is/iZGDzT

Copy link
Copy Markdown
Member

@n8sh n8sh Feb 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't use it here because it's not pure.

Copy link
Copy Markdown
Contributor Author

@carblue carblue Feb 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

@carblue carblue Feb 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

pragma(msg, "std.uni.d:Grapheme.this(this):", __LINE__, " ", typeof(&Grapheme.__postblit));

{
import core.exception : onOutOfMemoryError;
import core.memory : pureMalloc;
Expand All @@ -7578,7 +7578,7 @@ public:
}
}

~this() pure @nogc nothrow
~this() pure @nogc nothrow @trusted
{
import core.memory : pureFree;
Copy link
Copy Markdown
Contributor

@wilzbach wilzbach Feb 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

if (isBig)
Expand Down Expand Up @@ -7612,7 +7612,7 @@ private:
}
}

void convertToBig() pure @nogc nothrow
void convertToBig() pure @nogc nothrow @trusted
Copy link
Copy Markdown
Contributor

@wilzbach wilzbach Feb 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stdx.allocator to the help!

Copy link
Copy Markdown
Contributor Author

@carblue carblue Feb 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for Your review and helpful hints!
Currently I won't touch function bodies and be focussed on function/struct signatures for -dip1000 compliance.
@wilzbach Please close this PR, it will be integrated in an update for PR #6041 as there are overlaps.

{
import core.exception : onOutOfMemoryError;
import core.memory : pureMalloc;
Expand Down