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

RPIT hidden types can be ill-formed #114728

Closed
aliemjay opened this issue Aug 11, 2023 · 3 comments · Fixed by #115008 or #121679
Closed

RPIT hidden types can be ill-formed #114728

aliemjay opened this issue Aug 11, 2023 · 3 comments · Fixed by #115008 or #121679
Assignees
Labels
A-impl-trait Area: impl Trait. Universally / existentially quantified anonymous types with static dispatch. C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-high High priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@aliemjay
Copy link
Member

The following UB shouldn't compile:

type Static<'a> = &'static &'a ();
trait Extend<'a> {
    fn extend(self, _: &'a str) -> &'static str;
}
impl<'a> Extend<'a> for Static<'a> {
    fn extend(self, s: &'a str) -> &'static str {
        s
    }
}
fn boom<'a>(arg: Static<'_>) -> impl Extend<'a> {
    arg
}
fn main() {
    let y = boom(&&()).extend(&String::from("temporary"));
    println!("{}", y);
}

This regressed in v1.61. I guess it is #94081.

Discovered when investigating #114572.

@aliemjay aliemjay added the T-types Relevant to the types team, which will review and decide on the PR/issue. label Aug 11, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 11, 2023
@aliemjay aliemjay added regression-from-stable-to-stable Performance or correctness regression from one stable version to another. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness C-bug Category: This is a bug. E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Aug 11, 2023
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Aug 11, 2023
@aliemjay aliemjay added the A-impl-trait Area: impl Trait. Universally / existentially quantified anonymous types with static dispatch. label Aug 11, 2023
@aliemjay aliemjay removed the E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc label Aug 11, 2023
@compiler-errors compiler-errors self-assigned this Aug 11, 2023
@apiraino
Copy link
Contributor

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-critical

@rustbot rustbot added P-critical Critical priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Aug 12, 2023
@apiraino apiraino added P-high High priority and removed P-critical Critical priority labels Sep 28, 2023
@apiraino
Copy link
Contributor

Downgrading priority as per compiler meeting (notes)

@lcnr lcnr assigned aliemjay and unassigned compiler-errors Nov 15, 2023
bors added a commit to rust-lang-ci/rust that referenced this issue Feb 29, 2024
stricter hidden type wf-check [based on rust-lang#115008]

Original work by `@aliemjay` in rust-lang#115008. A huge thanks to them for originally figuring out this approach ❤️

Fixes rust-lang#114728
Fixes rust-lang#114572

Instead of adding the `WellFormed` obligations when relating opaque types, I add always emit such an obligation when defining the hidden type.

This causes nested opaque types which aren't wf to error, see the comment below for the described impact. I believe this change to be desirable as it significantly reduces complexity by removing special-cases.

It also caused an issue with RPITIT: in defaulted trait methods, we add a `Projection(synthetic_assoc, rpit_of_trait_method)` clause to the `param_env`. This clause is not added to the `ParamEnv` of the nested coroutines. This caused a normalization failure in `fn check_coroutine_obligations` with the new solver. I fixed that by using the env of the typeck root instead.

r? `@oli-obk`
@bors bors closed this as completed in 09bc67b Mar 6, 2024
bors added a commit to rust-lang/miri that referenced this issue Mar 6, 2024
stricter hidden type wf-check [based on #115008]

Original work by `@aliemjay` in #115008. A huge thanks to them for originally figuring out this approach ❤️

Fixes rust-lang/rust#114728
Fixes rust-lang/rust#114572

Instead of adding the `WellFormed` obligations when relating opaque types, we now always emit such an obligation when defining the hidden type.

This causes nested opaque types which aren't wf to error, see the comment below for the described impact. I believe this change to be desirable as it significantly reduces complexity by removing special-cases.

It also caused an issue with RPITIT: in defaulted trait methods, we add a `Projection(synthetic_assoc, rpit_of_trait_method)` clause to the `param_env`. This clause is not added to the `ParamEnv` of the nested coroutines. This caused a normalization failure in `fn check_coroutine_obligations` with the new solver. I fixed that by using the env of the typeck root instead.

r? `@oli-obk`
lnicola pushed a commit to lnicola/rust-analyzer that referenced this issue Apr 7, 2024
stricter hidden type wf-check [based on #115008]

Original work by `@aliemjay` in #115008. A huge thanks to them for originally figuring out this approach ❤️

Fixes rust-lang/rust#114728
Fixes rust-lang/rust#114572

Instead of adding the `WellFormed` obligations when relating opaque types, we now always emit such an obligation when defining the hidden type.

This causes nested opaque types which aren't wf to error, see the comment below for the described impact. I believe this change to be desirable as it significantly reduces complexity by removing special-cases.

It also caused an issue with RPITIT: in defaulted trait methods, we add a `Projection(synthetic_assoc, rpit_of_trait_method)` clause to the `param_env`. This clause is not added to the `ParamEnv` of the nested coroutines. This caused a normalization failure in `fn check_coroutine_obligations` with the new solver. I fixed that by using the env of the typeck root instead.

r? `@oli-obk`
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
stricter hidden type wf-check [based on #115008]

Original work by `@aliemjay` in #115008. A huge thanks to them for originally figuring out this approach ❤️

Fixes rust-lang/rust#114728
Fixes rust-lang/rust#114572

Instead of adding the `WellFormed` obligations when relating opaque types, we now always emit such an obligation when defining the hidden type.

This causes nested opaque types which aren't wf to error, see the comment below for the described impact. I believe this change to be desirable as it significantly reduces complexity by removing special-cases.

It also caused an issue with RPITIT: in defaulted trait methods, we add a `Projection(synthetic_assoc, rpit_of_trait_method)` clause to the `param_env`. This clause is not added to the `ParamEnv` of the nested coroutines. This caused a normalization failure in `fn check_coroutine_obligations` with the new solver. I fixed that by using the env of the typeck root instead.

r? `@oli-obk`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-impl-trait Area: impl Trait. Universally / existentially quantified anonymous types with static dispatch. C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-high High priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
5 participants