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

[4/n] rustc: harden against InferOk having obligations in more cases. #37404

Merged
merged 1 commit into from
Nov 6, 2016

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented Oct 25, 2016

This is part of a series (prev | next) of patches designed to rework rustc into an out-of-order on-demand pipeline model for both better feature support (e.g. MIR-based early constant evaluation) and incremental execution of compiler passes (e.g. type-checking), with beneficial consequences to IDE support as well.
If any motivation is unclear, please ask for additional PR description clarifications or code comments.


This adds more asserts that InferOk results have no obligations, pending completion of #32730.

Each of these could accidentally drop obligations on the floor if they start getting produced by unification, and a future change does just that, in order to produce a "shallow success" (hopefully leading to ambiguities during trait selection), without the possibility of an eventual success - mostly guarded by ICEs for now.

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

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

Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors r+

match selcx.infcx().eq_impl_headers(true, TypeOrigin::Misc(DUMMY_SP), &a_impl_header,
&b_impl_header) {
Ok(InferOk { obligations, .. }) => {
// FIXME(#32730) propagate obligations
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just fyi, I've got a branch that aims to fix #32730 -- I haven't poked at it in a while, I was just retrying to rebase it today. I'll probably try to land some of the initial refactorings first in any case. (Have to see if this interferes with some of what you have going on later on...)

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Oct 31, 2016

📌 Commit 424f41d has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Nov 2, 2016

☔ The latest upstream changes (presumably #36131) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Contributor

bors commented Nov 3, 2016

🔒 Merge conflict

sophiajt pushed a commit to sophiajt/rust that referenced this pull request Nov 4, 2016
[3/n] rustc: unify and simplify managing associated items.

_This is part of a series ([prev](rust-lang#37401) | [next](rust-lang#37404)) of patches designed to rework rustc into an out-of-order on-demand pipeline model for both better feature support (e.g. [MIR-based](https://github.com/solson/miri) early constant evaluation) and incremental execution of compiler passes (e.g. type-checking), with beneficial consequences to IDE support as well.
If any motivation is unclear, please ask for additional PR description clarifications or code comments._

<hr>

`ImplOrTraitItem`/`impl_or_trait_item` have been renamed to `AssociatedItem`/`associated_item`.

The common fields from (what used to be) `ty::ImplOrTraitItem`'s variants have been pulled out, leaving only an `AssociatedKind` C-like enum to distinguish between methods, constants and types.

The type information has been removed from `AssociatedItem`, and as such the latter can now be computed on-demand from the local HIR map, i.e. an extern-crate-enabled `TraitItem | ImplItem`.
It may be moved to HIR in the future, if we intend to start using HIR types cross-crate.

`ty::ExplicitSelfCategory` has been moved to `rustc_typeck` and is produced on-demand from the signature of the method, and a `method_has_self_argument` field on `AssociatedItem`, which is used to indicate that the first argument is a sugary "method receiver" and as such, method call syntax can be used.
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Nov 4, 2016
[5/n] rustc: record the target type of every adjustment.

_This is part of a series ([prev](rust-lang#37404) | [next](rust-lang#37412)) of patches designed to rework rustc into an out-of-order on-demand pipeline model for both better feature support (e.g. [MIR-based](https://github.com/solson/miri) early constant evaluation) and incremental execution of compiler passes (e.g. type-checking), with beneficial consequences to IDE support as well.
If any motivation is unclear, please ask for additional PR description clarifications or code comments._

<hr>

The first commit rearranges `tcx.tables` so that all users go through `tcx.tables()`. This in preparation for per-body `Tables` where they will be requested for a specific `DefId`. Included to minimize churn.

The rest of the changes focus on adjustments, there are some renamings, but the main addition is the target type, always available in all cases (as opposed to just for unsizing where it was previously needed).

Possibly the most significant effect of this change is that figuring out the final type of an expression is now _always_ just one successful `HashMap` lookup (either the adjustment or, if that doesn't exist, the node type).
@eddyb
Copy link
Member Author

eddyb commented Nov 6, 2016

@bors r=nikomatsakis

@bors
Copy link
Contributor

bors commented Nov 6, 2016

📌 Commit aee1ee3 has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Nov 6, 2016

⌛ Testing commit aee1ee3 with merge 5fe733a...

bors added a commit that referenced this pull request Nov 6, 2016
[4/n] rustc: harden against InferOk having obligations in more cases.

_This is part of a series ([prev](#37402) | [next](#37408)) of patches designed to rework rustc into an out-of-order on-demand pipeline model for both better feature support (e.g. [MIR-based](https://github.com/solson/miri) early constant evaluation) and incremental execution of compiler passes (e.g. type-checking), with beneficial consequences to IDE support as well.
If any motivation is unclear, please ask for additional PR description clarifications or code comments._

<hr>

This adds more asserts that `InferOk` results have no obligations, pending completion of #32730.

Each of these could accidentally drop obligations on the floor if they start getting produced by unification, and a future change does just that, in order to produce a "shallow success" (hopefully leading to ambiguities during trait selection), _without_ the possibility of an eventual success - mostly guarded by ICEs for now.
@bors bors merged commit aee1ee3 into rust-lang:master Nov 6, 2016
@eddyb eddyb deleted the lazy-4 branch November 9, 2016 16:51
eddyb added a commit to eddyb/rust that referenced this pull request Nov 10, 2016
[3/n] rustc: unify and simplify managing associated items.

_This is part of a series ([prev](rust-lang#37401) | [next](rust-lang#37404)) of patches designed to rework rustc into an out-of-order on-demand pipeline model for both better feature support (e.g. [MIR-based](https://github.com/solson/miri) early constant evaluation) and incremental execution of compiler passes (e.g. type-checking), with beneficial consequences to IDE support as well.
If any motivation is unclear, please ask for additional PR description clarifications or code comments._

<hr>

`ImplOrTraitItem`/`impl_or_trait_item` have been renamed to `AssociatedItem`/`associated_item`.

The common fields from (what used to be) `ty::ImplOrTraitItem`'s variants have been pulled out, leaving only an `AssociatedKind` C-like enum to distinguish between methods, constants and types.

The type information has been removed from `AssociatedItem`, and as such the latter can now be computed on-demand from the local HIR map, i.e. an extern-crate-enabled `TraitItem | ImplItem`.
It may be moved to HIR in the future, if we intend to start using HIR types cross-crate.

`ty::ExplicitSelfCategory` has been moved to `rustc_typeck` and is produced on-demand from the signature of the method, and a `method_has_self_argument` field on `AssociatedItem`, which is used to indicate that the first argument is a sugary "method receiver" and as such, method call syntax can be used.
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.

4 participants