[Coding Rule]: assure visibility of unsafe keyword in unsafe code#358
Conversation
✅ Deploy Preview for scrc-coding-guidelines ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Updated guideline to ensure visibility of unsafe tokens in unsafe code, reflecting changes in Rust Edition 2024. Added rationale and examples to clarify requirements.
Update guideline to ensure visibility of 'unsafe' keyword in unsafe code, reflecting changes in Rust Edition 2024.
Clarify issues with using #[export_name] and #[link_section] attributes without unsafe wrappers in Rust 2024.
Removed visibility section and updated examples for compliance with Rust Edition 2024. Clarified unsafe usage in function declarations and attributes.
Removed exceptions section and added safety notes for unsafe attributes.
d8d653b to
e1044d5
Compare
|
👋 Hey @darkwisebear! You've been assigned to review this coding guideline PR. Your Role as ReviewerAs outlined in our contribution guide, please:
Review Checklist
Bot CommandsIf you need to pass this review:
To assign someone else:
Other commands:
|
darkwisebear
left a comment
There was a problem hiding this comment.
Looks good, I'd be in favor of merging, the comments are merely nits and minor things. I can't click the stuff in the checklist, consider them clicked.
|
|
||
| unsafe extern "C" { | ||
| // Here the assumption is that malloc is the one defined by C's stdlib.h | ||
| // and that size_of::<usize>() == size_of::<size_t>() |
There was a problem hiding this comment.
May be a bit picky, but this should probably be asserted on, as in const { assert!(std::mem::size_of::<usize>() == std::mem::size_of::<libc::size_t>()); } or just replace usize with libc::size_t to not contradict to #225.
| :id: gui_ZDLZzjeOwLSU | ||
| :category: required | ||
| :status: draft | ||
| :release: 1.85-latest |
There was a problem hiding this comment.
Afaiu this rule is only meaningful for edition <2024. If this is correct, should we add this to the metadata in this section? Would we need a new metdata field for this?
| * **Static Analysis Tools**: | ||
|
|
||
| * ``cargo-geiger`` - Counts and reports unsafe code usage | ||
| * ``cargo-audit`` - Checks for known vulnerabilities in dependencies | ||
| * Custom Clippy lints for project-specific requirements |
There was a problem hiding this comment.
While this part is informative, it's not helping enforcing the rule. I think this should be moved from this section.
|
✅ Rectified PR #358: latest review by @darkwisebear is |
plaindocs
left a comment
There was a problem hiding this comment.
Approving on behalf of @darkwisebear to get this merged.
|
✅ Rectified PR #358: latest review by @darkwisebear is |
|
@darkwisebear please open an issue with the remaining nits 🙇♂️ |
1c7d6bb
Created PR for #252 by @workingjubilee