Add new safety enum for inner extern items #124455

spastorino · 2024-04-27T23:09:58Z

This is in preparation for unsafe extern blocks that adds a safe variant for functions inside extern blocks.

r? @compiler-errors

rustbot · 2024-04-27T23:10:07Z

changes to the core type system

cc @compiler-errors, @lcnr

Some changes occurred in need_type_info.rs

cc @lcnr

This PR changes Stable MIR

cc @oli-obk, @celinval, @ouz-a

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

HIR ty lowering was modified

cc @fmease

rustbot · 2024-04-27T23:41:57Z

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

rustbot · 2024-04-28T01:37:59Z

Some changes occurred in src/librustdoc/clean/types.rs

cc @camelid

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

rustbot · 2024-04-28T02:31:30Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

rustbot · 2024-04-28T03:48:50Z

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

oli-obk · 2024-04-28T09:55:32Z

Why aren't we expanding the Unsafe enum instead? Is that too annoying for the places where Safe can't occur?

spastorino · 2024-04-28T14:43:14Z

Why aren't we expanding the Unsafe enum instead? Is that too annoying for the places where Safe can't occur?

My reasoning was not only this but also so things can be a bit safer as we won't need to handle safe when it basically can't happen.

spastorino · 2024-04-28T20:36:03Z

I'm having second thoughts about naming this FnSafety maybe just Safety is better as this is going to be applied to fns and statics.

compiler/rustc_ast/src/ast.rs

oli-obk · 2024-04-29T12:04:28Z

compiler/rustc_ast/src/ast.rs

+#[derive(HashStable_Generic)]
+pub enum FnSafety {
+    Unsafe(Span),
+    Default,


Maybe name this Inherited or FromContext. Without an explanation it's not obvious what this is supposed to be

Yeah, I've used Default as a way of saying no safety explicitly written means is going to take a default value.
Another possible name could be Implicit.
Out of all the values mentioned I slightly prefer Default but I don't mind changing it neither.
Another name we are using is Normal which I also think is not great, but it is another option.
Let's see if somebody else have some thought about this.

oli-obk · 2024-04-29T12:05:01Z

I'm having second thoughts about naming this FnSafety maybe just Safety is better as this is going to be applied to fns and statics.

Yea, just naming it Safety is fine as long as there's an explanation why it's different from Unsafe

bors · 2024-05-02T19:12:14Z

☔ The latest upstream changes (presumably #124621) made this pull request unmergeable. Please resolve the merge conflicts.

bors · 2024-05-04T00:45:48Z

☔ The latest upstream changes (presumably #123602) made this pull request unmergeable. Please resolve the merge conflicts.

celinval · 2024-05-06T18:09:07Z

compiler/stable_mir/src/mir/body.rs

@@ -913,10 +913,16 @@ pub enum Mutability {
    Mut,
 }

+#[derive(Copy, Clone, Debug, Eq, PartialEq)]
+pub enum Unsafe {


Can you please add documentation to these two enumerations? It isn't clear to me why we need two different enumerations here.

celinval · 2024-05-06T18:18:34Z

compiler/stable_mir/src/mir/body.rs

 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
 pub enum Safety {
    Unsafe,
-    Normal,
+    Default,


Don't we already know if a function / static is safe or unsafe by the time we are translating to StableMIR? If so, wouldn't it be better to keep Unsafe / Safe (Normal)?

I'm assuming that the way this is designed today, the Default value will require users to perform an extra query to know the context where this Fn / Static was declared to know whether it is safe or not.

BTW, this is just a suggestion. I know this is a big change already. If it makes sense, maybe just add a fixme comment for us to improve this later. Thanks

spastorino · 2024-05-13T15:15:31Z

I'm going to wait on a review and decision about this or #125077 or any other alternative before solving the conflicts and @celinval's thoughts as it is a lot of work to keep all these in a mergeable state.

spastorino · 2024-05-17T17:44:37Z

I think is better if for now we just do #125077

…=jackh726 Rename Unsafe to Safety Alternative to rust-lang#124455, which is to just have one Safety enum to use everywhere, this opens the posibility of adding `ast::Safety::Safe` that's useful for unsafe extern blocks. This leaves us today with: ```rust enum ast::Safety { Unsafe(Span), Default, // Safe (going to be added for unsafe extern blocks) } enum hir::Safety { Unsafe, Safe, } ``` We would convert from `ast::Safety::Default` into the right Safety level according the context.

…-blocks, r=compiler-errors Stabilize unsafe extern blocks (RFC 3484) # Stabilization report ## Summary This is a tracking issue for the RFC 3484: Unsafe Extern Blocks We are stabilizing `#![feature(unsafe_extern_blocks)]`, as described in [Unsafe Extern Blocks RFC 3484](rust-lang/rfcs#3484). This feature makes explicit that declaring an extern block is unsafe. Starting in Rust 2024, all extern blocks must be marked as unsafe. In all editions, items within unsafe extern blocks may be marked as safe to use. RFC: rust-lang/rfcs#3484 Tracking issue: rust-lang#123743 ## What is stabilized ### Summary of stabilization We now need extern blocks to be marked as unsafe and items inside can also have safety modifiers (unsafe or safe), by default items with no modifiers are unsafe to offer easy migration without surprising results. ```rust unsafe extern { // sqrt (from libm) may be called with any `f64` pub safe fn sqrt(x: f64) -> f64; // strlen (from libc) requires a valid pointer, // so we mark it as being an unsafe fn pub unsafe fn strlen(p: *const c_char) -> usize; // this function doesn't say safe or unsafe, so it defaults to unsafe pub fn free(p: *mut core::ffi::c_void); pub safe static IMPORTANT_BYTES: [u8; 256]; pub safe static LINES: SyncUnsafeCell<i32>; } ``` ## Tests The relevant tests are in `tests/ui/rust-2024/unsafe-extern-blocks`. ## History - rust-lang#124482 - rust-lang#124455 - rust-lang#125077 - rust-lang#125522 - rust-lang#126738 - rust-lang#126749 - rust-lang#126755 - rust-lang#126757 - rust-lang#126758 - rust-lang#126756 - rust-lang#126973 - rust-lang#127535 - rust-lang/rustfmt#6204 ## Unresolved questions I am not aware of any unresolved questions.

Rollup merge of rust-lang#127921 - spastorino:stabilize-unsafe-extern-blocks, r=compiler-errors Stabilize unsafe extern blocks (RFC 3484) # Stabilization report ## Summary This is a tracking issue for the RFC 3484: Unsafe Extern Blocks We are stabilizing `#![feature(unsafe_extern_blocks)]`, as described in [Unsafe Extern Blocks RFC 3484](rust-lang/rfcs#3484). This feature makes explicit that declaring an extern block is unsafe. Starting in Rust 2024, all extern blocks must be marked as unsafe. In all editions, items within unsafe extern blocks may be marked as safe to use. RFC: rust-lang/rfcs#3484 Tracking issue: rust-lang#123743 ## What is stabilized ### Summary of stabilization We now need extern blocks to be marked as unsafe and items inside can also have safety modifiers (unsafe or safe), by default items with no modifiers are unsafe to offer easy migration without surprising results. ```rust unsafe extern { // sqrt (from libm) may be called with any `f64` pub safe fn sqrt(x: f64) -> f64; // strlen (from libc) requires a valid pointer, // so we mark it as being an unsafe fn pub unsafe fn strlen(p: *const c_char) -> usize; // this function doesn't say safe or unsafe, so it defaults to unsafe pub fn free(p: *mut core::ffi::c_void); pub safe static IMPORTANT_BYTES: [u8; 256]; pub safe static LINES: SyncUnsafeCell<i32>; } ``` ## Tests The relevant tests are in `tests/ui/rust-2024/unsafe-extern-blocks`. ## History - rust-lang#124482 - rust-lang#124455 - rust-lang#125077 - rust-lang#125522 - rust-lang#126738 - rust-lang#126749 - rust-lang#126755 - rust-lang#126757 - rust-lang#126758 - rust-lang#126756 - rust-lang#126973 - rust-lang#127535 - rust-lang/rustfmt#6204 ## Unresolved questions I am not aware of any unresolved questions.

rustbot assigned compiler-errors Apr 27, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 27, 2024

spastorino force-pushed the add-new-fnsafety-enum branch 2 times, most recently from a01142f to 0a2dee6 Compare April 27, 2024 23:28

This comment has been minimized.

Sign in to view

spastorino force-pushed the add-new-fnsafety-enum branch from 0a2dee6 to 2cb0865 Compare April 27, 2024 23:41

This comment has been minimized.

Sign in to view

spastorino force-pushed the add-new-fnsafety-enum branch from 2cb0865 to 903fe08 Compare April 28, 2024 01:37

rustbot added the A-rustdoc-json Area: Rustdoc JSON backend label Apr 28, 2024

spastorino force-pushed the add-new-fnsafety-enum branch from 903fe08 to 7eb2f20 Compare April 28, 2024 01:44

This comment has been minimized.

Sign in to view

spastorino force-pushed the add-new-fnsafety-enum branch from 7eb2f20 to 5da6fe9 Compare April 28, 2024 02:31

This comment has been minimized.

Sign in to view

spastorino force-pushed the add-new-fnsafety-enum branch from 5da6fe9 to 10e8413 Compare April 28, 2024 03:48

spastorino mentioned this pull request Apr 28, 2024

Unsafe extern blocks #124482

Merged

oli-obk reviewed Apr 29, 2024

View reviewed changes

compiler/rustc_ast/src/ast.rs Outdated Show resolved Hide resolved

oli-obk reviewed Apr 29, 2024

View reviewed changes

spastorino force-pushed the add-new-fnsafety-enum branch from 10e8413 to 84138b2 Compare April 29, 2024 13:09

This comment has been minimized.

Sign in to view

spastorino force-pushed the add-new-fnsafety-enum branch from 84138b2 to 16bf3a7 Compare April 29, 2024 13:14

This comment has been minimized.

Sign in to view

spastorino force-pushed the add-new-fnsafety-enum branch 2 times, most recently from fdee834 to bd43f67 Compare April 29, 2024 13:55

This comment has been minimized.

Sign in to view

spastorino force-pushed the add-new-fnsafety-enum branch from bd43f67 to 0256365 Compare April 29, 2024 14:12

This comment has been minimized.

Sign in to view

spastorino force-pushed the add-new-fnsafety-enum branch 2 times, most recently from 3824cb3 to 6b76cac Compare April 29, 2024 17:40

spastorino changed the title ~~Add new fn safety enum for functions~~ Add new safety enum for inner extern items Apr 30, 2024

traviscross mentioned this pull request Apr 30, 2024

Tracking Issue for RFC 3484: Unsafe Extern Blocks #123743

Closed

10 tasks

spastorino force-pushed the add-new-fnsafety-enum branch from 6b76cac to e36f233 Compare May 3, 2024 14:11

This comment has been minimized.

Sign in to view

spastorino force-pushed the add-new-fnsafety-enum branch from e36f233 to 478644e Compare May 3, 2024 14:39

celinval reviewed May 6, 2024

View reviewed changes

spastorino mentioned this pull request May 13, 2024

Rename Unsafe to Safety #125077

Merged

spastorino added 2 commits May 16, 2024 17:32

Rename stable_mir::Safety to stable_mir::Unsafe

ba6f9aa

Add new safety enum to be used in inner extern block items

6e798b4

spastorino force-pushed the add-new-fnsafety-enum branch from 478644e to 6e798b4 Compare May 16, 2024 20:55

spastorino closed this May 17, 2024

spastorino mentioned this pull request Jul 18, 2024

Stabilize unsafe extern blocks (RFC 3484) #127921

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new safety enum for inner extern items #124455

Add new safety enum for inner extern items #124455

spastorino commented Apr 27, 2024

rustbot commented Apr 27, 2024

This comment has been minimized.

rustbot commented Apr 27, 2024

This comment has been minimized.

rustbot commented Apr 28, 2024

This comment has been minimized.

rustbot commented Apr 28, 2024

This comment has been minimized.

rustbot commented Apr 28, 2024

oli-obk commented Apr 28, 2024

spastorino commented Apr 28, 2024

spastorino commented Apr 28, 2024

oli-obk Apr 29, 2024

spastorino Apr 29, 2024 •

edited

Loading

oli-obk commented Apr 29, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bors commented May 2, 2024

This comment has been minimized.

bors commented May 4, 2024

celinval May 6, 2024

celinval May 6, 2024

celinval May 7, 2024

spastorino commented May 13, 2024 •

edited

Loading

spastorino commented May 17, 2024

Add new safety enum for inner extern items #124455

Add new safety enum for inner extern items #124455

Conversation

spastorino commented Apr 27, 2024

rustbot commented Apr 27, 2024

This comment has been minimized.

rustbot commented Apr 27, 2024

This comment has been minimized.

rustbot commented Apr 28, 2024

This comment has been minimized.

rustbot commented Apr 28, 2024

This comment has been minimized.

rustbot commented Apr 28, 2024

oli-obk commented Apr 28, 2024

spastorino commented Apr 28, 2024

spastorino commented Apr 28, 2024

oli-obk Apr 29, 2024

Choose a reason for hiding this comment

spastorino Apr 29, 2024 • edited Loading

Choose a reason for hiding this comment

oli-obk commented Apr 29, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bors commented May 2, 2024

This comment has been minimized.

bors commented May 4, 2024

celinval May 6, 2024

Choose a reason for hiding this comment

celinval May 6, 2024

Choose a reason for hiding this comment

celinval May 7, 2024

Choose a reason for hiding this comment

spastorino commented May 13, 2024 • edited Loading

spastorino commented May 17, 2024

spastorino Apr 29, 2024 •

edited

Loading

spastorino commented May 13, 2024 •

edited

Loading