Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Prefer core and alloc over std where possible #6370

Closed
alice-i-cecile opened this issue Oct 26, 2022 · 3 comments · Fixed by #15281
Closed

Prefer core and alloc over std where possible #6370

alice-i-cecile opened this issue Oct 26, 2022 · 3 comments · Fixed by #15281
Labels
A-Cross-Cutting Impacts the entire engine C-Code-Quality A section of code that is hard to understand or change X-Controversial There is active debate or serious implications around merging this PR

Comments

@alice-i-cecile
Copy link
Member

What problem does this solve or what need does it fill?

Some less common platforms (e.g. Nintendo Switch, and likely other consoles), do not support the full Rust standard library.
Bevy uses a number of features from the standard library, but it's unclear exactly which parts need to be disabled (or replaced) in order to achieve these ports.

What solution would you like?

Add and follow the three lints listed in rust-lang/rust-clippy#9103.

This will have no effect on existing platforms (these are just re-exports), but will make it much clearer which crates (and features within crates) are no_std compatible.

We should enable these lints at the level of each crate in their lib.rs, in order to make the dev experience less frustrating due to desyncs with CI.

What alternative(s) have you considered?

Obviously, console devs can simply make these changes in their own fork. However, making this change comes at very little cost, and reduces the frustration of keeping forks synchronized.

Additional context

See this thread for discussion about no_std requirements for the official Switch target.
@alice-i-cecile alice-i-cecile added the C-Code-Quality A section of code that is hard to understand or change label Oct 26, 2022
@alice-i-cecile
Copy link
Member Author

I would add a O-Console label here, but we don't have one yet :P

@alice-i-cecile alice-i-cecile added the X-Controversial There is active debate or serious implications around merging this PR label Oct 26, 2022
@james7132
Copy link
Member

Real fast attempt applying this just to bevy_transform and bevy_time, found some quick blockers.

  • std::borrow::Cow only exists in std, no core equivalent exists
  • The lints fire on the PartialEq macro without no_std enabled

@PatchMixolydic
Copy link

Cow seems to be available in alloc as alloc::borrow::Cow.

@james7132 james7132 mentioned this issue Nov 13, 2022
Closed
@alice-i-cecile alice-i-cecile added the A-Cross-Cutting Impacts the entire engine label Mar 24, 2024
This was referenced Sep 18, 2024
Merged
Merged
github-merge-queue bot pushed a commit that referenced this issue Sep 18, 2024
@bushrat011899
Allow bevy_utils in no_std Contexts (#15279)
bd48906 
# Objective

- Adjust `bevy_utils` to make it `no_std` compatible
- Partially replaces #6581
- Contributes to #8161
- Contributes to #6370

## Solution

Added `alloc` and `std` features to `bevy_utils` (`std` is enabled by
default), allowing the crate's use in `no_std` contexts.

## Testing

- CI passed locally.
- Used `bevy_utils` in a `no_std` crate as an experiment and compiled
successfully.

## Migration Guide

If you were importing `bevy_utils` and setting `default_features` to
`false`, but relying on elements which are now gated behind the `std` or
`alloc` features, include the relevant feature in your `Cargo.toml`.

## Notes

- Bevy already includes a single `no_std` crate, `bevy_ptr`, so there is
precedent for this change.
- As `bevy_utils` is widely used across the rest of Bevy, further work
to make Bevy `no_std` compatible would be blocked on this crate, if such
work was to be undertaken.
- Most of the changes in this PR are just the removal of an unnecessary
call to `to_string()` within unit tests.
github-merge-queue bot pushed a commit that referenced this issue Sep 27, 2024
@bushrat011899 @mockersf
Add core and alloc over std Lints (#15281)
d70595b 
# Objective

- Fixes #6370
- Closes #6581

## Solution

- Added the following lints to the workspace:
  - `std_instead_of_core`
  - `std_instead_of_alloc`
  - `alloc_instead_of_core`
- Used `cargo +nightly fmt` with [item level use
formatting](https://rust-lang.github.io/rustfmt/?version=v1.6.0&search=#Item%5C%3A)
to split all `use` statements into single items.
- Used `cargo clippy --workspace --all-targets --all-features --fix
--allow-dirty` to _attempt_ to resolve the new linting issues, and
intervened where the lint was unable to resolve the issue automatically
(usually due to needing an `extern crate alloc;` statement in a crate
root).
- Manually removed certain uses of `std` where negative feature gating
prevented `--all-features` from finding the offending uses.
- Used `cargo +nightly fmt` with [crate level use
formatting](https://rust-lang.github.io/rustfmt/?version=v1.6.0&search=#Crate%5C%3A)
to re-merge all `use` statements matching Bevy's previous styling.
- Manually fixed cases where the `fmt` tool could not re-merge `use`
statements due to conditional compilation attributes.

## Testing

- Ran CI locally

## Migration Guide

The MSRV is now 1.81. Please update to this version or higher.

## Notes

- This is a _massive_ change to try and push through, which is why I've
outlined the semi-automatic steps I used to create this PR, in case this
fails and someone else tries again in the future.
- Making this change has no impact on user code, but does mean Bevy
contributors will be warned to use `core` and `alloc` instead of `std`
where possible.
- This lint is a critical first step towards investigating `no_std`
options for Bevy.

---------

Co-authored-by: François Mockers <francois.mockers@vleue.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-Cross-Cutting Impacts the entire engine C-Code-Quality A section of code that is hard to understand or change X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add core and alloc over std Lints bushrat011899/bevy
3 participants
@james7132 @alice-i-cecile @PatchMixolydic