-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
Add core and alloc over std Lints
#15281
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.
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
Changes from 11 commits
a939e93
120e9e0
abb01eb
864631f
145ab01
8a99654
9e4c516
273cd3a
6960fd1
389310a
46aa899
f3c95da
ecc8d33
5c7fa79
270a49f
50e34bd
a6780ba
e28ff2a
f62b660
6bbca4e
cdbc80a
fff160e
4704134
a9e1664
12b5c66
e7efe08
b89e0d1
7a2b7a5
de8b0d6
8e453c7
a377ecb
6534168
546abdc
5418ce2
5d9b12f
99bb6f7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,7 @@ keywords = ["game", "engine", "gamedev", "graphics", "bevy"] | |
| license = "MIT OR Apache-2.0" | ||
| repository = "https://github.com/bevyengine/bevy" | ||
| documentation = "https://docs.rs/bevy" | ||
| rust-version = "1.79.0" | ||
| rust-version = "1.81.0" | ||
|
|
||
| [workspace] | ||
| exclude = [ | ||
|
|
@@ -45,6 +45,10 @@ ptr_as_ptr = "warn" | |
| ptr_cast_constness = "warn" | ||
| ref_as_ptr = "warn" | ||
|
|
||
| std_instead_of_core = "warn" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The downsides I can see are:
As such, before making such a change, it would be good to qualify exactly what it wins us. For example, getting closer to some potential Before merging, I'd want satisfying answers to:
I also question the viability of this effort for things like consoles when the majority of plugins and user code will still target
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for taking the time to look at this! As an immediate response, here's what I personally think in regards to those listed downsides:
Agreed, this will affect all contributors for Bevy going forward. However, I believe this is a relatively minor impact, as
This is true, effectively every crate root (
This is true and I don't have a meaningful rebuttal; this will add one layer of indirection between user and contributor code. What I would say is that we already have that for things like
I personally don't have access to any modern console SDKs so I can't comment on the necessity for a
Out of the above 5 crates, Once the above changes are made, I will be able to use Bevy on any platform, including the GameBoy Advance. What's more, I'd be able to use the high level abstractions like systems, queries, and the With the above context, here are my answers to your three questions:
Obviously this excludes other aspects of Bevy, (
To make a crate like
Yes, but I believe this introduces the very inconsistency in Bevy that we'd like to avoid. These lints make no restrictions around what code we write, or even how they're written, only the names of
I see this as a chicken-and-egg problem; Bevy's plugins are
For the modern consoles, definitely, but this would not be viable for anything retro. Additionally, with Bevy relying on the whole I apologise for the long response, especially considering just how large this PR itself is, and I thank you for taking the time to read it!
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I'd like to keep this if possible, I think with
Not long for this world frankly, the ECS folks kinda hate petgraph
Feature-flagging this is fine
Very doable.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, to provide some hard evidence for how close These changes allow this application to compile on `Cargo.toml`[package]
name = "bevy_no_std_test"
version = "0.1.0"
edition = "2021"
[dependencies]
bevy_app = { path = "../bevy_app", version = "0.15.0-dev", default-features = false }
bevy_ecs = { path = "../bevy_ecs", version = "0.15.0-dev", default-features = false }
[profile.dev]
opt-level = 3
debug = true
panic = "abort"`main.rs`//! Demonstrates a [`no_std`] Bevy application.
#![allow(unsafe_code)]
#![no_std]
#![no_main]
// Since we don't have access to `std`, and the `core` library can't assume there's
// a global allocator, we have to explicitly include it using `extern crate`.
extern crate alloc;
// Explicitly importing bevy sub-crates because I haven't threaded the `std` feature
// through bevy_internal and then bevy proper.
use bevy_app::prelude::*;
use bevy_ecs::prelude::*;
#[derive(Resource, Default)]
struct MyResource {
hello_world: bool,
}
#[derive(Component)]
struct Player;
fn main() {
App::empty()
.init_resource::<MyResource>()
.add_systems(Update, |my_resource: Res<MyResource>| {
assert_eq!(my_resource.hello_world, false);
})
.add_systems(Startup, |mut commands: Commands| {
commands.spawn(Player);
})
.add_systems(Update, |query: Query<&Player>| {
assert_eq!(query.iter().count(), 1);
})
.run();
}
/// You cannot rely on [`main`] being called for you in a `no_std` environment.
/// The exact name of your start function will depend on the platform you're developing for.
/// It's convention to name this function `_start`, and we use `#[no_mangle]` to ensure
/// the Rust compiler doesn't change this name.
#[no_mangle]
pub extern "C" fn _start() -> ! {
// Our start function is just going to call main, and if main exits we'll
// just loop forever. Quitting in a `no_std` context isn't defined, since
// you might be running on bare-metal hardware without power management!
main();
loop {}
}
/// Bevy requires access to a global allocator, another item which is platform specific
/// and so must be setup manually.
#[global_allocator]
static ALLOCATOR: allocator::Dummy = allocator::Dummy;
/// Obviously _your_ code will never panic. But, Rust still requires setting up
/// a panic handler _just in case_.
#[panic_handler]
fn panic(_info: &core::panic::PanicInfo) -> ! {
loop {}
}
/// Custom allocator for our no_std environment
mod allocator {
use alloc::alloc::{GlobalAlloc, Layout};
use core::ptr::null_mut;
/// This is a demonstration allocator and is _intentionally_ broken.
pub struct Dummy;
unsafe impl GlobalAlloc for Dummy {
unsafe fn alloc(&self, _layout: Layout) -> *mut u8 {
null_mut()
}
unsafe fn dealloc(&self, _ptr: *mut u8, _layout: Layout) {
panic!("dealloc should be never called")
}
}
}After applying the lints in this PR, it's about another 3000 diff-lines to make
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is a big win for bevy as the core 😉 of the ecosystem for a very small price.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok I'm convinced. Lets do this!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Brilliant! I'm going to resolve the merge conflicts now so hopefully this could be merged soon! |
||
| std_instead_of_alloc = "warn" | ||
| alloc_instead_of_core = "warn" | ||
|
Comment on lines
+51
to
+53
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These lints direct Bevy contributors to use |
||
|
|
||
| [workspace.lints.rust] | ||
| missing_docs = "warn" | ||
| unexpected_cfgs = { level = "warn", check-cfg = ['cfg(docsrs_dep)'] } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -86,7 +86,7 @@ fn generic_bench<P: Copy>( | |
|
|
||
| fn all_added_detection_generic<T: Component + Default>(group: &mut BenchGroup, entity_count: u32) { | ||
| group.bench_function( | ||
| format!("{}_entities_{}", entity_count, std::any::type_name::<T>()), | ||
| format!("{}_entities_{}", entity_count, core::any::type_name::<T>()), | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Most changes look like this, where some inline call to a |
||
| |bencher| { | ||
| bencher.iter_batched_ref( | ||
| || { | ||
|
|
@@ -110,8 +110,8 @@ fn all_added_detection_generic<T: Component + Default>(group: &mut BenchGroup, e | |
|
|
||
| fn all_added_detection(criterion: &mut Criterion) { | ||
| let mut group = criterion.benchmark_group("all_added_detection"); | ||
| group.warm_up_time(std::time::Duration::from_millis(500)); | ||
| group.measurement_time(std::time::Duration::from_secs(4)); | ||
| group.warm_up_time(core::time::Duration::from_millis(500)); | ||
| group.measurement_time(core::time::Duration::from_secs(4)); | ||
| for &entity_count in ENTITIES_TO_BENCH_COUNT { | ||
| generic_bench( | ||
| &mut group, | ||
|
|
@@ -129,7 +129,7 @@ fn all_changed_detection_generic<T: Component + Default + BenchModify>( | |
| entity_count: u32, | ||
| ) { | ||
| group.bench_function( | ||
| format!("{}_entities_{}", entity_count, std::any::type_name::<T>()), | ||
| format!("{}_entities_{}", entity_count, core::any::type_name::<T>()), | ||
| |bencher| { | ||
| bencher.iter_batched_ref( | ||
| || { | ||
|
|
@@ -158,8 +158,8 @@ fn all_changed_detection_generic<T: Component + Default + BenchModify>( | |
|
|
||
| fn all_changed_detection(criterion: &mut Criterion) { | ||
| let mut group = criterion.benchmark_group("all_changed_detection"); | ||
| group.warm_up_time(std::time::Duration::from_millis(500)); | ||
| group.measurement_time(std::time::Duration::from_secs(4)); | ||
| group.warm_up_time(core::time::Duration::from_millis(500)); | ||
| group.measurement_time(core::time::Duration::from_secs(4)); | ||
| for &entity_count in ENTITIES_TO_BENCH_COUNT { | ||
| generic_bench( | ||
| &mut group, | ||
|
|
@@ -179,7 +179,7 @@ fn few_changed_detection_generic<T: Component + Default + BenchModify>( | |
| let ratio_to_modify = 0.1; | ||
| let amount_to_modify = (entity_count as f32 * ratio_to_modify) as usize; | ||
| group.bench_function( | ||
| format!("{}_entities_{}", entity_count, std::any::type_name::<T>()), | ||
| format!("{}_entities_{}", entity_count, core::any::type_name::<T>()), | ||
| |bencher| { | ||
| bencher.iter_batched_ref( | ||
| || { | ||
|
|
@@ -208,8 +208,8 @@ fn few_changed_detection_generic<T: Component + Default + BenchModify>( | |
|
|
||
| fn few_changed_detection(criterion: &mut Criterion) { | ||
| let mut group = criterion.benchmark_group("few_changed_detection"); | ||
| group.warm_up_time(std::time::Duration::from_millis(500)); | ||
| group.measurement_time(std::time::Duration::from_secs(4)); | ||
| group.warm_up_time(core::time::Duration::from_millis(500)); | ||
| group.measurement_time(core::time::Duration::from_secs(4)); | ||
| for &entity_count in ENTITIES_TO_BENCH_COUNT { | ||
| generic_bench( | ||
| &mut group, | ||
|
|
@@ -227,7 +227,7 @@ fn none_changed_detection_generic<T: Component + Default>( | |
| entity_count: u32, | ||
| ) { | ||
| group.bench_function( | ||
| format!("{}_entities_{}", entity_count, std::any::type_name::<T>()), | ||
| format!("{}_entities_{}", entity_count, core::any::type_name::<T>()), | ||
| |bencher| { | ||
| bencher.iter_batched_ref( | ||
| || { | ||
|
|
@@ -252,8 +252,8 @@ fn none_changed_detection_generic<T: Component + Default>( | |
|
|
||
| fn none_changed_detection(criterion: &mut Criterion) { | ||
| let mut group = criterion.benchmark_group("none_changed_detection"); | ||
| group.warm_up_time(std::time::Duration::from_millis(500)); | ||
| group.measurement_time(std::time::Duration::from_secs(4)); | ||
| group.warm_up_time(core::time::Duration::from_millis(500)); | ||
| group.measurement_time(core::time::Duration::from_secs(4)); | ||
| for &entity_count in ENTITIES_TO_BENCH_COUNT { | ||
| generic_bench( | ||
| &mut group, | ||
|
|
@@ -308,7 +308,7 @@ fn multiple_archetype_none_changed_detection_generic<T: Component + Default + Be | |
| "{}_archetypes_{}_entities_{}", | ||
| archetype_count, | ||
| entity_count, | ||
| std::any::type_name::<T>() | ||
| core::any::type_name::<T>() | ||
| ), | ||
| |bencher| { | ||
| bencher.iter_batched_ref( | ||
|
|
@@ -356,8 +356,8 @@ fn multiple_archetype_none_changed_detection_generic<T: Component + Default + Be | |
|
|
||
| fn multiple_archetype_none_changed_detection(criterion: &mut Criterion) { | ||
| let mut group = criterion.benchmark_group("multiple_archetypes_none_changed_detection"); | ||
| group.warm_up_time(std::time::Duration::from_millis(800)); | ||
| group.measurement_time(std::time::Duration::from_secs(8)); | ||
| group.warm_up_time(core::time::Duration::from_millis(800)); | ||
| group.measurement_time(core::time::Duration::from_secs(8)); | ||
| for archetype_count in [5, 20, 100] { | ||
| for entity_count in [10, 100, 1000, 10000] { | ||
| multiple_archetype_none_changed_detection_generic::<Table>( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Required for
core::error::Error