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

Confusing diagnostic for invalid casts of pointers to trait objects #130030

Closed
WaffleLapkin opened this issue Sep 6, 2024 · 0 comments · Fixed by #130234
Closed

Confusing diagnostic for invalid casts of pointers to trait objects #130030

WaffleLapkin opened this issue Sep 6, 2024 · 0 comments · Fixed by #130234
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-raw-pointers Area: raw pointers, MaybeUninit, NonNull A-trait-objects Area: trait objects, vtable layout D-confusing Diagnostics: Confusing error or lint that should be reworked. D-terse Diagnostics: An error or lint that doesn't give enough information about the problem at hand. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@WaffleLapkin
Copy link
Member

WaffleLapkin commented Sep 6, 2024

Code

fn main() {
    let ptr: *const dyn Fn() = &|| ();
    _ = ptr as *const dyn Fn(u8);
}

Current output

error[E0308]: mismatched types
 --> src/main.rs:3:9
  |
3 |     _ = ptr as *const dyn Fn(u8);
  |         ^^^^^^^^^^^^^^^^^^^^^^^^ expected `()`, found `(u8,)`
  |
  = note: expected trait object `dyn Fn()`
             found trait object `dyn Fn(u8)`

Desired output

error[E0308]: invalid cast from `*const dyn Fn()` to `*const dyn Fn(u8)`
 --> src/main.rs:3:9
  |
3 |     _ = ptr as *const dyn Fn(u8);
  |         ^^^^^^^^^^^^^^^^^^^^^^^^ expected `()`, found `(u8,)`
  |
  = note: `Fn()` and `Fn(u8)` might have different vtables, leading to unsoundness
  = note: if you are absolutely sure you are using the resulting value safely, use `transmute`

Rationale and extra context

In #120248 I made checks for casting pointers to trait objects more strict: now they need to have the same generic arguments (this is due to the fact that for the cast to be safe, we need vtables to match).

The check is however made in a bit of a crude way, which results in this "mismatched types" diagnostic.

We should modify it so that the intent of the error is clear.

Rust Version

Reproduces on both stable 1.81.0 and 1.83.0-nightly (2024-09-05 9c01301).

@WaffleLapkin WaffleLapkin added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. D-confusing Diagnostics: Confusing error or lint that should be reworked. D-terse Diagnostics: An error or lint that doesn't give enough information about the problem at hand. A-raw-pointers Area: raw pointers, MaybeUninit, NonNull T-types Relevant to the types team, which will review and decide on the PR/issue. A-trait-objects Area: trait objects, vtable layout labels Sep 6, 2024
@bors bors closed this as completed in 75296fc Sep 25, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Sep 25, 2024
Rollup merge of rust-lang#130234 - lukas-code:ptr-cast-errors, r=WaffleLapkin

improve compile errors for invalid ptr-to-ptr casts with trait objects

This is a follow-up to rust-lang#120248 to improve some of its error messages.

1. Make the borrowcheck error for "type annotation requires that x must outlive y" actually point at the type annotation, i.e. the type `T` in a `x as T` cast. This makes the error more consistent with other errors caused by type annotation in other places, such as
```text
error: lifetime may not live long enough
 --> src/lib.rs:4:12
  |
3 | fn bar(a: &i32) {
  |           - let's call the lifetime of this reference `'1`
4 |     let b: &'static i32 = a;
  |            ^^^^^^^^^^^^ type annotation requires that `'1` must outlive `'static`
```

2. Don't say "cast" when we actually mean "coercion" and give borrowcheck errors from actual casts (which is currently just the check added in rust-lang#120248) a higher priority than ones from coercions. This can improve the errors for ptr-to-ptr cast between trait objects because they are are lowered as an upcast "unsizing" coercion if possible (which may be the identity upcast) followed by the actual cast.

3. Bring back the old "casting X as Y is invalid" message for type mismatch in the principals and reword the "vtable kinds may not match" message to more accurately describe the pointer metadata and not refer to "vtables" if the metadata is unknown.

fixes rust-lang#130030

r? `@WaffleLapkin` but feel free to reassign
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Sep 25, 2024
improve compile errors for invalid ptr-to-ptr casts with trait objects

This is a follow-up to rust-lang/rust#120248 to improve some of its error messages.

1. Make the borrowcheck error for "type annotation requires that x must outlive y" actually point at the type annotation, i.e. the type `T` in a `x as T` cast. This makes the error more consistent with other errors caused by type annotation in other places, such as
```text
error: lifetime may not live long enough
 --> src/lib.rs:4:12
  |
3 | fn bar(a: &i32) {
  |           - let's call the lifetime of this reference `'1`
4 |     let b: &'static i32 = a;
  |            ^^^^^^^^^^^^ type annotation requires that `'1` must outlive `'static`
```

2. Don't say "cast" when we actually mean "coercion" and give borrowcheck errors from actual casts (which is currently just the check added in rust-lang/rust#120248) a higher priority than ones from coercions. This can improve the errors for ptr-to-ptr cast between trait objects because they are are lowered as an upcast "unsizing" coercion if possible (which may be the identity upcast) followed by the actual cast.

3. Bring back the old "casting X as Y is invalid" message for type mismatch in the principals and reword the "vtable kinds may not match" message to more accurately describe the pointer metadata and not refer to "vtables" if the metadata is unknown.

fixes rust-lang/rust#130030

r? `@WaffleLapkin` but feel free to reassign
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-raw-pointers Area: raw pointers, MaybeUninit, NonNull A-trait-objects Area: trait objects, vtable layout D-confusing Diagnostics: Confusing error or lint that should be reworked. D-terse Diagnostics: An error or lint that doesn't give enough information about the problem at hand. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant