Adding move-only for Dlang arrays #10781
Conversation
… the new unit tests from the std/array.d
|
Thanks for your pull request and interest in making D better, @GalaxyRider111! 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 run digger -- build "master + phobos#10781" |
| @@ -1293,7 +1546,7 @@ if (!is(immutable T == immutable bool)) | |||
| assert(a[].equal(["test"])); | |||
| } | |||
|
|
|||
| @safe unittest | |||
| @system unittest | |||
There was a problem hiding this comment.
Making previously memory safe operations unsafe is an unfortunate breakage.
There was a problem hiding this comment.
It's not just unfortunate, it's absolutely unacceptable. All operations that were @safe prior to this PR must remain @safe.
There was a problem hiding this comment.
Ok guys, sorry, I had a bunch of errors about @safe, and I have tried this workaround to see if it would work, and somehow, it really did
| @@ -1593,7 +1848,7 @@ if (!is(immutable T == immutable bool)) | |||
| } | |||
|
|
|||
| // https://issues.dlang.org/show_bug.cgi?id=11459 | |||
| @safe unittest | |||
| @system unittest | |||
| @@ -1612,7 +1867,7 @@ if (!is(immutable T == immutable bool)) | |||
| } | |||
|
|
|||
| // https://issues.dlang.org/show_bug.cgi?id=8282 | |||
| @safe unittest | |||
| @system unittest | |||
| @@ -1656,7 +1911,7 @@ if (!is(immutable T == immutable bool)) | |||
| assert(c.i == 42); //fails | |||
| } | |||
|
|
|||
| @safe unittest | |||
| @system unittest | |||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
Please trim this excessive whitespace to a single empty line.
|
|
||
|
|
||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
Please trim excessive whitespace to a single empty line.
|
|
||
|
|
||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
Please trim excessive whitespace to a single empty line.
| /* | ||
| unittest | ||
| { | ||
| // Test front and back | ||
| Array!NoCopy arr; | ||
| arr.insertBack(NoCopy(10)); | ||
| arr.insertBack(NoCopy(20)); | ||
| assert(arr.front.x == 10); | ||
| assert(arr.back.x == 20); | ||
| } | ||
|
|
||
| unittest | ||
| { | ||
| // Test insertFront | ||
| Array!NoCopy arr; | ||
| arr.insertFront(NoCopy(5)); | ||
| arr.insertFront(NoCopy(3)); | ||
| assert(arr.length == 2); | ||
| assert(arr[0].x == 3); | ||
| assert(arr[1].x == 5); | ||
| } | ||
|
|
||
| unittest | ||
| { | ||
| // Test clear | ||
| Array!NoCopy arr; | ||
| arr.insertBack(NoCopy(7)); | ||
| arr.insertBack(NoCopy(8)); | ||
| arr.clear(); | ||
| assert(arr.empty); | ||
| } | ||
|
|
||
| unittest | ||
| { | ||
| // Test popFront / popBack | ||
| Array!NoCopy arr; | ||
| arr.insertBack(NoCopy(100)); | ||
| arr.insertBack(NoCopy(200)); | ||
| arr.popFront(); | ||
| assert(arr.length == 1); | ||
| assert(arr.front.x == 200); | ||
|
|
||
| arr.popBack(); | ||
| assert(arr.empty); | ||
| } | ||
| */ |
There was a problem hiding this comment.
Is there a reason these unittests are commented out?
There was a problem hiding this comment.
Oh yeah, forgot about these, thanks a lot for heads up. I had some erros about these, and commented them to focus on some other errors, and come back to them when I finish, but I kinda forgot. Thanks
| import std.algorithm.mutation : move, moveEmplace; | ||
|
|
||
| import std.traits : Unqual; | ||
|
|
||
|
|
||
| import core.exception : RangeError; | ||
| import std.range.primitives; | ||
| import std.traits; |
There was a problem hiding this comment.
Please remove the unnecessary empty line(s).
| if (_a == _b) return; | ||
|
|
| if (i == j) | ||
| return; |
| @@ -1391,6 +1645,7 @@ if (!is(immutable T == immutable bool)) | |||
| assert(a.length == 7); | |||
| assert(equal(a[1 .. 4], [1, 2, 3])); | |||
| } | |||
| */ | |||
There was a problem hiding this comment.
Disabling unittests doesn’t seem like a good idea.
| @disable this(this); | ||
| this(int v) @safe pure nothrow @nogc { x = v; } | ||
|
|
||
| // ADĂUGAT |
There was a problem hiding this comment.
Please either replace this with an English term or remove it.
Hello, I have tried to add move-only support for dlang arrays. In theory everything should work just fine. But, I'm gonna (maybe) continue updating this PR, if something goes wrong. If you have any suggestions, please, don't hesitate to reach out and help me.