Can basic SmartString functions be made constant?

[schungx]

For example, new, is_empty, len etc. can probably be made constant, which can be helpful for defining other constant functions that use SmartString.

I understand that SmartString is generic, so there would need to be separate constant implementations (all of them probably look exactly the same) for each function for each of the generic types.

[bodil]

The constructor as is can't be made const because it relies on MaybeUninit::zeroed(), but I'd be surprised if there isn't some way around that. The generic impls all allocate, of course, but as long as InlineString can be made const for at least the constructor, a lot of basic functionality should follow.

[bodil]

Oh, wait, no, this is actually blocking on const fns being unable to accept trait bounds on type variables. rust-lang/rust#57563

I'll leave this issue open to track progress on the above and implement it when it stabilises.

[schungx]

Well, technically speaking you don't need to stop at trait bounds.

You can do implementations for bound types:

impl SmartString<Compact> {
    pub const fn new() -> Self {
        ...
    }
}

Which means implementing the same functions for all trait combinations.

[bodil]

The problem is that InlineString needs a trait bound, but looking at the code now I'm not actually sure it needs to, it's really just for hedging against future implementations (like allowing you to configure the capacity of the inline string). Let me see if I can get rid of it...

[schungx]

That would be useful because we can then potentially put empty SmartStrings as constants, or even short constant strings if possible.

[bodil]

Ugh, well, I can remove the type parameter on InlineString but providing non-parameterised const impls for SmartString::new is proving to be a lot messier. It could be done but with a pretty severe amount of code duplication, I think it's probably better to wait until the above stabilises than introduce all that mess into the codebase.

On the other hand, it might be useful to expose InlineString in the meantime, it looks like a fair amount of useful functionality could be made const on it.

[schungx]

with a pretty severe amount of code duplication

That true. That's what I figured. How about a macro? Since all those implementations would not depend on the actual generic parameter and the code would all be the same.

[bodil]

There are a lot of trait impls which rely on SmartString::new(), and I'm a bit concerned about what kind a mess separating the config impls is going to make of the docs, but I'll give it a go.

[schungx]

It is not critical... right now it only affects making constants, which is not really a big deal, so I would say don't do it if it is too much of a mess.

I personally would keep new to be the same old API, but add new_const functions for selected common instance types.

And I'm not sure if it is impossible to make len etc. const since they rely on discriminant... It would be nice to have len and is_empty etc. be constant...

[bodil]

OK, after some experimentation, I can make a const empty constructor but I can't make a const constructor from a string slice (pending const ptr::copy_to() stabilising) and I can't make len() const without removing the memory corruption checks (pending panic!() in const fns stabilising).

I'm thinking I'll add the new_const() anyway, and simply deprecate it once trait bounds in const fns stabilises and one can just use new(). Additional const fns can be added once the relevant features stabilise.

[schungx]

I suppose without a const constructor from a string slice, it won't be very useful to have const len or const is_empty etc. So I suggest that's OK for now...

[schungx]

Kudos! Removing Mode from InlineString actually makes the code more streamlined and easier to read!