icu4x
icu4x copied to clipboard
CollatorBorrowed should have an infallible constructor from baked data
AFAICT, the only way for CollatorBorrowed::try_new to fail is to override baked data with badly-generated baked data.
It seems to me that broken baked data is a different category of error than e.g. a bogus Postcard, so it seems to me that it would be appropriate to panic in the case of broken baked data and not expect the app to handle an error result from baked data-backed CollatorBorrowed construction.
AFAICT, the only way for
CollatorBorrowed::try_newto fail is to override baked data with badly-generated baked data.
No, you can give it bad options.
https://github.com/unicode-org/icu4x/blob/6187c85412aa9154ac2d8e6f5310fa297ff0026b/components/collator/src/comparison.rs#L577-L594
I'm going to close this since the premise is incorrect, but we can reopen if that premise changes.
But you are correct: we do not error for "incorrect baked data"; we have GIGO behavior.
(we still try to not panic)
I thought we had discussed at some point having a specialized constructor like new_root() that was infallible.
See https://github.com/unicode-org/icu4x/issues/5856 for what we decided to do with Segmenter. I think we could do something very similar for Collator.
(going to re-open as I don't think this was addressed with @Manishearth's comment above)
Or we could close this as a duplicate of #6633
That's https://github.com/unicode-org/icu4x/issues/6633, we should close this as a duplicate then.
There are two potential issues: Adding const constructible new_root (#6633), and making try_new infallible (this issue, which cannot be done)
AFAICT, the only way for
CollatorBorrowed::try_newto fail is to override baked data with badly-generated baked data.No, you can give it bad options.
What bad options? The error returns in the code you quoted are returns on bad data, and we're not supposed to have bad baked data.
Yeah, it appears that the checks are for invariants between data structs, which shouldn't be reachable with baked data.
Collator[Borrowed]::try_new does a data request with the preferences locale, which can always fail.
@hsivonen am I missing something here?