matklad / once_cell

Rust library for single assignment cells and lazy statics without macros
Apache License 2.0
1.87k stars 109 forks source link

propagate critical-section feature selection into portable-atomic; other minor updates for v1.20.0 #260

Closed brodycj closed 1 month ago

brodycj commented 2 months ago

(updated)

CI build now fixed with TEMPORARY WORKAROUND in TEST_BETA task.


ORIGINAL RATIONALE RE: propagate critical-section feature selection into portable-atomic

Normally just using critical-section should be good enough for targets like super-outdated thumbv6m-none-eabi target. But in https://github.com/rustls/rustls/pull/2088 I would like to preserve existing use of alloc option, in which case I had to add explicit portable-atomic dependency with its critical-section feature enabled.

IMO it would be nice if I didn't have to add explicit sub-depencencies with this option. This is my proposal to enable critical-section for portable-atomic if portable-atomic is wanted.

Please let me know if you have any other idea or perspective concerning this.

matklad commented 2 months ago

Yes, this makes sense! Could you:

brodycj commented 2 months ago

I just force-pushed an update to update the Unreleased section of CHANGELOG.md, will try to fix the CI, update the minor version, and add the minor version heading to CHANGELOG.md next. Thanks!

brodycj commented 2 months ago

I have pushed some more updates that should resolve the CI build issues, bump the minor version, and update the change log.

I started using Option::unwrap_or_else as a cleaner way for result unwrapping to resolve the issues with recent MIRI versions & get rid of some unsafe blocks. I did use VS Code to inspect the Rust library code for Option::unwrap_or_else & Option::unwrap_unchecked; it looks to me like both cases are using match statements, with no difference in the expected Some arm.

I have also included a couple more minor updates:

@matklad it looks to me like there is quite a bit of duplicated code between the various OnceCell implementations; I am thinking it should be possible to reduce some of this duplicated code with some more extensive help from generics. I would like to try working on this kind of improvement if I can find some time, someday.

Many thanks for all of your efforts to offer & support this highly-useful package!

brodycj commented 2 months ago

ping @matklad

brodycj commented 2 months ago

@matklad any chance you can take a look?

brodycj commented 1 month ago

@matklad I think this should be ready now. A couple minor things I can try to fix next week if you like:

Thanks!

brodycj commented 1 month ago

@matklad I think this should be ready (yet again), with a TEMPORARY WORKAROUND in I made in the TEST_BETA task. Please let me know if you need anything else for this. Thanks!

matklad commented 1 month ago

published at https://crates.io/crates/once_cell/1.20.0

brodycj commented 1 month ago

🎉

kaspar030 commented 1 month ago
  • propagate critical-section feature selection into portable-atomic

This breaks our use case. portable-atomic treats critical-section and unsafe-assume-single-core as mutually exclusive. E.g., we depend on esp-rs, which for some platforms (e.g., esp32c2, c3) explicitly enables the latter.

Could we make this optional?

brodycj commented 1 month ago

I would like to state my assumption that despite issue #264, all other changes from this PR should be considered valid and should remain in master (and be part of the next release):

matklad please comment in case this is mistaken ... apologies for commenting on an already merged & closed PR!

P.S. This also updated Cargo.lock, assume we want to keep this update.