Refactor variance and remove last [pub] map
#41734
Conversation
There are now two queries: crate and item. The crate one computes the variance of all items in the crate; it is sort of an implementation detail, and not meant to be used. The item one reads from the crate one, synthesizing correct deps in lieu of the red-green algorithm. At the same time, remove the `variance_computed` flag, which was a horrible hack used to force invariance early on (e.g. when type-checking constants). This is only needed because of trait applications, and traits are always invariant anyway. Therefore, we now change to take advantage of the query system: - When asked to compute variances for a trait, just return a vector saying 'all invariant'. - Remove the corresponding "inferreds" from traits, and tweak the constraint generation code to understand that traits are always inferred.
make it work for traits etc uniformly
(Now that variances are not part of signature.)
alexcrichton
added
the
S-waiting-on-review
src/librustc/ty/mod.rs
Outdated
| /// variance of every item in the local crate. You should not use it | ||
| /// directly, because to do so will make your pass dependent on the | ||
| /// HIR of every item in the local crate. Instead, use | ||
| /// `tcx.item_variances()` to get the variance for a *particular* |
There was a problem hiding this comment.
This should probably say tcx.variances_of().
| f: Base<'a, 'a, 'b, 'c> | ||
| } | ||
|
|
||
| #[rustc_variance] // Combine + and o to yield o (just pay attention to 'a here) | ||
| struct Derived3<'a:'b, 'b, 'c> { //~ ERROR [o, -, *] | ||
| //~^ ERROR parameter `'c` is never used |
There was a problem hiding this comment.
Why do these error messages disappear?
There was a problem hiding this comment.
because I now halt compilation if we see any #[rustc_variance] annotations, rather than continuing on and reporting errors from other passes
| into two queries: | ||
|
|
||
| - `crate_variances` computes the variance for all items in the current crate. | ||
| - `item_variances` accesses the variance for an individual reading; it |
There was a problem hiding this comment.
Nit: this should be variances_of
|
I've looked through this PR, in particular regarding the dependency tracking related pieces, and it looks good to me. However, I know very little about the code in question here, so I'd feel better if someone else code also do a pass over the changes. @nikomatsakis suggested that @pnkfelix might be a good candidate for that. r? @pnkfelix |
|
Looks fine to me. @bors r+ |
|
📌 Commit 79de56a has been approved by |
|
@bors r- @nikomatsakis I halted bors to give you a chance to address the |
|
@pnkfelix ❤️ |
|
@bors r=pnkfelix |
|
📌 Commit 3da5daf has been approved by |
…iance, r=pnkfelix
Refactor variance and remove last `[pub]` map
This PR refactors variance to work in a more red-green friendly way. Because red-green doesn't exist yet, it has to be a bit hacky. The basic idea is this:
- We compute a big map with the variance for all items in the crate; when you request variances for a particular item, we read it from the crate
- We now hard-code that traits are invariant (which they are, for deep reasons, not gonna' change)
- When building constraints, we compute the transitive closure of all things within the crate that depend on what using `TransitiveRelation`
- this lets us gin up the correct dependencies when requesting variance of a single item
Ah damn, just remembered, one TODO:
- [x] Update the variance README -- ah, I guess the README updates I did are sufficient
r? @michaelwoerister
|
⌛ Testing commit 3da5daf with merge 42a4f37... |
This PR refactors variance to work in a more red-green friendly way. Because red-green doesn't exist yet, it has to be a bit hacky. The basic idea is this:
TransitiveRelationAh damn, just remembered, one TODO:
r? @michaelwoerister