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

safe keyword is allowed in all function contexts #126749

Closed
ehuss opened this issue Jun 20, 2024 · 5 comments · Fixed by #126758
Closed

safe keyword is allowed in all function contexts #126749

ehuss opened this issue Jun 20, 2024 · 5 comments · Fixed by #126758
Labels
C-bug Category: This is a bug. F-unsafe_extern_blocks `#![feature(unsafe_extern_blocks)]`

Comments

@ehuss
Copy link
Contributor

ehuss commented Jun 20, 2024

The safe keyword appears to be allowed in all function contexts. However, the RFC explicitly says it should only be allowed in extern blocks:

The safe keyword is contextual and is currently allowed only within extern blocks.

#![feature(unsafe_extern_blocks)]

safe fn foo() {}

I expected to see this happen: Compile error

Instead, this happened: Compile success

cc @spastorino FYI

macro fragment

Additionally, it looks like safe fn foo(); is accepted in $item macro fragment specifier. I think there are different decisions to make here:

EDIT: Concluded below this should not be an issue.

Meta

rustc 1.81.0-nightly (59e2c01c2 2024-06-17)
binary: rustc
commit-hash: 59e2c01c2217a01546222e4d9ff4e6695ee8a1db
commit-date: 2024-06-17
host: aarch64-apple-darwin
release: 1.81.0-nightly
LLVM version: 18.1.7

Tracking:

@ehuss ehuss added C-bug Category: This is a bug. F-unsafe_extern_blocks `#![feature(unsafe_extern_blocks)]` labels Jun 20, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jun 20, 2024
@compiler-errors
Copy link
Member

Should $item allow safe functions at all? (I think "yes", since it already accepts extern-block items.)

Yes, I don't think we need to be doing any context-specific parsing of macro item fragments. Validation that safe is only placed on items in an extern block should be done post-expansion IMO. That lets us write:

macro_rules! wrap_in_extern { ($i:item) => { unsafe extern { $i } } }

wrap_in_extern! { safe fn foo(); }

Should that be split out into a separate fragment, such as $item_2021 does not allow safe, and $item does in 2024?

We already allow item macro fragment specifiers start with the safe identifier, since safe might begin a macro item e.g. safe::hello_world! {}, so I don't think this is necessary.

@compiler-errors
Copy link
Member

But to repeat myself just for clarity, I think we just need to do post-expansion validation of function signatures to guarantee that safe is only written on items in extern blocks.

@ehuss
Copy link
Contributor Author

ehuss commented Jun 20, 2024

Another issue appears that safe is not feature-gated. The following seems to build without a feature gate:

safe fn foo() {}

@compiler-errors
Copy link
Member

Thanks. I'll fix the feature gating at least, since it's regressed to beta. Let me break that out into another issue.

@spastorino
Copy link
Member

Thanks @compiler-errors your fix at least make this thing not compile. I'm going to fix the underlying issue that I think started to happen as I ended going to a scheme where we have hir::Safety (safe|unsafe) and we do not have a default variant. I'm pretty sure that this thing originally wasn't happening but I've failed to add tests for this.
Anyway, going to fix this.

workingjubilee added a commit to workingjubilee/rustc that referenced this issue Jun 21, 2024
…fe-blocks, r=compiler-errors

Do not allow safe/unsafe on static and fn items

Fixes rust-lang#126749

r? `@compiler-errors`

Tracking:

- rust-lang#123743
bors added a commit to rust-lang-ci/rust that referenced this issue Jun 21, 2024
…-blocks, r=compiler-errors

Do not allow safe/unsafe on static and fn items

Fixes rust-lang#126749

r? `@compiler-errors`

Tracking:

- rust-lang#123743
bors added a commit to rust-lang-ci/rust that referenced this issue Jun 22, 2024
…-blocks, r=compiler-errors

Do not allow safe/unsafe on static and fn items

Fixes rust-lang#126749

r? `@compiler-errors`

Tracking:

- rust-lang#123743
@saethlin saethlin removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jun 22, 2024
tgross35 added a commit to tgross35/rust that referenced this issue Aug 3, 2024
…-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.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 3, 2024
…-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.
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Aug 3, 2024
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. F-unsafe_extern_blocks `#![feature(unsafe_extern_blocks)]`
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants