-
Notifications
You must be signed in to change notification settings - Fork 255
Pattern should always be VarULE #7440
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
base: main
Are you sure you want to change the base?
Conversation
| #[cfg(not(feature = "zerovec"))] | ||
| type Store: ?Sized + PartialEq + core::fmt::Debug; | ||
| #[cfg(feature = "zerovec")] | ||
| type Store: ?Sized + PartialEq + core::fmt::Debug + zerovec::ule::VarULE; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
observation: making a bound tighter based on a feature is not generally ok, but this trait is sealed
|
|
||
| #[cfg(feature = "datagen")] | ||
| impl<'data, P: PatternBackend> serde::Serialize for CompactPatterns<'data, P> | ||
| impl<'data, P: PatternBackend<Store = str>> serde::Serialize for CompactPatterns<'data, P> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
serde should only need P::Store: Serialize/Deserialize
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For Serialize, it looks like it works to write where Pattern<P>: Serialize.
For Deserialize, on this code:
impl<'de, 'data, P: PatternBackend> serde::Deserialize<'de>
for CompactPatterns<'data, P>
where
'de: 'data,
Pattern<P>: serde::Deserialize<'de>,I'm getting this error:
error[E0275]: overflow evaluating the requirement `&icu_pattern::Pattern<_>: displaynames::provider::_::_serde::Deserialize<'_>`
|
= help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`icu_experimental`)
= note: required for `&icu_pattern::Pattern<_>` to implement `displaynames::provider::_::_serde::Deserialize<'_>`
= note: 125 redundant requirements hidden
= note: required for `&icu_pattern::Pattern<_>` to implement `displaynames::provider::_::_serde::Deserialize<'_>`
For more information about this error, try `rustc --explain E0275`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works if the bound is on Box<Pattern<P>>.
|
I could alternatively remove the requirement that Store always implement VarULE and instead ask clients to write |
I tried doing this but then |
Follow-up from #7411