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

Further restricting what coercions are allowed on places of type ! #131297

Open
compiler-errors opened this issue Oct 5, 2024 · 6 comments
Open
Labels
A-coercions Area: implicit and explicit `expr as Type` coercions C-discussion Category: Discussion or questions that doesn't represent real issues. F-never_type `#![feature(never_type)]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-opsem Relevant to the opsem team

Comments

@compiler-errors
Copy link
Member

compiler-errors commented Oct 5, 2024

In #129392, I fixed a couple soundness holes having to do with the distinction between values and places and the ! type, which has special divergence and coercion behavior in HIR typeck.

To do so, I implemented a heuristic called expr_guaranteed_to_constitute_read_for_never (and same for patterns, pat_guaranteed...), which determines whether an expression/pattern is guaranteed to perform a read of a value of type !, which triggers the special divergence and coercion behavior. Read the description of the PR to see what the behavior is after the PR.

There's still some open questions about what expressions and patterns should be considered reads in the HIR (such as reading structs with only field subpatterns that don't read, like Struct { .. }), but I'd like to separate that since the PR #129392 is strictly an improvement over the existing behavior.

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Oct 5, 2024
@workingjubilee workingjubilee added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-discussion Category: Discussion or questions that doesn't represent real issues. T-opsem Relevant to the opsem team labels Oct 5, 2024
@fmease fmease added A-coercions Area: implicit and explicit `expr as Type` coercions F-never_type `#![feature(never_type)]` and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Oct 5, 2024
@compiler-errors
Copy link
Member Author

@lcnr also brought up a good point that ref/ref mut bindings probably also don't constitute reads. We don't allow coercions on let if ref/ref mut right now anyways, so it's less of an issue.

@RalfJung
Copy link
Member

RalfJung commented Oct 6, 2024

Cc @rust-lang/opsem

@digama0
Copy link
Contributor

digama0 commented Oct 6, 2024

The question about struct and 1-variant enum matches also leads to the following formulation of the question:

Is the following UB?

#[repr(u8)]
enum Foo {
    Foo,
}
fn main() {
    let x: Foo = unsafe { std::mem::uninitialized() };
    let Foo::Foo = x;
}

I think the answer is no, it is defined behavior to do a discriminant read on a 1-variant enum because nothing needs to be read and there is no data to read in the first place (Foo::Foo is all padding, so the fact that it is uninit makes no difference).

If we agree on this, then I think we should say that struct patterns, and 1-variant enum patterns, should not be considered reads in this HIR analysis.

@compiler-errors
Copy link
Member Author

If we agree on this, then I think we should say that struct patterns, and 1-variant enum patterns, should not be considered reads in this HIR analysis.

Yep, that's approximately what I was thinking.

@jwong101
Copy link
Contributor

jwong101 commented Oct 7, 2024

I think the answer is no, it is defined behavior to do a discriminant read on a 1-variant enum because nothing needs to be read and there is no data to read in the first place.

I could see that argument for repr(Rust) 1-variant enums. But the Foo enum has an explicit discriminant representation of type u8 that can only have 0 as it's value(the default discriminant value). Codegen currently exploits that by slapping range metadata on parameters+returns of type Foo.

@RalfJung
Copy link
Member

RalfJung commented Oct 8, 2024

Yeah I think that code is definitely UB on the let x: Foo = unsafe line. It doesn't have a padding byte, it has a 1-byte discriminant, and that must be initialized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-coercions Area: implicit and explicit `expr as Type` coercions C-discussion Category: Discussion or questions that doesn't represent real issues. F-never_type `#![feature(never_type)]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-opsem Relevant to the opsem team
Projects
None yet
Development

No branches or pull requests

7 participants