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

typeck::check::check_item's ItemFn handling should subst more #13140

Closed
pnkfelix opened this issue Mar 25, 2014 · 1 comment · Fixed by #13157
Closed

typeck::check::check_item's ItemFn handling should subst more #13140

pnkfelix opened this issue Mar 25, 2014 · 1 comment · Fixed by #13157

Comments

@pnkfelix
Copy link
Member

Spawned off of issue #5121.

The concrete problem that we are encountering is that the type fn_tpt.ty extracted by check_item still has formal generic parameters in it (e.g. so-called EarlyBound lifetimes) that should have been replaced with concrete types (such as unification variables, so-called Free lifetimes).

The solution in several of these instances is that typeck::check::check_itemhandling of ItemFn should subst more, in the same manner as done in check_method_body.

That, or the substitution should be moved into the common control paths that are shared between check_item and check_method_body, e.g. check_bare_fn or perhaps check_fn.

(See also #13139 for the bug to try to safe-guard against future occurrences of this bug.)

Aside: Normally I would have just repurposed #5121 for this task, but local experimentation has shown that fixing this problem will not solve all of the ICE's nor all of the instances of unsoundness that are linked on #5121. So I am trying to track which issues are resolved and which remain unresolved by filing a separate issue and then checking off checkboxes accordingly.

@nikomatsakis
Copy link
Contributor

cc me

bors added a commit that referenced this issue Mar 29, 2014
r? @nikomatsakis

Fix #13140

Includes two fixes, and a semi-thorough regression test.

(There is another set of tests that I linked from #5121, but those are sort of all over the place, while the ones I've included here are more directly focused on the issues at hand.)
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.

2 participants