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

Implemented Default for arrays up to [T; 32] #27825

Merged
merged 1 commit into from
Aug 15, 2015

Conversation

withoutboats
Copy link
Contributor

Implemented Default for arrays up to length 32 where T: Default using an additional macro.

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @brson (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@marcusklaas
Copy link
Contributor

I'm not sure how exactly array_impls! works, but I think it should be possible to create array literals in macros.

@alexcrichton
Copy link
Member

Could this reduce the verbosity a bit by taking a strategy similar to this peel! macro?

@withoutboats
Copy link
Contributor Author

@alexcrichton I don't think so, because array syntax requires that each successive macro invocation insert a different integer literal.

I have managed to shorten it by removing all the ::default() calls. Now each macro invocation fits on one line, at least.

EDIT: well, maybe there's a way that's a bit more complicated than that, let me sketch it out and see.

@marcusklaas the problem with creating array literals is that for each item in the array, the macro invocation needs a token to expand to (unless the array is restricted to Copy types). You can see in the commit that, for example, the 32-len array needs 32 Ts, and the 31-len array needs 31 Ts.


Not clear on the stability tagging scheme and if this impl needs to be tagged with something. #[stable(since = "1.4.0")]?

@marcusklaas
Copy link
Contributor

@withoutboats You're almost there! You can 'dynamically' create the integers by using peel!'s strategy. You have to pass along a $counter:expr argument to your macro, which you can either increment of decrement by 1 with each recursion!

I'll try to make something in play.

edit2: it works: https://play.rust-lang.org/?gist=33022e935f049b9a8e8c&version=nightly

@withoutboats
Copy link
Contributor Author

Okay, this should be good now. I had no idea that [T; 2 + 2] was a valid type.

@alexcrichton
Copy link
Member

Thanks! Could you also:

  • Squash these commits together
  • Tag the impl blocks with #[stable(since = "1.4.0", feature = "array_default")]

@withoutboats
Copy link
Contributor Author

Done. 😄

@alexcrichton
Copy link
Member

@bors: r+ 975a8ed

@bors
Copy link
Contributor

bors commented Aug 15, 2015

⌛ Testing commit 975a8ed with merge 1e1b7f3...

bors added a commit that referenced this pull request Aug 15, 2015
Implemented Default for arrays up to length 32 where T: Default using an additional macro.
@bors bors merged commit 975a8ed into rust-lang:master Aug 15, 2015
@bluss bluss added the relnotes Marks issues that should be documented in the release notes of the next release. label Aug 18, 2015
@andrey-gvrd
Copy link

Can you give an example of usage? My attempt:

#[feature(array_default)]
fn main() {
    let mut s: [String; 2] = [Default::default(); 2];
}

Doesn't seem to work. I'm on rustc 1.5.0 (3d7cd77e4 2015-12-04).

@jonas-schievink
Copy link
Contributor

@andrey-gvrd String isn't Copy. Quoting the array docs:

Arrays values are created either with an explicit expression that lists each element: [x, y, z] or a repeat expression: [x; N]. The repeat expression requires that the element type is Copy.

Use vec![Default::default(); 2] to create a Vec<String> instead.

@withoutboats
Copy link
Contributor Author

@andrey-gvrd The way to use this is actually let mut s: [String; 2] = Default::default();. You do not need to use the array declaration syntax. You also don't need to use a feature, because this impl is stable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
relnotes Marks issues that should be documented in the release notes of the next release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants