Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,10 @@ jobs:
cargo update -p serde_bytes --precise 0.11.16
cargo update -p indexmap --precise 2.5.0
cargo update -p once_cell --precise 1.20.3
- run: cargo clippy -- -D warnings
- run: cargo clippy --features integer128 -- -D warnings
- run: cargo clippy --features indexmap -- -D warnings
- run: cargo clippy --all-features -- -D warnings
- run: cargo clippy -- -D warnings -A unknown-lints
- run: cargo clippy --features integer128 -- -D warnings -A unknown-lints
- run: cargo clippy --features indexmap -- -D warnings -A unknown-lints
- run: cargo clippy --all-features -- -D warnings -A unknown-lints

clippy-fuzz:
name: "Clippy: Fuzzer"
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### API Changes

- Add `ron::Options::to_io_writer` and `ron::Options::to_io_writer_pretty` to allow writing into an `io::Writer` ([#561](https://github.com/ron-rs/ron/pull/561))
- Breaking: `ron::value::Number` is now non-exhaustive, to avoid breaking `match`es when feature unification enables more of its variants than expected ([#564](https://github.com/ron-rs/ron/pull/564))

## [0.9.0] - 2025-03-18

Expand Down
39 changes: 38 additions & 1 deletion src/value/number.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,34 @@ use std::{

use serde::{de::Visitor, Serialize, Serializer};

/// A wrapper for any numeric primitive type in Rust
/// A wrapper for any numeric primitive type in Rust.
///
/// Some varints of the `Number` enum are enabled by features:
/// - `Number::I128` and `Number::U128` by the `integer128` feature
///
/// To ensure that feature unification does not break `match`ing over `Number`,
/// the `Number` enum is non-exhaustive.
///
/// <details>
/// <summary>Exhaustively matching on <code>Number</code> in tests</summary>
///
/// If you want to ensure that you exhaustively handle every variant, you can
/// match on the hidden `Number::__NonExhaustive` variant.
///
/// <div class="warning">
/// Matching on this variant means that your code may break when RON is
/// upgraded or when feature unification enables further variants in the
/// <code>Number</code> enum than your code expects.
/// </div>
///
/// It is your responsibility to only *ever* match on `Number::__NonExhaustive`
/// inside tests, e.g. by using `#[cfg(test)]` on the particular match arm, to
/// ensure that only your tests break (e.g. in CI) when your code is not
/// exhaustively matching on every variant, e.g. after a version upgrade or
/// feature unification.
/// </details>
#[derive(Copy, Clone, Debug, PartialEq, PartialOrd, Eq, Hash, Ord)]
#[cfg_attr(doc, non_exhaustive)]
pub enum Number {
I8(i8),
I16(i16),
Expand All @@ -22,6 +48,14 @@ pub enum Number {
U128(u128),
F32(F32),
F64(F64),
#[cfg(not(doc))]
#[allow(private_interfaces)]
__NonExhaustive(private::Never),
Comment on lines +51 to +53
Copy link

Choose a reason for hiding this comment

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

Unsure whats going on but this didn't make the enum exhaustive. I wonder if the compiler recognized that this variant cannot be constructed so therefore it doesn't need to be matched?

Copy link
Member Author

Choose a reason for hiding this comment

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

Uff, maybe this comes from some of the newer uninhabited type matching ...

Since private::Never is private, we can make the type inhabited but not constructible. Would changing this be a breaking change?

Copy link

Choose a reason for hiding this comment

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

Currently, people can exhaustively match on Number. Making it actually non-exhaustive would then be a breaking change.

Copy link
Member Author

Choose a reason for hiding this comment

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

We’ll definitely need a compile-fail test then to ensure there are no more slip ups - I’m sorry that the first fix didn’t catch this.

}

mod private {
#[derive(Copy, Clone, Debug, PartialEq, PartialOrd, Eq, Hash, Ord)]
pub enum Never {}
}

impl Serialize for Number {
Expand All @@ -41,6 +75,7 @@ impl Serialize for Number {
Self::U128(v) => serializer.serialize_u128(*v),
Self::F32(v) => serializer.serialize_f32(v.get()),
Self::F64(v) => serializer.serialize_f64(v.get()),
Self::__NonExhaustive(never) => match *never {},
}
}
}
Expand All @@ -65,6 +100,7 @@ impl Number {
Self::U128(v) => visitor.visit_u128(*v),
Self::F32(v) => visitor.visit_f32(v.get()),
Self::F64(v) => visitor.visit_f64(v.get()),
Self::__NonExhaustive(never) => match *never {},
}
}
}
Expand Down Expand Up @@ -216,6 +252,7 @@ impl Number {
Number::U128(v) => v as f64,
Number::F32(v) => f64::from(v.get()),
Number::F64(v) => v.get(),
Self::__NonExhaustive(never) => match never {},
}
}
}
Expand Down
Loading