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

Bad replacement for unsafe extern block suggestion #126756

Closed
ehuss opened this issue Jun 20, 2024 · 3 comments · Fixed by #126973
Closed

Bad replacement for unsafe extern block suggestion #126756

ehuss opened this issue Jun 20, 2024 · 3 comments · Fixed by #126973
Assignees
Labels
C-bug Category: This is a bug. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. F-unsafe_extern_blocks `#![feature(unsafe_extern_blocks)]`

Comments

@ehuss
Copy link
Contributor

ehuss commented Jun 20, 2024

The span for the suggestion to add an unsafe qualifier to an extern block ends up deleting the entire extern token.

#![feature(unsafe_extern_blocks)]

extern "C" {
    unsafe fn foo();
}

Generates this output:

error: items in unadorned `extern` blocks cannot have safety qualifiers
 --> src/main.rs:4:5
  |
3 | extern "C" {
  | ---------- help: add unsafe to this `extern` block
4 |     unsafe fn foo();
  |     ^^^^^^^^^^^^^^^^

When the suggestion is applied, the code results in:

#![feature(unsafe_extern_blocks)]

 {
    unsafe fn foo();
}

Although the suggestion is currently marked MaybeIncorrect, I think it should still provide something that is closer to being correct than to delete it. It also seems like this should be a relatively simple replacement that should support MachineApplicable. Ref:

#[suggestion(code = "", applicability = "maybe-incorrect")]
pub block: Span,

I expected to see this happen: Suggestion adds the unsafe keyword to the front of extern

Instead, this happened: Deletes the extern

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. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. 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
@chenyukang chenyukang self-assigned this Jun 23, 2024
@traviscross
Copy link
Contributor

cc @spastorino

@spastorino spastorino self-assigned this Jun 25, 2024
@spastorino
Copy link
Member

spastorino commented Jun 25, 2024

@chenyukang if you want to take this one, go ahead. Also assigned myself for tracking purposes but I can fix this easily too.

@chenyukang
Copy link
Member

@chenyukang if you want to take this one, go ahead. Also assigned myself for tracking purposes but I can fix this easily too.

sorry for the delay, submitted a PR #126973.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jun 26, 2024
…e-suggestion-error, r=spastorino

Fix bad replacement for unsafe extern block suggestion

Fixes rust-lang#126756

r? `@spastorino`

link rust-lang#123743
@bors bors closed this as completed in b272086 Jun 26, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jun 26, 2024
Rollup merge of rust-lang#126973 - chenyukang:yukang-fix-126756-unsafe-suggestion-error, r=spastorino

Fix bad replacement for unsafe extern block suggestion

Fixes rust-lang#126756

r? ``@spastorino``

link rust-lang#123743
@saethlin saethlin removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jun 29, 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. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. F-unsafe_extern_blocks `#![feature(unsafe_extern_blocks)]`
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants