Add UnvalidatedChar #3444
Merged
Add UnvalidatedChar #3444
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sffc
reviewed
May 21, 2023
utils/zerovec/src/ule/unvalidated.rs
Outdated
| /// assert!(matches!(b.try_as_char(), Err(_))); | ||
| /// ``` | ||
| #[inline] | ||
| pub fn try_as_char(self) -> Result<char, core::char::CharTryFromError> { |
There was a problem hiding this comment.
Nit, here and below: I think this should be "to" instead of "as" since we are moving (copying) the type.
There was a problem hiding this comment.
Thanks, fixed! Do you know why the clippy lint (wrong_self_convention) didn't trigger (for the other two)?
There was a problem hiding this comment.
I've had really mixed results with that lint, but @Manishearth might be able to shed some light on how it works
robertbastian
requested changes
May 22, 2023
robertbastian
previously approved these changes
May 22, 2023
robertbastian
approved these changes
May 22, 2023
sffc
approved these changes
May 22, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1968
This adds the unvalidated, three-byte char type
UnvalidatedCharwithAsULE.It lives next to
UnvalidatedStrfor now, but it can easily be moved again.I borrowed some decisions from
UnvalidatedStrin regards to theDebugandSerializeimplementations:Debugwill try to pretty-print but fall back to bytes if invalidSerializewill fail if the data contained in theUnvalidatedCharis invalidI implemented
EqULEbecause it applies, but from reading other discussions, it seems we want to move away from it? In which case I would remove it.Furthermore, I was unsure about
Ord- I couldn't think of any reasonable total order we could use here that is consistent with<char as Ord>. If we need it, I'm open to suggestions.