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

????? add slice::array_chunks #72334

Closed
wants to merge 4 commits into from
Closed

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented May 18, 2020

adds a currently somewhat unuseable implementation of #60735.
cc @DutchGhost

I am not sure if this does more harm than good by showing it in the stable docs, but this felt like a nice challenge of my knowledge of the current state of const generics.

r? @varkor for the const generics bit

edit: the biggest problem is #71154, which prevents all uses of slice.array_chunks::<{expr}>() and some type inference stuff which may already be fixed on the currently nightly

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 18, 2020
@DutchGhost
Copy link
Contributor

DutchGhost commented May 18, 2020

Oh! array_chunks, I like that name!

I remember the compiler got a little wonky when array_chunks was a method on [T].

I also remember it was a little less wonky when using a freestanding function as I wrote in #60735 (comment).

Perhaps it's something to investigate if we want this feature to (finally) be usable in nightly?

However I found the free standing function a bit of a downside if it goes for how the resulting code looks at userlevel. <[...]>::array_chunks(slice) vs slice.array_chunks().

@lcnr
Copy link
Contributor Author

lcnr commented May 18, 2020

To be fair, the compiler is always a little wonky atm 😆

Free standing functions work as const_chunks_exact::<_, 3> is not a type dependent path,
i.e. we already know which method is used here during ast lowering.

For v.array_chunks, we must know the type of v to get the method definition of array_chunks, and therefore the expected type of 3. This would currently cause a cycle error if we tried to look it up. I tried to explain this problem in more detail in the description of #71154.

<[...]>::array_chunks(slice) also works for methods taking self

@DutchGhost
Copy link
Contributor

Can we make it a method, but enforce function style calling for now? Mainly I find it concerning that when called as method, rustc ICE's. I don't think its good for libcore to have code that crashes rustc when called 'wrong'

@leonardo-m
Copy link

Panics if N is 0.

Wish this to happen at compile-time.

@lcnr
Copy link
Contributor Author

lcnr commented May 19, 2020

Can we make it a method, but enforce function style calling for now?

Sadly not, as slice.array_chunks::<{expr}>() cases an ice even in cases where slice.array_chunks is invalid (e.g. array chunks is not a method of slice).

So even if we change array_chunks to take slice: &Self instead, we still get the same ICE and also can't use let [a, b] = slice.array_chunks().next().unwrap(); anymore.

@varkor varkor marked this pull request as draft May 24, 2020 16:53
@varkor
Copy link
Member

varkor commented May 24, 2020

I'm going to mark this as a draft for now, because I don't think we're ready to add this even unstably yet, when it's too prone to ICEing. Hopefully we'll be able to get the remaining issues ironed out before too long.

@Muirrum
Copy link
Member

Muirrum commented Jun 20, 2020

@rustbot modify labels to S-waiting-on-author -S-waiting-on-review

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 20, 2020
@Dylan-DPC-zz
Copy link

I'm going to close this. Leaving it open will just accumulate conflicts and things may change in the future. Better to create a new PR when revisiting it.

@Dylan-DPC-zz Dylan-DPC-zz added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 20, 2020
@lcnr lcnr deleted the array_chunks branch June 20, 2020 16:31
@lcnr lcnr restored the array_chunks branch July 9, 2020 13:25
@lcnr
Copy link
Contributor Author

lcnr commented Jul 15, 2020

Reopened as #74373.

bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 1, 2020
add `slice::array_chunks` to std

Now that rust-lang#74113 has landed, these methods are suddenly usable. A rebirth of rust-lang#72334

Tests are directly copied from `chunks_exact` and some additional tests for type inference.

r? @withoutboats as you are both part of t-libs and working on const generics. closes rust-lang#60735
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants