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

impl Serialize and Deserialize for std::num::NonZero* #1191

Merged
merged 1 commit into from
Mar 25, 2018

Conversation

SimonSapin
Copy link
Contributor

… gated on the unstable Cargo feature.

These are new standard library types. Tracking issue: rust-lang/rust#49137

where
S: Serializer,
{
self.clone().get().serialize(serializer)
Copy link
Member

Choose a reason for hiding this comment

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

Why is this cloned?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I copied this from the generic NonZero<T> impl which does not assume T: Copy (get takes self by value), but these concrete types are indeed Copy so Clone is not necessary. I’ve removed it.

@@ -1934,6 +1935,39 @@ where
}
}

macro_rules! nonzero_integers {
( $( $T: ident, )+ ) => {
nonzero_integers!(@ $(::std::num::$T,)+);
Copy link
Member

Choose a reason for hiding this comment

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

The @ trick doesn't work on 1.13

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I’ve verified that it does if I swap the order of the macro’s two variants/patterns.

@SimonSapin SimonSapin force-pushed the nonzero branch 3 times, most recently from a7ece65 to f7d0354 Compare March 24, 2018 19:24
@SimonSapin
Copy link
Contributor Author

On Travis:

lib: serde
    Updating registry `https://github.com/rust-lang/crates.io-index`
   Compiling serde v1.0.35 (file:///home/travis/build/serde-rs/serde/serde)
error: Unrecognized option: 'features'
error: Could not compile `serde`.
To learn more, run the command again with --verbose.
The command "./travis.sh" exited with 101.

I think this is when running cargo clippy --features 'rc unstable' -- -Dclippy, but it seems unrelated to this PR…

@dtolnay
Copy link
Member

dtolnay commented Mar 24, 2018

The clippy trouble is rust-lang/rust-clippy#2566.

… gated on the `unstable` Cargo feature.

These are new standard library types.
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Thanks!

@dtolnay dtolnay merged commit 4a2612e into serde-rs:master Mar 25, 2018
@SimonSapin SimonSapin deleted the nonzero branch March 26, 2018 09:06
SimonSapin added a commit to servo/webrender that referenced this pull request Mar 27, 2018
This will enable using serde-rs/serde#1191 in Servo
bors-servo pushed a commit to servo/webrender that referenced this pull request Mar 27, 2018
WIP: Upgrade serde

This will enable using serde-rs/serde#1191 in Servo

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/2577)
<!-- Reviewable:end -->
bors-servo pushed a commit to servo/webrender that referenced this pull request Mar 27, 2018
WIP: Upgrade serde

This will enable using serde-rs/serde#1191 in Servo

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/2577)
<!-- Reviewable:end -->
SimonSapin added a commit to servo/webrender that referenced this pull request Mar 27, 2018
This will enable using serde-rs/serde#1191 in Servo

This includes a rebase of serde-rs/serde@master...Gankro:deserialize_from_enums4
bors-servo pushed a commit to servo/webrender that referenced this pull request Mar 27, 2018
Upgrade serde

This will enable using serde-rs/serde#1191 in Servo

Includes a rebase of serde-rs/serde@master...Gankro:deserialize_from_enums4

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/2577)
<!-- Reviewable:end -->
SimonSapin added a commit to servo/webrender that referenced this pull request Mar 27, 2018
This will enable using serde-rs/serde#1191 in Servo

This includes a rebase of serde-rs/serde@master...Gankro:deserialize_from_enums4
bors-servo pushed a commit to servo/webrender that referenced this pull request Mar 27, 2018
Upgrade serde

This will enable using serde-rs/serde#1191 in Servo

Includes a rebase of serde-rs/serde@master...Gankro:deserialize_from_enums4

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/2577)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants