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

bug with polymorphisation + closures that inherit unused parameters #74636

Closed
davidtwco opened this issue Jul 22, 2020 · 0 comments · Fixed by #74717
Closed

bug with polymorphisation + closures that inherit unused parameters #74636

davidtwco opened this issue Jul 22, 2020 · 0 comments · Fixed by #74717

Comments

@davidtwco
Copy link
Member

#74614 discovered a regression in polymorphisation, fixed by disabling polymorphisation in #74633. This issue will track the fix for the regression, based on the discussion in Zulip (with thanks to @lcnr, @oli-obk and @eddyb).

#74614 can be reduced to the following MCVE (src/test/ui/issues/issue-74614.rs after #74633):

fn test<T>() { std::mem::size_of::<T>(); }

pub fn foo<T>(_: T) -> &'static fn() {
    &(test::<T> as fn())
}

fn outer<T>() {
    foo(|| ());
}


fn main() {
    outer::<u8>();
}

outer is polymorphized because T is not used in outer or outer::{{closure}}#0. outer::{{closure}}#0 therefore inherits T from outer and that ends up being T in foo<T>.

Without a transitive analysis (which isn't possible without hitting cycle errors), outer cannot consider T used based on how outer::{{closure}}#0 is used in other functions.

There are two issues that this can cause - when that closure is used in a cast or when that closure is used in reflection.

When used in a cast (as in the example above), the following lines will be hit:

// All reifications must be monomorphic, bail out otherwise.
if src.layout.ty.needs_subst() {
throw_inval!(TooGeneric);
}

// All reifications must be monomorphic, bail out otherwise.
if src.layout.ty.needs_subst() {
throw_inval!(TooGeneric);
}

Using still_further_specializable instead of needs_subst here fixes the issue, as that flag isn't set for the parent substitutions of closures.

When used in reflection (see the example below), then this will ICE once #74538 lands, but doesn't right now.

use std::any::TypeId;

pub fn foo<T: 'static>(_: T) -> TypeId {
    TypeId::of::<T>()
}

fn outer<T: 'static>() {
    foo(|| ());
}

fn main() {
    outer::<u8>();
}

Reflection is more complicated because it makes the result of polymorphisation observable.

bors added a commit to rust-lang-ci/rust that referenced this issue Jul 22, 2020
…phisation, r=wesleywiser

Disable polymorphisation

Fixes rust-lang#74614.

This PR disables polymorphisation to fix the regression in rust-lang#74614 after investigation into the issue makes it clear that the fix won't be trivial. ~~I'll file an issue shortly to replace rust-lang#74614 with the findings so far.~~ rust-lang#74636 has been filed to track the fix of the underlying regression.

r? @eddyb
@bors bors closed this as completed in 22e6099 Aug 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant