-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Make asm label blocks safe context #131544
Conversation
`asm!()` is forced to be wrapped inside unsafe. If there's no special treatment, the label blocks would also always be unsafe with no way of opting out.
rustbot has assigned @petrochenkov. Use |
Out { expr: None, reg: _, late: _ } | ||
| Const { value: _, span: _ } | ||
| SymFn { value: _, span: _ } | ||
| SymStatic { def_id: _ } => {} |
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.
The default visitor uses visit_anon_const
and visit_qpath
in this place, don't you need them here?
To avoid potential mistakes, it probably makes sense to add a public fn walk_inline_asm_operand
helper to intravisit.rs.
match op {
Label { .. } => self.in_safety_context(SafetyContext::Safe, walk_inline_asm_operand(...)),
_ => walk_inline_asm_operand(...),
}
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.
This is the THIR visitor which doesn't have visit_anon_const and visit_qpath.
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.
ping @compiler-errors
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.
Sorry, pinged wrong person. @petrochenkov
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.
Sorry, I never noticed the update, it's better to set the S-waiting-on-review
label if a PR is waiting on review.
If this part cannot be improved, then it's good to go.
@bors r+ |
…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#130236 (unstable feature usage metrics) - rust-lang#131544 (Make asm label blocks safe context) - rust-lang#131586 (Support s390x z13 vector ABI) - rust-lang#132489 (Fix closure arg extraction in `extract_callable_info`, generalize it to async closures) - rust-lang#133078 (tests: ui/inline-consts: add issue number to a test, rename other tests) - rust-lang#133283 (Don't exclude relnotes from `needs-triage` label) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#131544 - nbdd0121:asm_goto_safe_block, r=petrochenkov Make asm label blocks safe context Tracking issue: rust-lang#119364 `asm!()` is forced to be wrapped inside unsafe. If there's no special treatment, the label blocks would also always be unsafe with no way of opting out. It was suggested that a simple fix is to make asm label blocks safe: rust-lang#119364 (comment). `@rustbot` labels: +A-inline-assembly +F-asm
Tracking issue: #119364
asm!()
is forced to be wrapped inside unsafe. If there's no special treatment, the label blocks would also always be unsafe with no way of opting out. It was suggested that a simple fix is to make asm label blocks safe: #119364 (comment).@rustbot labels: +A-inline-assembly +F-asm