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.

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

[Merged by Bors] - Make Query fields private #7149

Closed
wants to merge 2 commits into from

Conversation

BoxyUwU
Copy link
Member

@BoxyUwU BoxyUwU commented Jan 10, 2023

Query's fields being pub(crate) means that the struct can be constructed via safe code from anywhere in bevy_ecs . This is Not Good since it is intended that all construction of this type goes through Query::new which is an unsafe fn letting various Query methods rely on those invariants holding even though they can be trivially bypassed.

This has no user facing impact

@BoxyUwU BoxyUwU added A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change P-Unsound A bug that results in undefined compiler behavior labels Jan 10, 2023
@mockersf
Copy link
Member

bors try

bors bot added a commit that referenced this pull request Jan 10, 2023
@BoxyUwU BoxyUwU added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jan 10, 2023
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bors r+

bors bot pushed a commit that referenced this pull request Jan 10, 2023
`Query`'s fields being `pub(crate)` means that the struct can be constructed via safe code from anywhere in `bevy_ecs` . This is Not Good since it is intended that all construction of this type goes through `Query::new` which is an `unsafe fn` letting various `Query` methods rely on those invariants holding even though they can be trivially bypassed.

This has no user facing impact
@bors
Copy link
Contributor

bors bot commented Jan 10, 2023

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Jan 10, 2023
`Query`'s fields being `pub(crate)` means that the struct can be constructed via safe code from anywhere in `bevy_ecs` . This is Not Good since it is intended that all construction of this type goes through `Query::new` which is an `unsafe fn` letting various `Query` methods rely on those invariants holding even though they can be trivially bypassed.

This has no user facing impact
@bors
Copy link
Contributor

bors bot commented Jan 10, 2023

Build failed (retrying...):

@alice-i-cecile
Copy link
Member

bors r-

@bors
Copy link
Contributor

bors bot commented Jan 10, 2023

Canceled.

@alice-i-cecile
Copy link
Member

I think you need to rebase?

@BoxyUwU
Copy link
Member Author

BoxyUwU commented Jan 10, 2023

no it just tried to merge it with another PR that has been r+'d at the same time

@mockersf
Copy link
Member

now that #7150 has been merged, you need to rebase this PR. The test query_validates_world_id creates a query but without the field you added in this PR

@BoxyUwU BoxyUwU force-pushed the private_query_fields branch from 6369a58 to 27cd8e9 Compare January 10, 2023 17:50
@BoxyUwU
Copy link
Member Author

BoxyUwU commented Jan 10, 2023

this is what i get for adding a test </3

@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Jan 10, 2023
`Query`'s fields being `pub(crate)` means that the struct can be constructed via safe code from anywhere in `bevy_ecs` . This is Not Good since it is intended that all construction of this type goes through `Query::new` which is an `unsafe fn` letting various `Query` methods rely on those invariants holding even though they can be trivially bypassed.

This has no user facing impact
@bors
Copy link
Contributor

bors bot commented Jan 10, 2023

Build failed:

@BoxyUwU
Copy link
Member Author

BoxyUwU commented Jan 10, 2023

bors retry

bors bot pushed a commit that referenced this pull request Jan 10, 2023
`Query`'s fields being `pub(crate)` means that the struct can be constructed via safe code from anywhere in `bevy_ecs` . This is Not Good since it is intended that all construction of this type goes through `Query::new` which is an `unsafe fn` letting various `Query` methods rely on those invariants holding even though they can be trivially bypassed.

This has no user facing impact
@bors bors bot changed the title Make Query fields private [Merged by Bors] - Make Query fields private Jan 10, 2023
@bors bors bot closed this Jan 10, 2023
alradish pushed a commit to alradish/bevy that referenced this pull request Jan 22, 2023
`Query`'s fields being `pub(crate)` means that the struct can be constructed via safe code from anywhere in `bevy_ecs` . This is Not Good since it is intended that all construction of this type goes through `Query::new` which is an `unsafe fn` letting various `Query` methods rely on those invariants holding even though they can be trivially bypassed.

This has no user facing impact
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
`Query`'s fields being `pub(crate)` means that the struct can be constructed via safe code from anywhere in `bevy_ecs` . This is Not Good since it is intended that all construction of this type goes through `Query::new` which is an `unsafe fn` letting various `Query` methods rely on those invariants holding even though they can be trivially bypassed.

This has no user facing impact
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change P-Unsound A bug that results in undefined compiler behavior S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants