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

withoutboats commented Aug 14, 2015

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

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Aug 14, 2015

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

This comment has been minimized.

Copy link
Contributor

marcusklaas commented Aug 14, 2015

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

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Aug 14, 2015

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

@withoutboats

This comment has been minimized.

Copy link
Contributor Author

withoutboats commented Aug 14, 2015

@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

This comment has been minimized.

Copy link
Contributor

marcusklaas commented Aug 14, 2015

@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

This comment has been minimized.

Copy link
Contributor Author

withoutboats commented Aug 14, 2015

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

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Aug 14, 2015

Thanks! Could you also:

  • Squash these commits together
  • Tag the impl blocks with #[stable(since = "1.4.0", feature = "array_default")]
@withoutboats withoutboats force-pushed the withoutboats:default_array branch from f26f7b9 to 975a8ed Aug 15, 2015
@withoutboats

This comment has been minimized.

Copy link
Contributor Author

withoutboats commented Aug 15, 2015

Done. 😄

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Aug 15, 2015

@bors

This comment has been minimized.

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
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@bluss bluss added the relnotes label Aug 18, 2015
@andrey-gvrd

This comment has been minimized.

Copy link

andrey-gvrd commented Dec 23, 2015

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

This comment has been minimized.

Copy link
Member

jonas-schievink commented Dec 23, 2015

@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

This comment has been minimized.

Copy link
Contributor Author

withoutboats commented Dec 23, 2015

@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.

@withoutboats withoutboats deleted the withoutboats:default_array branch Dec 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
9 participants
You can’t perform that action at this time.