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

Impossible to return unboxed closure from some functions. #66426

Closed
wdanilo opened this issue Nov 14, 2019 · 11 comments
Closed

Impossible to return unboxed closure from some functions. #66426

wdanilo opened this issue Nov 14, 2019 · 11 comments
Labels
F-trait_alias `#![feature(trait_alias)]` F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` 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. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@wdanilo
Copy link

wdanilo commented Nov 14, 2019

Hi! I'm trying to write a lens library for Rust (see this for reference about lenses: https://github.com/ekmett/lens/wiki). I'm trying to write it in an zero-overhead style, so all lens-usages could be optimized away during compilation stage, thus, I'm trying to create unboxed closures and glue them together. However, a very strange error occurs here. Consider the following code:

#![feature(type_alias_impl_trait)]
#![feature(trait_alias)]

use std::marker::PhantomData;

trait OptGetter<'t,A,B> = Fn(&'t A) -> Option<&'t B>
    where A:'t, B:'t, Self:Copy;

struct Lens<'t,Get,A,B>
    where Get: OptGetter<'t,A,B> {
    get: Get,
    phantom: PhantomData<(&'t(),A,B)>
}

type CombinedOptGetter<'t,Get1,Get2,A,B,C> = impl OptGetter<'t,A,C>;
fn combine_opt_getter <'t,Get1,Get2,A,B,C>
(f1: Get1, f2: Get2) -> CombinedOptGetter<'t,Get1,Get2,A,B,C>
    where Get1:OptGetter<'t,A,B>,
          Get2:OptGetter<'t,B,C> {
    move |a| f1(a).and_then(f2)
}

impl<'t,Get,A,B> Lens<'t,Get,A,B>
    where Get: OptGetter<'t,A,B> {
    pub fn new(get: Get) -> Self {
        let phantom = PhantomData;
        Self { get, phantom }
    }

    pub fn combine<Get2, C>(self, that: Lens<'t,Get2,B,C>)
    -> Lens<'t,CombinedOptGetter<'t,Get,Get2,A,B,C>,A,C>
        where Get2: OptGetter<'t,B,C> {
        Lens::new(combine_opt_getter(self.get, that.get))
    }
}

trait OptionLens {
    type Out;
    fn _Some(self) -> Self::Out;
}

type SomeGetter<'t,A> = impl OptGetter<'t,Option<A>,A>;
fn some_getter<'t,A:'t>() -> Lens<'t,SomeGetter<'t,A>,Option<A>,A> {
    Lens::new(move|t: &'t Option<A>| t.as_ref())
}

fn _Some<'t, Get, A, B> (this: Lens<'t,Get,A,Option<B>>) -> i32
where Get: OptGetter<'t,A,Option<B>> {
    let wrapped: Lens<'t,SomeGetter<'t,B>,Option<B>,B> = some_getter();
    let wrapper: Lens<'t,CombinedOptGetter<'t,Get,SomeGetter<'t,B>,A,Option<B>,B>,A,B> = this.combine(wrapped);
    7
}

It compiles fine. However, if you try to return wrapper from the _Some function, the code will not compile and the errrors we get are not very explanatory either. With this code instead:

fn _Some<'t, Get, A, B> (this: Lens<'t,Get,A,Option<B>>) -> Lens<'t,CombinedOptGetter<'t,Get,SomeGetter<'t,B>,A,Option<B>,B>,A,B>
where Get: OptGetter<'t,A,Option<B>> {
    let wrapped: Lens<'t,SomeGetter<'t,B>,Option<B>,B> = some_getter();
    let wrapper: Lens<'t,CombinedOptGetter<'t,Get,SomeGetter<'t,B>,A,Option<B>,B>,A,B> = this.combine(wrapped);
    wrapper
}

We get:

error[E0283]: type annotations needed: cannot resolve `_: OptGetter<'t, std::option::Option<B>, B>`
  --> lib/optics/src/lib.rs:42:1
   |
42 | type SomeGetter<'t,A> = impl OptGetter<'t,Option<A>,A>;
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: the return type of a function must have a statically known size

error: concrete type differs from previous defining opaque type use
  --> lib/optics/src/lib.rs:47:1
   |
47 | / fn _Some<'t, Get, A, B> (this: Lens<'t,Get,A,Option<B>>) -> Lens<'t,CombinedOptGetter<'t,Get,SomeGetter<'t,B>,A,Option<B>,B>,A,B>
48 | | where Get: OptGetter<'t,A,Option<B>> {
49 | |     let wrapped: Lens<'t,SomeGetter<'t,B>,Option<B>,B> = some_getter();
50 | |     let wrapper: Lens<'t,CombinedOptGetter<'t,Get,SomeGetter<'t,B>,A,Option<B>,B>,A,B> = this.combine(wrapped);
51 | |     wrapper
52 | | }
   | |_^ expected `[closure@lib/optics/src/lib.rs:44:15: 44:48]`, got `[type error]`
   |
note: previous use here
  --> lib/optics/src/lib.rs:43:1
   |
43 | / fn some_getter<'t,A:'t>() -> Lens<'t,SomeGetter<'t,A>,Option<A>,A> {
44 | |     Lens::new(move|t: &'t Option<A>| t.as_ref())
45 | | }
   | |_^
@Aaron1011
Copy link
Member

Minimized:

#![feature(type_alias_impl_trait)]
#![feature(trait_alias)]

type Foo = impl Copy;

enum Wrapper<T> {
    First(T),
    Second
}

fn produce() -> Wrapper<Foo> {
    Wrapper::Second
}

This appears to be caused by the fact that we create a new inference variable for Foo from Wrapper<Foo>, but never unify it because produce does not constrain the opaque type.

As a side note, we should probably be cheking for type errors when we collect opaque type uses, so that we don't print useless errors like:

expected `[closure@lib/optics/src/lib.rs:44:15: 44:48]`, got `[type error]`

@wdanilo
Copy link
Author

wdanilo commented Nov 15, 2019

@Aaron1011 thanks for tracking it down. Tbh, I don't see why your code is a minimized version of mine – I don't return from any function variant without the impl trait. This is just a note – I don't know the internals of rustc so I can miss a lot of info here :)

@Aaron1011
Copy link
Member

I've opened #66431. It addresses the first 'layer' of errors, but there appear to more (thankfully, better) errors that appear after this one is no longer emitted.

@Centril Centril added F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` 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. T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Nov 15, 2019
Aaron1011 added a commit to Aaron1011/rust that referenced this issue Nov 18, 2019
Related: rust-lang#66426

This commit adds handling for opaque types during inference variable
fallback. Type variables generated from the instantiatino of opaque
types now fallback to the opque type itself.

Normally, the type variable for an instantiated opaque type is either
unified with the concrete type, or with the opaque type itself (e.g when
a function returns an opaque type by calling another function).

However, it's possible for the type variable to be left completely
unconstrained. This can occur in code like this:

```rust
pub type Foo = impl Copy;
fn produce() -> Option<Foo> {
    None
}
```

Here, we'll instantatiate the `Foo` in `Option<Foo>` to a fresh type
variable, but we will never unify it with anything due to the fact
that we return a `None`.

This results in the error message:

`type annotations needed: cannot resolve `_: std::marker::Copy``

pointing at `pub type Foo = impl Copy`.

This message is not only confusing, it's incorrect. When an opaque type
inference variable is completely unconstrained, we can always fall back
to using the opaque type itself. This effectively turns that particular
use of the opaque type into a non-defining use, even if it appears in a
defining scope.
JohnTitor added a commit to JohnTitor/rust that referenced this issue Nov 19, 2019
…varkor

Fix 'type annotations needed' error with opaque types

Related: rust-lang#66426

This commit adds handling for opaque types during inference variable
fallback. Type variables generated from the instantiation of opaque
types now fallback to the opaque type itself.

Normally, the type variable for an instantiated opaque type is either
unified with the concrete type, or with the opaque type itself (e.g when
a function returns an opaque type by calling another function).

However, it's possible for the type variable to be left completely
unconstrained. This can occur in code like this:

```rust
pub type Foo = impl Copy;
fn produce() -> Option<Foo> {
    None
}
```

Here, we'll instantatiate the `Foo` in `Option<Foo>` to a fresh type
variable, but we will never unify it with anything due to the fact
that we return a `None`.

This results in the error message:
```
type annotations needed: cannot resolve `_: std::marker::Copy
```

pointing at `pub type Foo = impl Copy`.

This message is not only confusing, it's incorrect. When an opaque type
inference variable is completely unconstrained, we can always fall back
to using the opaque type itself. This effectively turns that particular
use of the opaque type into a non-defining use, even if it appears in a
defining scope.
Centril added a commit to Centril/rust that referenced this issue Nov 19, 2019
…varkor

Fix 'type annotations needed' error with opaque types

Related: rust-lang#66426

This commit adds handling for opaque types during inference variable
fallback. Type variables generated from the instantiation of opaque
types now fallback to the opaque type itself.

Normally, the type variable for an instantiated opaque type is either
unified with the concrete type, or with the opaque type itself (e.g when
a function returns an opaque type by calling another function).

However, it's possible for the type variable to be left completely
unconstrained. This can occur in code like this:

```rust
pub type Foo = impl Copy;
fn produce() -> Option<Foo> {
    None
}
```

Here, we'll instantatiate the `Foo` in `Option<Foo>` to a fresh type
variable, but we will never unify it with anything due to the fact
that we return a `None`.

This results in the error message:
```
type annotations needed: cannot resolve `_: std::marker::Copy
```

pointing at `pub type Foo = impl Copy`.

This message is not only confusing, it's incorrect. When an opaque type
inference variable is completely unconstrained, we can always fall back
to using the opaque type itself. This effectively turns that particular
use of the opaque type into a non-defining use, even if it appears in a
defining scope.
@Aaron1011
Copy link
Member

This now compiles on the latest nightly.

@Aaron1011
Copy link
Member

Actually, this does not compile - I didn't use the modified _Some function when testing.

@jackh726
Copy link
Member

jackh726 commented Feb 4, 2021

Errors on current master:

error: non-defining opaque type use in defining scope
  --> src/main.rs:64:6
   |
64 | ) -> Lens<'t, CombinedOptGetter<'t, Get, SomeGetter<'t, B>, A, Option<B>, B>, A, B>
   |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
note: used non-generic type `impl OptGetter<'t, Option<B>, B>` for generic parameter
  --> src/main.rs:20:34
   |
20 | type CombinedOptGetter<'t, Get1, Get2, A, B, C> = impl OptGetter<'t, A, C>;
   |                                  ^^^^

error: non-defining opaque type use in defining scope
  --> src/main.rs:64:6
   |
64 | ) -> Lens<'t, CombinedOptGetter<'t, Get, SomeGetter<'t, B>, A, Option<B>, B>, A, B>
   |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
note: used non-generic type `Option<B>` for generic parameter
  --> src/main.rs:20:43
   |
20 | type CombinedOptGetter<'t, Get1, Get2, A, B, C> = impl OptGetter<'t, A, C>;
   |                                           ^

error: aborting due to 2 previous errors

@oli-obk oli-obk added the F-trait_alias `#![feature(trait_alias)]` label Feb 19, 2021
@nikomatsakis
Copy link
Contributor

nikomatsakis commented Jun 29, 2021

For min_type_alias_impl_trait, I believe this is covered by #86732 I was mistaken

@nikomatsakis
Copy link
Contributor

The minimized example (playground:

#![feature(min_type_alias_impl_trait)]

type Foo = impl Copy;

enum Wrapper<T> {
    First(T),
    Second
}

fn produce() -> Wrapper<Foo> {
    Wrapper::Second
}

gives what appears to me to be the correct error:

error: could not find defining uses
 --> src/lib.rs:3:12
  |
3 | type Foo = impl Copy;
  |            ^^^^^^^^^

but the diagnostics are not very good.

@nikomatsakis
Copy link
Contributor

In particular, the value of Foo is not constrained at all by this function. If you extend to

#![feature(min_type_alias_impl_trait)]

type Foo = impl Copy;

enum Wrapper<T> {
    First(T),
    Second
}

fn produce() -> Wrapper<Foo> {
    Wrapper::First(22)
}

then it works.

@oli-obk
Copy link
Contributor

oli-obk commented Sep 9, 2022

The diagnostic for

#![feature(type_alias_impl_trait)]

type Foo = impl Copy;

enum Wrapper<T> {
    First(T),
    Second
}

fn produce() -> Wrapper<Foo> {
    Wrapper::Second
}

Playground

is now

10 | fn produce() -> Wrapper<Foo> {
   |                 ------------ type must be known at this point
11 |     Wrapper::Second
   |     ^^^^^^^^^^^^^^^ cannot infer type of the type parameter `T` declared on the enum `Wrapper`
   |
   = note: cannot satisfy `_: Copy`
help: consider specifying the generic argument
   |
11 |     Wrapper::<T>::Second
   |            +++++

@oli-obk
Copy link
Contributor

oli-obk commented Sep 9, 2022

I don't believe there is anything left to do here. Please open a new issue if there are issues beyond the failure to infer the type

@oli-obk oli-obk closed this as completed Sep 9, 2022
Repository owner moved this from Todo to Done in type alias impl trait stabilization Sep 9, 2022
@oli-obk oli-obk moved this from Done to Wontfix in type alias impl trait stabilization Sep 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F-trait_alias `#![feature(trait_alias)]` F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` 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. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
Development

No branches or pull requests

6 participants