Skip to content
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.

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

Use inout for ref returning members in std.container.array #2573

Merged
merged 1 commit into from
Oct 1, 2014

Conversation

nordlow
Copy link
Contributor

@nordlow nordlow commented Sep 30, 2014

@quickfur
Copy link
Member

What about unittests that call these functions with mutable/immutable arguments? I'd like something to fail if somebody inadvertently removes these inouts.

@nordlow
Copy link
Contributor Author

nordlow commented Sep 30, 2014

Ok. I added tests for opIndex(), back() and front(). Neither compile if I remove the inout qualifiers for the respective function.

If there are more functions in other containers I can make them inout too. If so should I do that in this PR or others?

BTW:

    const Array!int a = [0];
    foreach (e; a) {}

errors as

array.d(860,5): Error: template std.array.popFront cannot deduce function from argument types !()(const(Array!int)), candidates are:
/home/per/opt/x86_64-unknown-linux-gnu/dmd/linux/bin64/src/phobos/std/array.d(573,6):        std.array.popFront(T)(ref T[] a) if (!isNarrowString!(T[]) && !is(T[] == void[]))
/home/per/opt/x86_64-unknown-linux-gnu/dmd/linux/bin64/src/phobos/std/array.d(596,6):        std.array.popFront(C)(ref C[] str) if (isNarrowString!(C[]))

because of const qualifier of a. Could this be made to work somehow?

@nordlow nordlow force-pushed the inout-array branch 5 times, most recently from 643fb86 to 9028242 Compare September 30, 2014 19:15
@quickfur
Copy link
Member

Shouldn't a slice of the Array be taken when iterating over it? At least, that's what I thought it should do. Const/immutable built-in arrays can't be iterated over either, because popping an array (slice) modifies it. In the built-in case, though, foreach works because it implicitly makes a slice of the array, changing it from const(T[]) to const(T)[].

@nordlow
Copy link
Contributor Author

nordlow commented Sep 30, 2014

Added inout support for range aswell along with unittests.

@quickfur
Copy link
Member

quickfur commented Oct 1, 2014

LGTM, thanks!

@quickfur
Copy link
Member

quickfur commented Oct 1, 2014

Auto-merge toggled on

quickfur pushed a commit that referenced this pull request Oct 1, 2014
Use inout for ref returning members in std.container.array
@quickfur quickfur merged commit 4ae1b14 into dlang:master Oct 1, 2014
assert(a.back == 2);
auto r = a[];
size_t i;
foreach (e; r)
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this works when T in Array!T has mutable indirection. Unfortunately, constifying opSlice is not a satisfying solution to this problem.

monarchdodra added a commit that referenced this pull request Oct 1, 2014
@schuetzm
Copy link
Contributor

@MartinNowak
Copy link
Member

@mihails-strasuns
Copy link

#2643

@nordlow
Copy link
Contributor Author

nordlow commented Oct 27, 2014

I propose to close this in favour of #2631.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
9 participants