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

Tracking issue for closure types that reference themselves #46062

Closed
nikomatsakis opened this issue Nov 17, 2017 · 4 comments
Closed

Tracking issue for closure types that reference themselves #46062

nikomatsakis opened this issue Nov 17, 2017 · 4 comments
Labels
C-future-compatibility Category: Future-compatibility lints 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

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Nov 17, 2017

This issue describes a bug fix regarding "cyclic closures". Specifically, we fixed a bug in rustc in which a closure type T was capable of directly invoking itself without going through a virtual call. This is not intended to be allowed: in order to keep closure inference tractable, closure types are intended to form a strict hierarchy with respect to one another, in terms of which other closures they can invoke.

What kind of code is affected

In fact, it is rather difficult to have a closure that directly invokes itself without going through a Fn object or a fn pointer type. It was however possible if you used an "fixed-point combinator" like this:

#![feature(conservative_impl_trait)]

fn fix<F>(f: F) -> impl Fn()
where F: Fn(&F)
{
  move || f(&f)
}

fn main() {
    let x = fix(|x| {
      // in here, the closure has a shared reference to itself, `x`
    });
    x();
}

This hole has since been closed and the code above no longer compiles.

How can you fix it?

The easiest fix is to rewrite your closure into a top-level function, or into a method. In some cases, you may also be able to have your closure call itself by capturing a &Fn() object or fn() pointer that refers to itself. That is permitting, since the closure would be invoking itself via a virtual call, and hence does not directly reference its own type.

Why was there no warning period?

This bug was fixed was without a warning period, even though there is the potential that stable code was affected. This is not our normal practice. We chose in this case to take that action because:

(a) it was quite difficult to arrange a comprehensive warning
(b) crater run indicated no affected crates
(c) this is a bug fix
(d) we are able to direct users to this error message in the event of an error occurring

If however you were negatively affected, we very much apologize. We would like to hear about that. If for some reason you cannot update your code readily, we would consider backing out this change and re-applying it later after you've had some time to adapt, presuming the issue is found promptly enough for that to be practical.

@nikomatsakis nikomatsakis added C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. 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 17, 2017
@nikomatsakis nikomatsakis changed the title Tracking issue for cyclic closures and the fixed-point combinator Tracking issue for closure types that take themselves as argument Nov 17, 2017
@nikomatsakis
Copy link
Contributor Author

cc @arielb1 @rust-lang/lang

@nikomatsakis nikomatsakis changed the title Tracking issue for closure types that take themselves as argument Tracking issue for closure types that reference themselves Nov 18, 2017
@thepowersgang
Copy link
Contributor

thepowersgang commented Nov 25, 2017

A far smaller reproduction, provided by Arnavion (which doesn't use impl Trait) while determining why this broke some of my OS code (which had button callbacks getting references to the button itself which is generic over the callback type)

struct B<F: Fn(B<F>)>(F);

fn main() {
    let _b = B(|_b| ());
}

@Centril
Copy link
Contributor

Centril commented Nov 11, 2018

@nikomatsakis there has been no movement here for a year now so I think we can close it?

@Centril Centril added the C-future-compatibility Category: Future-compatibility lints label Dec 3, 2018
@Centril Centril removed the C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. label Jan 13, 2019
@nikomatsakis
Copy link
Contributor Author

@Centril agreed. I'm going to close it. I would not expect to revert this change without some significant thought, as it vastly simplifies the "closure upvar inference" that we do (i.e., figuring out whether a given upvar is borrowed "by ref" or "by mut ref" etc).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-future-compatibility Category: Future-compatibility lints 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
None yet
Development

No branches or pull requests

3 participants