ZeroCopy aliases #1777
ZeroCopy aliases #1777
Conversation
robertbastian
requested review from
Manishearth,
sffc,
a team,
zbraniecki,
nciric and
dminor
as code owners
|
@Manishearth could you please take a look at the |
|
@robertbastian I have a fix, along with code making the rest of your stuff compile; mind if I push to the same branch? |
|
Commits are at https://github.com/Manishearth/icu4x/tree/pr-1777, either way. I plan to extract some of it to a separate PR, for one the trait implementation situation is annoying and will likely get worse |
| .iter() | ||
| .map(zerofrom::ZeroFrom::zero_from) | ||
| { | ||
| if let Ok(from) = raw_from.parse::<LanguageIdentifier>() { |
There was a problem hiding this comment.
Note: I think this is still parsing each time for all canonicalize operations
There was a problem hiding this comment.
^ Please fix if this causes us to allocate memory for every operation. If there is no memory being allocated, this is a nice-to-fix but not must-fix
There was a problem hiding this comment.
Yes it is, but this is the fallback field which we're trying to keep empty. If you have a way to check uts35_rule_matches on a string I'm all ears, but I don't see it without parsing.
There was a problem hiding this comment.
To do this without allocation we could make Variants a Cow<'data, str>. The only functionality it has is to iterate through the variants, which would be very cheap with something like self.0.split('-').map(Variant::from_raw_unchecked). The downside would be that this adds a lifetime to LanguageIdentifier and Locale.
There was a problem hiding this comment.
OK, if this is for fallback data, and fallback data is currently empty, I have no concerns.
Modifying the data model of LanguageIdentifier or Locale is understood to be off the table with much discussion in #831.
Closes #1034