Get rid of the smol_str dependency

[dthul]

Would it be possible to remove the dependency on smol_str? Or at least fix it to a specific minor version. rust-analyzer/smol_str#25 just broke my build (even with the latest stable compiler) and the author of smol_str clarified that smol_str will only support the latest compiler (not sure why latest stable doesn't work for me at the moment).

[imalsogreg]

How would we feel about replacing smol_str with smartstring.

• It's from Bodil, which is always a +1 in my experience.
• It seems to depend on memory layout details of the rust compiler, which is slightly scary, but there is a function in the library for validating up from that the assumption is valid, so there will not be a risk of corrupted data.
• It fills the same niece as smol_str - allocation free short Strings.
• Noting the caveat above, it's less toolchain-specific than smol_str.

I could put together PR's for both stripe-rs and async-stripe to do this migration.

[arlyon]

I think we have the opportunity to do two things here; one is to replace smol str and the second is to establish a minimum rust version and pin CI to it.

While I personally tend to use latest rust, I understand that not everyone can, and see no issue in making it an official guarantee of the API. Thoughts @seanpianka?

[imalsogreg]

SGTM.

How about the smartstring proposal specifically? Are we happy to move forward with that, perhaps doing a rust version pin at the same time, perhaps pinning later?

[seanpianka]

Sorry for the delay.

I don't have experience using either smol_str or smartstring directly, but I find it preferable to support several previous releases back from the latest. The aws-sdk-rust's README describes a particular approach to maintaining latest versions that I find to be a happy medium:

"The SDK currently requires a minimum of Rust 1.53, and is not guaranteed to build on compiler versions earlier than that. While we are still in alpha, we will be keeping the minimum compiler version two releases behind the latest stable release where possible (so if the latest stable is 1.55, we will be on 1.53). However, we are not making any guarantees around this at present. Increases in minimum required Rust version will be called out in the Release Notes for new releases of the SDK."

This seems like a sane approach that avoids requiring stripe-rs users to upgrade their toolchain with every new release of stripe-rs and Rust.

I ran into a similar issue recently, in fact. A project of mine uses a 1.54 feature, and I noticed that a user forked the project just to remove a single doc comment from the src/lib.rs. That's not a fantastic user experience, and we can avoid similar scenarios by adopting a policy of supporting N-3 versions of the compiler (or N-M, with whatever we decide M to be).

"How about the smartstring proposal specifically?"

One thing to note here is that smol_str has no unsafe code, whereas smartstring does have a nontrivial amount of unsafe and reliance on implementation details of the compiler. Perhaps, instead of introducing new unsafe code, what about simply pinning the dependency to a minor version? That would avoid the need for any refactoring.

Edit: I skimmed through fasterthanlime's analysis of the two crates which, while possibly outdated or fixed by now, specifies that the clone for smol_str is O(1) whereas the clone for smartstring is O(n). With how much we make use of these more efficient string types, perhaps we should also account for the complexity of some common operations.

[arlyon]

I am personally less inclined to pin the minor version purely because it means people will more likely end up with multiple copies of the library in their binary. I remember on the reddit discussion that matklad actually recommended smartstring. I'll find it:

Thanks for teaching me about SmartString, it looks nice!

People should probably prefer that to SmolStr, as the latter is only really intended for use inside Rust analyzer, and doesn’t try to be a good general purpose library.

So, my vote: move to smartstring (though I agree the unsafe is annoying), and pin to N-3 which, as of 1.56 release today, means 1.53. This should be set in CI which I believe is currently broken on this repo.

I think migrating CI to github actions should probably happen fairly soon as well to support this guarantee.

[imalsogreg]

@arlyon When you say pin to a version of rust - are you just talking about stripe-rs's CI? Or do you mean that the library will stop supporting earlier versions? Selfishly we are on 1.52 at work :D

[arlyon]

Yes, just in CI. Obviously things should still 'just work' with earlier rust versions it just means we'll be less stringent with accepting prs that use language features older than the MSRV.

Out of interest, I tried to do a crude 'bisect' to figure out the lowest compiler version that actually compiles, and it turns out to be 1.49 (on async-stripe) so we might as well just start there actually, and consider upping it if we need newer language features later on.