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

Guts of trait reform: Reimplement trait matching algorithm #17197

Merged
merged 10 commits into from
Sep 16, 2014

Conversation

nikomatsakis
Copy link
Contributor

This patch does not make many functional changes, but does a lot of restructuring towards the goals of #5527. This is the biggest patch, basically, that should enable most of the other patches in a relatively straightforward way.

Major changes:

  • Do not track impls through trans, instead recompute as needed.
  • Isolate trait matching code into its own module, carefully structure to distinguish various phases (selection vs confirmation vs fulfillment)
  • Consider where clauses in their more general form
  • Integrate checking of builtin bounds into the trait matching process, rather than doing it separately in kind.rs (important for opt-in builtin bounds)

What is not included:

  • Where clauses are still not generalized. This should be a straightforward follow-up patch.
  • Caching. I did not include much caching. I have plans for various kinds of caching we can do. Should be straightforward. Preliminary perf measurements suggested that this branch keeps compilation times roughly what they are.
  • Method resolution. The initial algorithm I proposed for Trait reform #5527 does not work as well as I hoped. I have a revised plan which is much more similar to what we do today.
  • Deref vs deref-mut. The initial fix I had worked great for autoderef, but not for explicit deref.
  • Permitting blanket impls to overlap with specific impls. Initial plan to consider all nested obligations before considering an impl to match caused many compilation errors. We have a revised plan but it is not implemented here, should be a relatively straightforward extension.

@nikomatsakis nikomatsakis force-pushed the issue-5527-trait-reform-revisited branch from 1df889c to 5b48f0a Compare September 12, 2014 16:20
@nikomatsakis
Copy link
Contributor Author

Argh. I just merged this this morning, but I already see merge conflicts! Probably best to do review now and I'll patch it up afterwards.

@nikomatsakis
Copy link
Contributor Author

Note: see doc.rs for an intro to the trait matching code.

@aturon
Copy link
Member

aturon commented Sep 12, 2014

Very excited to see this close to landing!

@nikomatsakis nikomatsakis force-pushed the issue-5527-trait-reform-revisited branch from 5b48f0a to 627ca58 Compare September 12, 2014 20:39
@nikomatsakis
Copy link
Contributor Author

rebased so that it is mergable

@nikomatsakis nikomatsakis force-pushed the issue-5527-trait-reform-revisited branch from 627ca58 to 13693c4 Compare September 12, 2014 21:47
@nikomatsakis
Copy link
Contributor Author

rebased: correct typos

@nikomatsakis
Copy link
Contributor Author

Address nits and refactor the trait resolution that is specific to trans into trans.

see the lang-items for Sized etc. @acrichto and @thestinger had no
objections.
- Unify the "well-formedness" checking that typeck was already doing with what
  was taking place in kind.
- Move requirements that things be sized into typeck.
- I left the checking on upvars in kind, though I think it should eventually be
  refactored into regionck (which would perhaps be renamed).

This reflects a general plan to convert typeck so that it registers
obligations or other pending things for conditions it cannot check
eventually. This makes it easier to identify all the conditions that
apply to an AST expression, but can also influence inference in somec
cases (e.g., `Send` implies `'static`, so I already had to promote a lot
of the checking that `kind.rs` was doing into typeck, this branch just
continues the process).
@nikomatsakis nikomatsakis force-pushed the issue-5527-trait-reform-revisited branch from 66f8172 to 48bc291 Compare September 15, 2014 19:28
@nikomatsakis
Copy link
Contributor Author

Rebased.

bors added a commit that referenced this pull request Sep 16, 2014
…sited, r=pcwalton

This patch does not make many functional changes, but does a lot of restructuring towards the goals of #5527. This is the biggest patch, basically, that should enable most of the other patches in a relatively straightforward way.

Major changes:

- Do not track impls through trans, instead recompute as needed.
- Isolate trait matching code into its own module, carefully structure to distinguish various phases (selection vs confirmation vs fulfillment)
- Consider where clauses in their more general form
- Integrate checking of builtin bounds into the  trait matching process, rather than doing it separately in kind.rs (important for opt-in builtin bounds)

What is not included:

- Where clauses are still not generalized. This should be a straightforward follow-up patch.
- Caching. I did not include much caching. I have plans for various kinds of caching we can do. Should be straightforward. Preliminary perf measurements suggested that this branch keeps compilation times roughly what they are.
- Method resolution. The initial algorithm I proposed for #5527 does not work as well as I hoped. I have a revised plan which is much more similar to what we do today.
- Deref vs deref-mut. The initial fix I had worked great for autoderef, but not for explicit deref. 
- Permitting blanket impls to overlap with specific impls. Initial plan to consider all nested obligations before considering an impl to match caused many compilation errors. We have a revised plan but it is not implemented here, should be a relatively straightforward extension.
bors added a commit that referenced this pull request Sep 16, 2014
…sited, r=pcwalton

This patch does not make many functional changes, but does a lot of restructuring towards the goals of #5527. This is the biggest patch, basically, that should enable most of the other patches in a relatively straightforward way.

Major changes:

- Do not track impls through trans, instead recompute as needed.
- Isolate trait matching code into its own module, carefully structure to distinguish various phases (selection vs confirmation vs fulfillment)
- Consider where clauses in their more general form
- Integrate checking of builtin bounds into the  trait matching process, rather than doing it separately in kind.rs (important for opt-in builtin bounds)

What is not included:

- Where clauses are still not generalized. This should be a straightforward follow-up patch.
- Caching. I did not include much caching. I have plans for various kinds of caching we can do. Should be straightforward. Preliminary perf measurements suggested that this branch keeps compilation times roughly what they are.
- Method resolution. The initial algorithm I proposed for #5527 does not work as well as I hoped. I have a revised plan which is much more similar to what we do today.
- Deref vs deref-mut. The initial fix I had worked great for autoderef, but not for explicit deref. 
- Permitting blanket impls to overlap with specific impls. Initial plan to consider all nested obligations before considering an impl to match caused many compilation errors. We have a revised plan but it is not implemented here, should be a relatively straightforward extension.
@bors bors closed this Sep 16, 2014
@bors bors merged commit eafeb33 into rust-lang:master Sep 16, 2014
@nikomatsakis nikomatsakis deleted the issue-5527-trait-reform-revisited branch March 30, 2016 16:15
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.

6 participants