Skip to content

Conversation

@sffc
Copy link
Member

@sffc sffc commented Dec 12, 2025

Breaking change in icu_provider extracted from #7309.

The trait unfortunately wasn't implemented with EncodeAsVarULE in mind, so this is a breaking change in icu_provider in a trait that isn't used by anything except hello_world (which lives in that crate) and is only implemented by macros that are versioned along with the crate. Do we care? Please speak up.

@sffc sffc requested review from a team and Manishearth as code owners December 12, 2025 02:15
Copy link
Member

@Manishearth Manishearth left a comment

Choose a reason for hiding this comment

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

Yeah, this makes sense. Unfortunate oversight. I'm wary of the breaking change, but it does not seem to be a real problem yet.

@sffc
Copy link
Member Author

sffc commented Dec 13, 2025

cargo-semver-check suggested that I add a default for the associated type, but when I do that, I get an error saying that associated type defaults are not stable. rust-lang/rust#29661

@sffc
Copy link
Member Author

sffc commented Dec 13, 2025

Here's the semver check report

    Checking icu_provider v2.1.1 -> v2.1.1 (no change; assume patch)
     Checked [   0.027s] 184 checks: 180 pass, 4 fail, 0 warn, 16 skip

--- failure trait_associated_type_added: non-sealed public trait added associated type without default value ---

Description:
A non-sealed trait has gained an associated type without a default value, which breaks downstream implementations of the trait
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#trait-new-item-no-default
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.45.0/src/lints/trait_associated_type_added.ron

Failed in:
  trait associated type icu_provider::ule::MaybeEncodeAsVarULE::EncodeableStruct in file /home/runner/work/icu4x/icu4x/provider/core/src/varule_traits.rs:36

--- failure trait_method_added: pub trait method added ---

Description:
A non-sealed public trait added a new method without a default implementation, which breaks downstream implementations of the trait
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#trait-new-item-no-default
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.45.0/src/lints/trait_method_added.ron

Failed in:
  trait method icu_provider::ule::MaybeEncodeAsVarULE::maybe_as_encodeable in file /home/runner/work/icu4x/icu4x/provider/core/src/varule_traits.rs:41

--- failure trait_method_missing: pub trait method removed or renamed ---

Description:
A trait method is no longer callable, and may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#major-any-change-to-trait-item-signatures
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.45.0/src/lints/trait_method_missing.ron

Failed in:
  method maybe_encode_as_varule of trait MaybeEncodeAsVarULE, previously in file /home/runner/work/icu4x/icu4x/semver-checks/target/semver-checks/git-origin_main/d6f790cc393030a5d1382de3f18bbc857d401ae6/provider/core/src/varule_traits.rs:35

--- failure trait_no_longer_dyn_compatible: trait no longer dyn compatible ---

Description:
Trait is no longer dyn compatible, which breaks `dyn Trait` usage.
        ref: https://doc.rust-lang.org/stable/reference/items/traits.html#object-safety
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.45.0/src/lints/trait_no_longer_dyn_compatible.ron

Failed in:
  trait MaybeEncodeAsVarULE in file /home/runner/work/icu4x/icu4x/provider/core/src/varule_traits.rs:34

     Summary semver requires new major version: 4 major and 0 minor checks failed

If I switch the function name back to what it was before and add a default associated type (not possible in stable Rust), I think 3/4 of those errors go away.

But I'm going to merge this for now since @Manishearth approved it.

@sffc sffc merged commit d41a393 into unicode-org:main Dec 13, 2025
31 of 32 checks passed
@sffc sffc deleted the varule-core-change branch December 13, 2025 00:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants