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

Make use of the implemented red/green algorithm for variance #47696

Merged
merged 1 commit into from
Jan 26, 2018

Conversation

Zoxc
Copy link
Contributor

@Zoxc Zoxc commented Jan 24, 2018

@kennytm kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 24, 2018
@kennytm
Copy link
Member

kennytm commented Jan 24, 2018

The dep-graph-variance-alias test failed.

[00:59:15] failures:
[00:59:15] 
[00:59:15] ---- [compile-fail] compile-fail/dep-graph-variance-alias.rs stdout ----
[00:59:15] 	
[00:59:15] error: /checkout/src/test/compile-fail/dep-graph-variance-alias.rs:29: unexpected error: '29:1: 29:45: no path from `TypeAlias` to `ItemVariances`'
[00:59:15] 
[00:59:15] error: /checkout/src/test/compile-fail/dep-graph-variance-alias.rs:29: expected error not found: OK
[00:59:15] 
[00:59:15] error: 1 unexpected errors found, 1 expected errors not found
[00:59:15] status: exit code: 101
[00:59:15] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/compile-fail/dep-graph-variance-alias.rs" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/compile-fail" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/compile-fail/dep-graph-variance-alias.stage2-x86_64-unknown-linux-gnu" "-Crpath" "-O" "-Zmiri" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-Z" "query-dep-graph" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/compile-fail/dep-graph-variance-alias.stage2-x86_64-unknown-linux-gnu.aux" "-A" "unused"
[00:59:15] unexpected errors (from JSON output): [
[00:59:15]     Error {
[00:59:15]         line_num: 29,
[00:59:15]         kind: Some(
[00:59:15]             Error
[00:59:15]         ),
[00:59:15]         msg: "29:1: 29:45: no path from `TypeAlias` to `ItemVariances`"
[00:59:15]     }
[00:59:15] ]
[00:59:15] 
[00:59:15] not found errors (from test file): [
[00:59:15]     Error {
[00:59:15]         line_num: 29,
[00:59:15]         kind: Some(
[00:59:15]             Error
[00:59:15]         ),
[00:59:15]         msg: "OK"
[00:59:15]     }
[00:59:15] ]
[00:59:15] 
[00:59:15] thread '[compile-fail] compile-fail/dep-graph-variance-alias.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:1252:13
[00:59:15] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[00:59:15] 
[00:59:15] 
[00:59:15] failures:
[00:59:15]     [compile-fail] compile-fail/dep-graph-variance-alias.rs
[00:59:15] 
[00:59:15] test result: FAILED. 2523 passed; 1 failed; 15 ignored; 0 measured; 0 filtered out

@michaelwoerister
Copy link
Member

r? @nikomatsakis
(I'm out sick)

@@ -23,7 +23,7 @@ struct Foo<T> {
f: T
}

#[rustc_if_this_changed]
#[rustc_if_this_changed(Krate)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Huh, this looks suspicious.

Copy link
Contributor

Choose a reason for hiding this comment

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

This change doesn't seem right to me. I think we do expect to have to recompute item-variances for Use if the type alias changes. I would sort of expect that it would be an input into the AdtDef or what have you for Use... but I guess it might not be working out that way? Did you dig into what's going on here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The CrateVariances dep node is eval_always, so it only depends on Krate and not on any Hir nodes. This in turn means that ItemVariances won't transitively depend on any Hir nodes, just Krate. So there won't be a path from Hir(TypeAlias) to ItemVariances. This is still fine though since CrateVariances always gets recomputed. It is also correct without that because Krate has an implicit dependency on everything in a crate.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, makes sense. Thanks!

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jan 25, 2018

📌 Commit 62afc43 has been approved by nikomatsakis

@nikomatsakis nikomatsakis added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 25, 2018
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Jan 25, 2018
Make use of the implemented red/green algorithm for variance

r? @michaelwoerister
bors added a commit that referenced this pull request Jan 26, 2018
@bors bors merged commit 62afc43 into rust-lang:master Jan 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants