Skip to content

dropck: do not add types with escaped-regions into the breadcrumbs set. #27236

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

Conversation

pnkfelix
Copy link
Member

dropck: do not add types with escaped-regions into the breadcrumbs set.

In particular, when we traverse a type like:

(for <'r> Fn(&'r u8), for <'r> Fn(&'r u8))

we should not add the first type Fn(&'r u8) to the bread-crumbs set,
because that type ends up being structurally equivalent to the
second Fn(&'r u8) type, even though they each should be considered
distinct since they occur under distinct for <'r> binders.

Fix #25750.

In particular, when we traverse a type like:

  `(for <'r> Fn(&'r u8), for <'r> Fn(&'r u8))`

we should not add the first type `Fn(&'r u8)` to the bread-crumbs set,
because that type ends up being structurally equivalent to the
*second* `Fn(&'r u8)` type, even though they each should be considered
distinct since they occur under distinct `for <'r>` binders.

Fix rust-lang#25750.
@rust-highfive
Copy link
Contributor

r? @Aatch

(rust_highfive has picked a reviewer for you, use r? to override)

@pnkfelix
Copy link
Member Author

r? @nikomatsakis

@arielb1
Copy link
Contributor

arielb1 commented Jul 23, 2015

What's the issue here again (test?)? I think adding a skip_current_subtree before the continue; when we meet a breadcrumb would be a wiser choice (actually, it is required, see #27240) - if there is an issue with that, it would be nice to at least comment on it.

Anyway, I think ty_walk is a stupid choice here, given that we don't actually treat anything nominally here (except trivially for primitives, and semi-accidentally for tuples).

@pnkfelix
Copy link
Member Author

@arielb1 you're right I should have included a regression test.

@pnkfelix
Copy link
Member Author

@arielb1 also I agree that it would probably be good to add a skip_current_subtree before the continue; ... its possible that doing so would resolve #25750 as well, ... I will think about it.

@pnkfelix
Copy link
Member Author

(closing, i suspect @arielb1 's approach of #27261 is going to be a more realistic PR to land.)

@pnkfelix pnkfelix closed this Jul 28, 2015
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 this pull request may close these issues.

5 participants