Add FixedLengthVarULE and user in icu_plurals #7394

sffc · 2026-01-09T21:13:09Z

See #7391
Depends on #7399

I was making a PR that got a bit bigger than I wanted, so I split this out into a standalone change.

We finally have VarZeroCow, a heap-or-reference container for a VarULE. Another reasonable container to want is what I'm proposing in this PR, a FixedLengthVarULE, which stores the VarULE on the stack with a fixed number of bytes.

This can be used to compose ULEs and VarULEs in const contexts, as I show with the PluralElementsPackedULE example.

robertbastian

I don't see the problem this solves.

sffc · 2026-01-09T21:36:18Z

This can be used to compose ULEs and VarULEs in const contexts, as I show with the PluralElementsPackedULE example.

It can also be used to create a string or zerovec on the stack, without any additional dependencies.

Manishearth

I don't really understand the motivation: why is VarZeroCow<'static> insufficient? What is the benefit of having these values on the stack instead of static memory?

As far as I can tell, anything you ought to be able to do with this type in const should be doable in non-const as well.

Manishearth · 2026-01-09T21:42:48Z

components/plurals/src/provider.rs

+    /// use icu::locale::locale;
+    /// use zerovec::ule::FixedLengthVarULE;
+    ///
+    /// let value = "hello, world!"; // 13 bytes long


issue: this isn't a motivating example: this doesn't work in const anyway (since there's try_from_encodeable)

(It's an example, but not a motivating one)

It would work in const if I replaced try_from_encodeable with new_unchecked, I just wanted to not have unsafe code in the docs test

Yeah, that's fine, I was just saying that the example itself wasn't motivating the new zerovec API.

But I have a clearer idea of what's going on now.

Manishearth · 2026-01-09T21:46:23Z

Ah, I understand. This lets you copy the data into a new, stack-owned buffer, which you can't do with static data. Hm

robertbastian · 2026-01-09T21:53:55Z

components/plurals/src/provider.rs

+    /// ```
+    ///
+    /// [generic_const_exprs]: https://doc.rust-lang.org/beta/unstable-book/language-features/generic-const-exprs.html#generic_const_exprs
+    pub const fn new_singleton_mn<const M: usize, const N: usize>(


I don't like these APIs where you have to guess the output.

I don't fully understand your comment, but this is in an unstable module.

you have to know M and N for this not to panic

Manishearth

I'd like to hear more about the use case for new_singleton_mn in a const context, since as far as I can tell that would need unsafe code anyway.

I think I see use cases for the new type: being able to create a self-contained stack VarULE is useful.

I think the type name is somewhat misleading. Perhaps ConstStackVarULE? or ConstOwnedVarULE?

I think the type is relatively niche and we should name it in a way that reflects that, and document it as such. Stuffing it in the ule module sounds good, otherwise. In the long term I'd like to organize the mess that is that module.

Manishearth · 2026-01-09T21:58:10Z

components/plurals/src/provider.rs

+            remainder[i] = input.as_bytes()[i];
+            i += 1;
+        }
+        *start = 0;


issue: mention "first byte = 0 for a singleton" for redundancy

Manishearth · 2026-01-09T22:11:20Z

components/plurals/src/provider.rs

+    /// use zerovec::ule::FixedLengthVarULE;
+    ///
+    /// const plural_ule: FixedLengthVarULE<1, PluralElementsPackedULE<str>> =
+    ///     PluralElementsPackedULE::new_singleton_mn::<0, 1>(FixedLengthVarULE::EMPTY_STR);


sffc · 2026-01-09T22:11:31Z

I added a const example with no unsafe.

In general this is useful for making default/empty VarULEs in const contexts that are composed of inner ULE types.

Manishearth · 2026-01-09T22:14:21Z

In general this is useful for making default/empty VarULEs in const contexts that are composed of inner ULE types.

Right, but where are we hoping to do that? I'm still missing parts of the story here I think.

sffc · 2026-01-09T22:17:20Z

In general this is useful for making default/empty VarULEs in const contexts that are composed of inner ULE types.

Right, but where are we hoping to do that? I'm still missing parts of the story here I think.

I want to use this in call sites such as

    pub const PLURAL_PATTERN_0: &'static PluralElementsPackedULE<SinglePlaceholderPattern> =
        unsafe { PluralElementsPackedULE::from_bytes_unchecked(&[0, 1]) };

in order to remove the unsafe. @robertbastian may or may not agree with that particular call site but it serves as a useful illustration.

Manishearth · 2026-01-09T22:30:39Z

Hmm. I guess this gets more useful if we have consts on various VarULE types for common patterns.

I do worry that this is a fair amount of unsafe code to get rid of some easily-tested unsafe, though. Are we planning on using this const pattern a lot?

An alternative potential route might be to define a macro that can define VarULE consts from unchecked bytes, where it also generates a test. This macro becomes unsafe to use, BUT its safety invariant is mostly "if the test passes this macro usage is fine".

Not saying that's better, but making sure we have considered the design space.

robertbastian · 2026-01-09T22:30:40Z

I honestly think for that call site this would be a regression. The current code is unsafe, but it's a single expression, similar to hundreds that we have in baked data, and there's a test that it's correct. This PR adds a lot of weird APIs and unsafe code just to make that one expression safe.

Manishearth · 2026-01-09T22:32:01Z

Overall I've found the defaults story in VarULE to be unsatisfying, so "more consts" is definitely an interesting proposition. BUT I'm not fully convinced we should do it this way. Unsure.

sffc · 2026-01-09T22:39:40Z

Note: the use case for this type of helper will only grow, because the composition stuff (VarTuple) is fairly new and we are using it more and more.

sffc · 2026-01-09T22:41:06Z

I honestly think for that call site this would be a regression. The current code is unsafe, but it's a single expression, similar to hundreds that we have in baked data, and there's a test that it's correct. This PR adds a lot of weird APIs and unsafe code just to make that one expression safe.

It's not just about that one expression. It's about that class of expressions, which I claim is a class that will get bigger over time.

sffc · 2026-01-09T22:47:24Z

I also just want to point out:

    pub const PLURAL_PATTERN_0: &'static PluralElementsPackedULE<SinglePlaceholderPattern> =
        unsafe { PluralElementsPackedULE::from_bytes_unchecked(&[0, 1]) };

This code requires subject matter expertise in 3 different components: pattern (to know that 1 means "single placeholder pattern with a single placeholder"), plurals (to know that 0 means "singleton plural elements"), and compact (to know that this combination of pattern and plurals is suitable for PLURAL_PATTERN_0). Yes, the test helps a lot. But, you should be able to write a call site such as the following, and no extra test is required.

    pub const PLURAL_PATTERN_0: /* some type */ =
        PluralElementsPackedULE::new_singleton(SinglePlaceholderPattern::PASS_THROUGH_ULE);

(That line of code does two things that we don't have yet: new_singleton, which is gated on Rust feature generic_const_exprs, and PASS_THROUGH_ULE, because the ULE composition needs to use the correct types. This comment is not an opportunity to bikeshed either of those APIs.)

robertbastian

I still don't agree with this, but leaving a technical review

robertbastian · 2026-01-09T22:40:00Z

components/plurals/src/provider.rs

+    /// ```
+    ///
+    /// [generic_const_exprs]: https://doc.rust-lang.org/beta/unstable-book/language-features/generic-const-exprs.html#generic_const_exprs
+    pub const fn new_singleton_mn<const M: usize, const N: usize>(


"singleton" is the wrong term, we don't use that with plurals. this method is equivalent to PluralElements::new, so it should just be new

The word "singleton" is already used in this file multiple times, including in test function names.

The difference between singleton and non-singleton matters a great deal to PluralElementsPackedULE, because it results in two different encodings.

I thought the caller shouldn't have to know encoding details?

this also doesn't need to be qualified as _mn; it's an unstable type so we can just change the generics if generic_const_exprs ever gets stabilised (I wouldn't hold my breath). so just new

I'd rather keep the _mn because it's a pain to debug cross-crate function naming issues when the function mostly stays the same but const parameters change a little. But I will remove singleton.

robertbastian · 2026-01-09T22:40:14Z

components/plurals/src/provider.rs

+        input: ConstStackVarULE<M, V>,
+    ) -> ConstStackVarULE<N, PluralElementsPackedULE<V>> {
+        #[allow(clippy::panic)] // for safety, and documented
+        if N != M + 1 {


issue: this should be a const assertion, not a runtime panic

please suggest code that compiles with a const assertion.

if you can't do it then we shouldn't have this yet

hm? We have lots of const code with runtime assertions because there isn't a good way to write const assertions.

Even the Rust standard library does runtime panics on const parameters. For example:

https://doc.rust-lang.org/std/primitive.slice.html#method.as_chunks

slice::as_chunks panics if the const parameter N is zero. There was a lengthy discussion on the proposal thread, starting here: rust-lang/rust#74985 (comment)

robertbastian · 2026-01-09T22:41:41Z

components/plurals/src/provider.rs

+            remainder[i] = input.as_bytes()[i];
+            i += 1;
+        }
+        // First byte = 0 for a singleton


this is not "for a singleton" but "for a other case"

and this 0 represents a PluralCategoryAndMetadata, so you should use that type to construct it

It is for a singleton. That is the term from the byte encoding definition.

Actually to be fully correct we should add in the FourBitMetadata.

yes a PackedPluralElementsULE<T> corresponds to a PluralElements<(FourBitMetadata, T)>, so the constructor should take that tuple.

done. (not as a tuple)

all other APIs use (FourBitMetadata, T), please at least change the order

robertbastian · 2026-01-09T22:43:59Z

utils/zerovec/src/ule/fixed_length.rs

+/// ConstStackVarULE::<20, str>::try_from_encodeable("hello, world!").unwrap_err();
+/// ```
+#[derive(Copy, Clone, PartialEq, Eq)]
+pub struct ConstStackVarULE<const N: usize, V: VarULE + ?Sized> {


issue: this is not generally on the stack. the point is that it's Sized so it can be on the stack. SizedVarULE sounds like a trait, maybe SizedVarULEBytes? EncodedSizedVarULE? EncodedVarULEBytes?

I like SizedVarULEBytes

robertbastian · 2026-01-09T22:45:39Z

utils/zerovec/src/ule/fixed_length.rs

+    ///
+    /// Returns an error if the byte length in the container is not the correct length
+    /// for the encodeable object.
+    pub fn try_from_encodeable(input: impl EncodeAsVarULE<V>) -> Result<Self, UleError> {


Suggested change

pub fn try_from_encodeable(input: impl EncodeAsVarULE<V>) -> Result<Self, UleError> {

pub fn try_from_encodeable(input: &impl EncodeAsVarULE<V>) -> Result<Self, UleError> {

?

Not sure the recommend style here. It doesn't matter if the trait is implemented on a reference. @Manishearth ?

I don't think this is style choice. you only test this with a &str input, but say you have a String, you don't want to accidentally pass that by value.

robertbastian · 2026-01-09T22:51:30Z

This code requires subject matter expertise in 3 different components: pattern (to know that 1 means "single placeholder pattern with a single placeholder"), plurals (to know that 0 means "singleton plural elements"), and compact (to know that this combination of pattern and plurals is suitable for PLURAL_PATTERN_0)

No it doesn't. I wrote that code and don't know how these things are encoded. I just wrote the test and then put the bytes there that make the test pass. I think that's a valid and very readable approach.

But, you should be able to write a call site such as the following, and no extra test is required

Yes! I would love that. But it's not feasible with current Rust/zerovec.

robertbastian · 2026-01-09T23:23:54Z

components/plurals/src/provider.rs

+    /// let plural_ule = PluralElementsPackedULE::new_singleton_mn::<13, 14>(inner_ule, metadata);
+    /// let rules = PluralRules::try_new(locale!("en").into(), Default::default()).unwrap();
+    ///
+    /// assert_eq!(plural_ule.as_varule().get(0.into(), &rules).1, "hello, world!");


nit: merge the assertions

Suggested change

/// assert_eq!(plural_ule.as_varule().get(0.into(), &rules).1, "hello, world!");

/// assert_eq!(plural_ule.as_varule().get(0.into(), &rules), ("hello, world!", metadata));

robertbastian · 2026-01-09T23:24:32Z

components/plurals/src/provider.rs

+    /// const plural_ule: SizedVarULEBytes<1, PluralElementsPackedULE<str>> =
+    ///     PluralElementsPackedULE::new_singleton_mn::<0, 1>(SizedVarULEBytes::EMPTY_STR, FourBitMetadata::zero());


or better just integrate into the example above, this is very repetitive

Suggested change

/// const plural_ule: SizedVarULEBytes<1, PluralElementsPackedULE<str>> =

/// PluralElementsPackedULE::new_singleton_mn::<0, 1>(SizedVarULEBytes::EMPTY_STR, FourBitMetadata::zero());

/// let plural_ule = const {

/// PluralElementsPackedULE::new_singleton_mn::<0, 1>(SizedVarULEBytes::EMPTY_STR, FourBitMetadata::zero()) };

robertbastian · 2026-01-09T23:26:01Z

components/plurals/src/provider.rs

+    /// ```
+    ///
+    /// [generic_const_exprs]: https://doc.rust-lang.org/beta/unstable-book/language-features/generic-const-exprs.html#generic_const_exprs
+    pub const fn new_singleton_mn<const M: usize, const N: usize>(


this also doesn't need to be qualified as _mn; it's an unstable type so we can just change the generics if generic_const_exprs ever gets stabilised (I wouldn't hold my breath). so just new

Split from #7394

Manishearth · 2026-01-10T02:51:11Z

It's not just about that one expression. It's about that class of expressions, which I claim is a class that will get bigger over time.

I am not yet convinced that this is the case, but the use case seems sufficiently well outlined and I agree it is valid.. I would be in favor of us having more consts like this. As I said before, the default construction problem in varule is something I've wanted to see improvement on.

Regarding the MN stuff I'm overall okay with landing suboptimal API we know that we can clean up in future Rust. Though I don't know if we have a timeline for const expressions in generics?

sffc · 2026-01-10T04:10:49Z

Though I don't know if we have a timeline for const expressions in generics?

Well, it works in nightly, and rust-lang/rust#76560 has a lot of upvotes. It seems like there are concrete next steps but no one is currently actively funding it.

sffc · 2026-01-10T05:07:28Z

OK I added an ergonomic to_sized_varule_bytes! macro for construction, which even works with type hinting, and then migrated the compactdecimal call site to:

    pub const PLURAL_PATTERN_0: SizedVarULEBytes<
        2,
        PluralElementsPackedULE<SinglePlaceholderPattern>,
    > = PluralElementsPackedULE::new_mn(
        FourBitMetadata::zero(),
        to_sized_varule_bytes!(SinglePlaceholderPattern::PASS_THROUGH),
    );

Const variables always require a type, or else the type could be elided, except for the number 2.

Manishearth · 2026-01-10T05:52:04Z

@sffc Thanks!

I think if we're open to using a macro we don't need the SizedVarULEBytes type at all. The macro can construct a const/static array, and then construct a second const/static that references it.

This is what I was referring to earlier:

An alternative potential route might be to define a macro that can define VarULE consts from unchecked bytes, where it also generates a test. This macro becomes unsafe to use, BUT its safety invariant is mostly "if the test passes this macro usage is fine".

I can write more about this later.

sffc · 2026-01-11T05:24:16Z

If we can't remove the unsafe, then a macro of that shape isn't worth doing in my opinion.

Manishearth · 2026-01-11T05:35:41Z

Sorry, I should have been more explicit: I think using the design of your macro so far combined with my model, this can be done without callsite unsafe.

Add FixedLengthVarULE and user in icu_plurals

0a337f7

sffc requested review from Manishearth and zbraniecki as code owners January 9, 2026 21:13

sffc requested review from robertbastian and removed request for zbraniecki January 9, 2026 21:13

sffc added 2 commits January 9, 2026 13:19

clippy

2df46a9

MSRV

fd0f17a

robertbastian reviewed Jan 9, 2026

View reviewed changes

Manishearth requested changes Jan 9, 2026

View reviewed changes

sffc requested a review from Manishearth January 9, 2026 21:49

robertbastian requested changes Jan 9, 2026

View reviewed changes

sffc added 2 commits January 9, 2026 14:09

Add empty impls for str, slices

415999f

Add const example

8dd4e32

Manishearth reviewed Jan 9, 2026

View reviewed changes

Review: Add extra comment

7a25487

Manishearth reviewed Jan 9, 2026

View reviewed changes

ConstStackVarULE and fmt

031ad7f

sffc requested review from Manishearth and robertbastian January 9, 2026 22:20

robertbastian requested changes Jan 9, 2026

View reviewed changes

sffc added 3 commits January 9, 2026 14:55

SizedVarULEBytes

3af9c1b

Bugfix: should be checked against 0x10 or 0x0F, not 0x80

789e7ce

Add FourBitMetadata support

576d7c3

sffc mentioned this pull request Jan 9, 2026

FourBitMetadata should be checked against 0x10 or 0x0F, not 0x80 #7395

Merged

robertbastian reviewed Jan 9, 2026

View reviewed changes

sffc added a commit that referenced this pull request Jan 10, 2026

FourBitMetadata should be checked against 0x10 or 0x0F, not 0x80 (#7395)

ddcf86b

Split from #7394

sffc added 5 commits January 9, 2026 18:03

new_mn

2ce55ac

argument order

56f5e42

Merge assertions

1143c4c

use const block

87c2ad7

elide the first const parameter

2120095

sffc added 3 commits January 9, 2026 20:56

Add to_sized_varule_bytes! and from_varule

76d4930

Merge branch 'main' into fixed-length-varule

8723e53

Remove unsafe expression from compactdecimal/provider.rs

44a1ecb

sffc requested review from a team and younies as code owners January 10, 2026 05:05

sffc mentioned this pull request Jan 11, 2026

Require VarULE concrete fns to behave the same as trait fns #7399

Open

sffc added the waiting-on-reviewer PRs waiting for action from the reviewer for >7 days label Jan 12, 2026

	pub fn try_from_encodeable(input: impl EncodeAsVarULE<V>) -> Result<Self, UleError> {
	pub fn try_from_encodeable(input: &impl EncodeAsVarULE<V>) -> Result<Self, UleError> {

	/// assert_eq!(plural_ule.as_varule().get(0.into(), &rules).1, "hello, world!");
	/// assert_eq!(plural_ule.as_varule().get(0.into(), &rules), ("hello, world!", metadata));

		/// const plural_ule: SizedVarULEBytes<1, PluralElementsPackedULE<str>> =
		/// PluralElementsPackedULE::new_singleton_mn::<0, 1>(SizedVarULEBytes::EMPTY_STR, FourBitMetadata::zero());

Add FixedLengthVarULE and user in icu_plurals #7394

Are you sure you want to change the base?

Add FixedLengthVarULE and user in icu_plurals #7394

Uh oh!

Conversation

sffc commented Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

robertbastian left a comment

Choose a reason for hiding this comment

Uh oh!

sffc commented Jan 9, 2026

Uh oh!

Manishearth left a comment

Choose a reason for hiding this comment

Uh oh!

Manishearth Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Manishearth commented Jan 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Manishearth left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sffc commented Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Manishearth commented Jan 9, 2026

Uh oh!

sffc commented Jan 9, 2026

Uh oh!

Manishearth commented Jan 9, 2026

Uh oh!

robertbastian commented Jan 9, 2026

Uh oh!

Manishearth commented Jan 9, 2026

Uh oh!

sffc commented Jan 9, 2026

Uh oh!

sffc commented Jan 9, 2026

Uh oh!

sffc commented Jan 9, 2026

Uh oh!

robertbastian left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sffc Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

sffc commented Jan 9, 2026 •

edited

Loading

Manishearth Jan 9, 2026 •

edited

Loading

sffc commented Jan 9, 2026 •

edited

Loading

sffc Jan 9, 2026 •

edited

Loading

Manishearth commented Jan 10, 2026 •

edited

Loading