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

Compiler panic on type inference for impl Generator yielding a reference #52304

Closed
bergus opened this issue Jul 12, 2018 · 12 comments · Fixed by #99549
Closed

Compiler panic on type inference for impl Generator yielding a reference #52304

bergus opened this issue Jul 12, 2018 · 12 comments · Fixed by #99549
Labels
A-coroutines Area: Coroutines A-diagnostics Area: Messages for errors, warnings, and lints A-impl-trait Area: impl Trait. Universally / existentially quantified anonymous types with static dispatch. C-bug Category: This is a bug. E-needs-test Call for participation: Writing correctness tests. F-coroutines `#![feature(coroutines)]` I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@bergus
Copy link

bergus commented Jul 12, 2018

I'm getting

error: internal compiler error: librustc/infer/error_reporting/mod.rs:184: ReEmpty
thread 'main' panicked at 'Box', librustc_errors/lib.rs:554:9

when trying to compile

#![feature(generators, generator_trait)]
use std::ops::Generator;
fn example() -> impl Generator {
    || { yield &1 }
}

on rustc 1.29.0-nightly (e5f6498 2018-07-10) - Playground demo.

This seems to happen when I yield a reference type and don't specify the exact type in impl Generator<Yield=&Whatever>.

@kennytm kennytm added I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ A-coroutines Area: Coroutines T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels Jul 12, 2018
@Zoxc
Copy link
Contributor

Zoxc commented Aug 1, 2018

We don't generate an error message for the ReEmpty region here and panic instead. Here is the message that should be printed with a placeholder:

error[E0700]: hidden type for `impl Trait` captures lifetime that does not appear in bounds
 --> test-region.rs:3:17
  |
3 | fn example() -> impl Generator {
  |                 ^^^^^^^^^^^^^^
  |
  = note: hidden type `[generator@test-region.rs:4:5: 4:20 for<'r> {i32, &'r i32, ()}]` captures BUG[ReEmpty]

Should ReEmpty be allowed for impl Trait? cc @rust-lang/compiler

@Zoxc Zoxc added the A-impl-trait Area: impl Trait. Universally / existentially quantified anonymous types with static dispatch. label Aug 1, 2018
@estebank estebank added the A-diagnostics Area: Messages for errors, warnings, and lints label Aug 1, 2018
@arielb1
Copy link
Contributor

arielb1 commented Aug 4, 2018

Why are we capturing ReEmpty here?

@Zoxc
Copy link
Contributor

Zoxc commented Aug 6, 2018

There isn't any ReEmpty in the type. There's some scary code which replaces regions with ReEmpty which is closure related. I'm not sure what is going on there, but perhaps generators should be treated the same? cc @nikomatsakis

@eddyb
Copy link
Member

eddyb commented Aug 18, 2018

@Zoxc Oh, all of that code is for exporting impl Trait concrete types from inference results, which needs to "connect up" lifetimes inside the inferred concrete type with lifetime parameters.

@arielb1
Copy link
Contributor

arielb1 commented Aug 19, 2018

I think the problem is that the yield_ty of the generator ends up being &'ReEmpty u32, because nothing is constraining it to be anything else. I wonder why this isn't occurring with closures.

@arielb1
Copy link
Contributor

arielb1 commented Aug 19, 2018

This doesn't occur with closures because regionck forces the return type of the closure to outlive the closure's call scope, which prevents ReEmpty from being inferred.

@arielb1
Copy link
Contributor

arielb1 commented Aug 19, 2018

I think that the correct way to fix it would be to require all "implicit" projections in impl Trait types to be well-formed. that goes in the wrong direction.

@arielb1
Copy link
Contributor

arielb1 commented Aug 19, 2018

The requirement we want is that that "implicit" impl Generator for [generator type] makes sense.

That requires that the projections in that impl are alive as long as [generator type] is alive (we don't consider that as a "well-formed" requirement, but the "projection outlives" rules assume that). That does not hold if regions in the projections can be inferred to ReEmpty - we need to be checking that.

@arielb1
Copy link
Contributor

arielb1 commented Aug 19, 2018

I think that in closures, CallSite (from the fix to #29793) is serving that rule with lexical lifetimes - not sure what is checking that with NLL.

@arielb1 arielb1 added the I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness label Aug 19, 2018
@arielb1
Copy link
Contributor

arielb1 commented Aug 19, 2018

This can, in fact, be used as a soundness hole (assuming you can call resume):

#![feature(generators, generator_trait)]
use std::fmt::Debug;

use std::ops::Generator;
fn gen_taker<G: Generator>(mut g: G)
    where G::Yield: Debug, G::Return: Debug
{
    let a = unsafe { g.resume() };
    drop(g);
    println!("{:?}", a);
}
fn main() {
    gen_taker(|| {
        let x = 1;
        yield &x
    });
}

@arielb1
Copy link
Contributor

arielb1 commented Aug 19, 2018

Note: the version above is fixed by NLL. need to check whether fix is tight.

@Centril Centril added F-coroutines `#![feature(coroutines)]` requires-nightly This issue requires a nightly compiler in some way. labels Jul 28, 2019
@rossmacarthur
Copy link
Contributor

@arielb1 any ideas on how to check whether the "fix is tight"? Otherwise perhaps this issue can be closed?

@Elinvynia Elinvynia added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jun 9, 2020
@LeSeulArtichaut LeSeulArtichaut removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jun 9, 2020
@matthiaskrgr matthiaskrgr added the E-needs-test Call for participation: Writing correctness tests. label Feb 5, 2022
JohnTitor added a commit to JohnTitor/rust that referenced this issue Jul 21, 2022
Signed-off-by: Yuki Okushi <jtitor@2k36.org>
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Jul 21, 2022
…rrors

Add regression test for rust-lang#52304

Closes rust-lang#52304
r? `@compiler-errors`

Signed-off-by: Yuki Okushi <jtitor@2k36.org>
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jul 21, 2022
…rrors

Add regression test for rust-lang#52304

Closes rust-lang#52304
r? ``@compiler-errors``

Signed-off-by: Yuki Okushi <jtitor@2k36.org>
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Jul 21, 2022
…rrors

Add regression test for rust-lang#52304

Closes rust-lang#52304
r? ````@compiler-errors````

Signed-off-by: Yuki Okushi <jtitor@2k36.org>
bors added a commit to rust-lang-ci/rust that referenced this issue Jul 21, 2022
…askrgr

Rollup of 11 pull requests

Successful merges:

 - rust-lang#98707 (std: use futex-based locks on Fuchsia)
 - rust-lang#99413 (Add `PhantomData` marker for dropck to `BTreeMap`)
 - rust-lang#99454 (Add map_continue and continue_value combinators to ControlFlow)
 - rust-lang#99523 (Fix the stable version of `AsFd for Arc<T>` and `Box<T>`)
 - rust-lang#99526 (Normalize the arg spans to be within the call span)
 - rust-lang#99528 (couple of clippy::perf fixes)
 - rust-lang#99549 (Add regression test for rust-lang#52304)
 - rust-lang#99552 (Rewrite `orphan_check_trait_ref` to use a `TypeVisitor`)
 - rust-lang#99557 (Edit `rustc_index::vec::IndexVec::pick3_mut` docs)
 - rust-lang#99558 (Fix `remap_constness`)
 - rust-lang#99559 (Remove unused field in ItemKind::KeywordItem)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors closed this as completed in d425fe8 Jul 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-coroutines Area: Coroutines A-diagnostics Area: Messages for errors, warnings, and lints A-impl-trait Area: impl Trait. Universally / existentially quantified anonymous types with static dispatch. C-bug Category: This is a bug. E-needs-test Call for participation: Writing correctness tests. F-coroutines `#![feature(coroutines)]` I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.