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

[8/n] rustc: clean up lookup_item_type and remove TypeScheme. #37688

Merged
merged 1 commit into from
Nov 12, 2016

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented Nov 10, 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.


#35605 introduced an uniform lookup_generics but didn't clean up everything. This PR changes:

  • tcx.tcache -> tcx.item_types
  • TypeScheme (grouping Ty and ty::Generics) is removed
  • tcx.item_types entries no longer duplicated in tcx.tables.node_types
  • tcx.lookup_item_type(def_id).ty -> tcx.item_type(def_id)
  • tcx.lookup_item_type(def_id).generics -> tcx.item_generics(def_id)
  • tcx.lookup_generics(def_id) -> tcx.item_generics(def_id)
  • tcx.lookup_{super_,}predicates(def_id) -> tcx.item_{super_,}predicates(def_id)

@rust-highfive
Copy link
Collaborator

r? @pnkfelix

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

@@ -556,7 +556,7 @@ pub struct DebruijnIndex {
///
/// These are regions that are stored behind a binder and must be substituted
/// with some concrete region before being used. There are 2 kind of
/// bound regions: early-bound, which are bound in a TypeScheme/TraitDef,
/// bound regions: early-bound, which are bound in and item's Generics,
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: s/and/an/

@@ -546,16 +552,11 @@ fn convert_field<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>,
{
let tt = ccx.icx(struct_predicates).to_ty(&ExplicitRscope, &field.ty);
ty_f.fulfill_ty(tt);
write_ty_to_tcx(ccx, field.id, tt);
Copy link
Contributor

Choose a reason for hiding this comment

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

tables.node_types is never used for items now, that's why all write_ty_to_tcxs were removed from collect?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct, it was all a bit of a mess before, but that's all incompatible with per-body tables. Should add this change to the description.

@petrochenkov
Copy link
Contributor

petrochenkov commented Nov 10, 2016

I've reviewed this and will r+ if nobody minds (modulo the question).

@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Nov 10, 2016

📌 Commit 3f9eba1 has been approved by petrochenkov

@alexcrichton alexcrichton added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Nov 10, 2016
eddyb added a commit to eddyb/rust that referenced this pull request Nov 11, 2016
[8/n] rustc: clean up lookup_item_type and remove TypeScheme.

_This is part of a series ([prev](rust-lang#37676) | [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](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>

* `tcx.tcache` -> `tcx.item_types`
* `TypeScheme` (grouping `Ty` and `ty::Generics`) is removed
* `tcx.item_types` entries no longer duplicated in `tcx.tables.node_types`
* `tcx.lookup_item_type(def_id).ty` -> `tcx.item_type(def_id)`
* `tcx.lookup_item_type(def_id).generics` -> `tcx.item_generics(def_id)`
* `tcx.lookup_generics(def_id)` -> `tcx.item_generics(def_id)`
* `tcx.lookup_{super_,}predicates(def_id)` -> `tcx.item_{super_,}predicates(def_id)`
eddyb added a commit to eddyb/rust that referenced this pull request Nov 12, 2016
[8/n] rustc: clean up lookup_item_type and remove TypeScheme.

_This is part of a series ([prev](rust-lang#37676) | [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](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>

* `tcx.tcache` -> `tcx.item_types`
* `TypeScheme` (grouping `Ty` and `ty::Generics`) is removed
* `tcx.item_types` entries no longer duplicated in `tcx.tables.node_types`
* `tcx.lookup_item_type(def_id).ty` -> `tcx.item_type(def_id)`
* `tcx.lookup_item_type(def_id).generics` -> `tcx.item_generics(def_id)`
* `tcx.lookup_generics(def_id)` -> `tcx.item_generics(def_id)`
* `tcx.lookup_{super_,}predicates(def_id)` -> `tcx.item_{super_,}predicates(def_id)`
bors added a commit that referenced this pull request Nov 12, 2016
@bors bors merged commit 3f9eba1 into rust-lang:master Nov 12, 2016
@eddyb eddyb deleted the lazy-8 branch November 12, 2016 12:17
bors added a commit that referenced this pull request Nov 28, 2016
[7/n] rustc: desugar UFCS in HIR and don't use DefMap for associated resolutions.

_This is part of a series ([prev](#37412) | [next](#37688)) 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>

Previously, a path like `T::Assoc::method`, while equivalent to `<<T>::Assoc>::method`, wasn't desugared in any way at the HIR level and everything inspecting it had to either deal with knowing only `T` (before typeck) or knowing only the definition of `method` (after typeck).
Such a path also had only one `NodeId` and associated resolution during typeck modified `DefMap`, in a way that would be hard for incremental recompilation to track, and inconvenient for partial type conversions from HIR to `Ty`, which are required to break faux-cycles in on-demand type collection.

The desugarings performed by this PR are as follows:
* `use a::{b,c};` is flattened to `use a as _; use a::b; use a::c;`
  * as resolution is complete, `use a as _;` doesn't do anything, except get checked for stability
* `Vec::new` (an expression) becomes `Vec<..>::new<..>`, to distinguish it from `<Vec>::new<..>`
  * the "infer all parameters" `<..>` form is internal and not even pretty-printed
  * used when there are no type parameters at all, in an expression or pattern path segment
* `T::A::B` becomes `<<T>::A>::B` in a type, and `<<T<..>>::A<..>>::B<..>` in an expression/pattern
  * one additional `hir::Ty` node is created for each prefix, starting with the fully-resolved type (`T`) and extending it with each segment (e.g. `<T>::A`)
* fully-resolved paths contain their `Def` in HIR, getting rid of the `DefMap` and absolving incremental recompilation of needing to manually look up nodes to handle that side information

Not keeping the `DefMap` around meant that associated resolutions had to be stored somewhere else:
* expressions and patterns use a new `NodeId -> Def` map in `ty::Tables`
  * compatible with the future per-body (constant / `fn` / closure) `Tables`
* types are accessible via `Ty` and the usual per-item generics / predicates / type
  * `rustdoc` and `save-analysis` are the only situations which insist on mapping syntactical types to semantical ones, or at least understand the resolution of associated types, therefore the type conversion cache, i.e. a `NodeId -> Ty` map, is exposed by typeck for this purpose
  * stability had to be split into a pass that runs on HIR and checks the results of name resolution, and impromptu checks triggered by `typeck` for associated paths, methods, fields, etc.
  * privacy using semantic types results in accurate reachability for `impl Trait`, which fixes #35870, and thorough introspection of associated types, which may allow relaxing private-in-public checking on bounds, while keeping the intended ban on projections with private type parameters

cc @petrochenkov
bors added a commit that referenced this pull request Dec 2, 2016
[9/n] rustc: move type information out of AdtDef and TraitDef.

_This is part of a series ([prev](#37688) | [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](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>

Both `AdtDef` and `TraitDef` contained type information (field types, generics and predicates) which was required to create them, preventing their use before that type information exists, or in the case of field types, *mutation* was required, leading to a variance-magicking implementation of `ivar`s.

This PR takes that information out and the resulting cleaner setup could even eventually end up merged with HIR, because, just like `AssociatedItem` before it, there's no dependency on types anymore.
(With one exception, variant discriminants should probably be moved into their own map later.)
critiqjo pushed a commit to critiqjo/rustdoc that referenced this pull request Dec 16, 2016
[9/n] rustc: move type information out of AdtDef and TraitDef.

_This is part of a series ([prev](rust-lang/rust#37688) | [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](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>

Both `AdtDef` and `TraitDef` contained type information (field types, generics and predicates) which was required to create them, preventing their use before that type information exists, or in the case of field types, *mutation* was required, leading to a variance-magicking implementation of `ivar`s.

This PR takes that information out and the resulting cleaner setup could even eventually end up merged with HIR, because, just like `AssociatedItem` before it, there's no dependency on types anymore.
(With one exception, variant discriminants should probably be moved into their own map later.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants