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

wfcheck incorrectly assumes unnormalized types to be wf #100910

Closed
lcnr opened this issue Aug 23, 2022 · 3 comments · Fixed by #100989
Closed

wfcheck incorrectly assumes unnormalized types to be wf #100910

lcnr opened this issue Aug 23, 2022 · 3 comments · Fixed by #100989
Assignees
Labels
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-critical Critical priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@lcnr
Copy link
Contributor

lcnr commented Aug 23, 2022

struct Foo<T>(T);

trait GoodBye {
    type Forget;
}
impl<T> GoodBye for T {
    type Forget = ();
}

trait NeedsWf<'a, 'b> {
    type Assoc;
}

impl<'a, 'b> NeedsWf<'a, 'b> for Foo<<&'a &'b () as GoodBye>::Forget> {
    type Assoc = &'a &'b ();
}

fn main() {}

this compiles with #100676 because we now assume unnormalized types in the impl header to be well formed when computing implied bounds. We don't check that when using the impl so we should be able to transmute lifetimes with this, though I haven't tried that yet.

Going to fix that myself once I am back home in 2 weeks. cc @rust-lang/types

@lcnr lcnr added regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness C-bug Category: This is a bug. P-critical Critical priority T-types Relevant to the types team, which will review and decide on the PR/issue. labels Aug 23, 2022
@lcnr lcnr self-assigned this Aug 23, 2022
@lcnr lcnr added I-prioritize Issue: Indicates that prioritization has been requested for this issue. P-critical Critical priority and removed P-critical Critical priority I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Aug 23, 2022
@lcnr
Copy link
Contributor Author

lcnr commented Aug 23, 2022

assigning P-critical after a very long and deep discussion in https://rust-lang.zulipchat.com/#narrow/stream/245100-t-compiler.2Fwg-prioritization.2Falerts/topic/.23100910.20wfcheck.20incorrectly.20assumes.20unnormalized.20types.20to.20b.E2.80.A6

@pnkfelix
Copy link
Member

(FYI I also nominated PR #100676 for discussion at T-compiler meeting, essentially because of this associated issue; I hadn't seen that this was tagged as P-critical at that time. I'm leaving it nominated for now, just to ensure doubly that we discuss the matter.)

@lcnr
Copy link
Contributor Author

lcnr commented Aug 25, 2022

this is an existing issue on stable if we prevent the projection from normalizing in the impl while normalizing it when using the impl:

struct Foo<T>(T);

trait GoodBye {
    type Forget;
}
impl<T> GoodBye for T {
    type Forget = ();
}

trait NeedsWf<'a, 'b> {
    type Assoc;
}

impl<'a, 'b> NeedsWf<'a, 'b> for Foo<<&'a &'b () as GoodBye>::Forget>
where
    &'a &'b (): GoodBye, // this line prevents us from normalizing in the param env.
{
    type Assoc = &'a &'b ();
}

fn needs_wf<'a, 'b, T: NeedsWf<'a, 'b>>() {}

fn foo<'a: 'a, 'b: 'b>(_: &'b String) {
    needs_wf::<'a, 'b, Foo<()>>();
}

fn main() {
    let x = String::from("hello");
    foo::<'static, '_>(&x);
}

that's the same issue as #98543, which has been only fixed for function signatures

edit: that's #100051

@bors bors closed this as completed in 3b3f3b7 Aug 27, 2022
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. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-critical Critical priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. 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.

2 participants