Skip to content

NLL and closures: higher-ranked lifetime bounds are not enforced in type tests #98693

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
aliemjay opened this issue Jun 29, 2022 · 4 comments · Fixed by #98713
Closed

NLL and closures: higher-ranked lifetime bounds are not enforced in type tests #98693

aliemjay opened this issue Jun 29, 2022 · 4 comments · Fixed by #98713
Assignees
Labels
A-NLL Area: Non-lexical lifetimes (NLL) 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-beta Performance or correctness regression from stable to beta. T-types Relevant to the types team, which will review and decide on the PR/issue.
Milestone

Comments

@aliemjay
Copy link
Member

aliemjay commented Jun 29, 2022

This code compiles after #95565 although it shouldn't: (playground)

fn assert_static<T>() where for<'a> T: 'a, {}

fn test<T>() {
    || {
        assert_static::<T>();
    };
}

This is similar to #98095 in that we're not taking into account placeholder regions when "promoting" type-tests from the closure to its caller:

for ur in self.scc_values.universal_regions_outlived_by(r_scc) {

@rustbot label T-types regression-from-stable-to-beta C-bug A-NLL I-unsound

@rustbot rustbot added A-NLL Area: Non-lexical lifetimes (NLL) C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-types Relevant to the types team, which will review and decide on the PR/issue. I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jun 29, 2022
@aliemjay
Copy link
Member Author

I guess it would be an easy fix
@rustbot claim

@steffahn
Copy link
Member

The unsoundness exploitation in #98095 can easily be adapted for this issue, so it is in fact a soundness issue: playground

@Mark-Simulacrum Mark-Simulacrum added this to the 1.63.0 milestone Jun 30, 2022
@jackh726 jackh726 added the I-types-nominated Nominated for discussion during a types team meeting. label Jun 30, 2022
@apiraino
Copy link
Contributor

issue touched in today's T-compiler meeting, notes from Zulip chat. Will also be discussed tomorrow during the T-types meeting.

@apiraino
Copy link
Contributor

apiraino commented Jul 4, 2022

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 Jul 4, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jul 10, 2022
promote placeholder bounds to 'static obligations

In NLL, when we are promoting a bound out from a closure, if we have a requirement that `T: 'a` where `'a` is in a higher universe, we were previously ignoring that, which is totally wrong. We should be promoting those constraints to `'static`, since universes are not expressible across closure boundaries.

Fixes rust-lang#98693

~~(Marking as WIP because I'm still running tests, haven't add the new test, etc)~~

r? `@jackh726`
@bors bors closed this as completed in 2cb7d1c Jul 11, 2022
@jackh726 jackh726 removed the I-types-nominated Nominated for discussion during a types team meeting. label Jul 29, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-NLL Area: Non-lexical lifetimes (NLL) 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-beta Performance or correctness regression from stable to beta. 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.

6 participants