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

Safety Audit #77

Merged
merged 74 commits into from
May 18, 2022
Merged

Safety Audit #77

merged 74 commits into from
May 18, 2022

Conversation

Anders429
Copy link
Owner

This PR contains the safety audit conducted in #70. It also contains a fix to #75, fixing the trybuild tests. A few other things are thrown in as well; this one really took a while. Current me is cursing past me for not writing SAFETY comments when the code was originally written, especially considering how large and complicated this library has become.

@Anders429
Copy link
Owner Author

Looks like that trybuild test is still failing due to some kind of regression in the latest nightly. On that note, something should be done to make that error message better. It should only appear once, not once for every component included in the entities.

This was linked to issues May 17, 2022
@codecov-commenter
Copy link

codecov-commenter commented May 17, 2022

Codecov Report

Merging #77 (cd19907) into master (fb0e3bc) will increase coverage by 18.53%.
The diff coverage is 55.80%.

@@             Coverage Diff             @@
##           master      #77       +/-   ##
===========================================
+ Coverage   21.68%   40.22%   +18.53%     
===========================================
  Files          55       56        +1     
  Lines        4003     5907     +1904     
===========================================
+ Hits          868     2376     +1508     
- Misses       3135     3531      +396     
Impacted Files Coverage Δ
src/archetype/identifier/impl_serde.rs 0.00% <0.00%> (ø)
src/archetype/impl_debug.rs 0.00% <0.00%> (ø)
src/archetype/impl_eq.rs 0.00% <0.00%> (ø)
src/archetype/impl_serde.rs 0.00% <0.00%> (ø)
src/archetypes/impl_debug.rs 0.00% <0.00%> (ø)
src/archetypes/impl_eq.rs 0.00% <0.00%> (ø)
src/archetypes/iter.rs 0.00% <0.00%> (ø)
src/archetypes/par_iter.rs 0.00% <0.00%> (ø)
src/entities/mod.rs 0.00% <ø> (ø)
src/entities/seal/storage.rs 0.00% <0.00%> (ø)
... and 28 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fb0e3bc...cd19907. Read the comment docs.

@Anders429
Copy link
Owner Author

Turns out, the issue wasn't a regression in nightly at all. In order for rustc to include the path of a standard library function in the error output, the rust-src component needed to be included. This is why I was getting the $RUST/alloc/... path on my local machine, but not on the CI runs.

@Anders429 Anders429 merged commit d94a970 into master May 18, 2022
@Anders429 Anders429 deleted the safety branch May 18, 2022 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

trybuild test failure on nightly. Safety Audit
2 participants