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

[WIP] rustc_typeck: make "global" WF requirements hard errors, in type aliases. #54394

Closed
wants to merge 4 commits into from

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented Sep 20, 2018

Based on #54033, a theory I want to test on crater.
Unlike #54033, this only errors on "unusable" aliases, as in, no choice of type parameters can make them WF, because they include nested non-WF types/constants.

r? @nikomatsakis

@eddyb
Copy link
Member Author

eddyb commented Sep 20, 2018

@bors try

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 20, 2018
@bors
Copy link
Contributor

bors commented Sep 20, 2018

⌛ Trying commit 1713f89 with merge f1a3f05968ed2a674e09b6efa4ad9df90816e2b6...

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
[01:12:37] ....................................................................................................
[01:12:40] .......................................................i............................................
[01:12:43] ....................................................................................................
[01:12:46] ....................................................................................................
[01:12:48] ...iiiiiiiii........................................................................................
[01:12:55] ....................................................................................................
[01:12:58] ......................................................................................i.............
[01:13:01] ....................................................................................................
[01:13:04] .........................................i.i..ii....................................................
---
travis_time:start:test_rustdoc
Check compiletest suite=rustdoc mode=rustdoc (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[01:25:29] 
[01:25:29] running 260 tests
[01:25:56] ...........F...........i............................................................................
[01:26:29] ............................................................
[01:26:29] failures:
[01:26:29] 
[01:26:29] ---- [rustdoc] rustdoc/const-evalutation-ice.rs stdout ----
[01:26:29] ---- [rustdoc] rustdoc/const-evalutation-ice.rs stdout ----
[01:26:29] 
[01:26:29] error: rustdoc failed!
[01:26:29] thread 'main' panicked at 'Some tests failed', tools/compiletest/src/main.rs:496:22
[01:26:29] status: exit code: 1
[01:26:29] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustdoc" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc/const-evalutation-ice/auxiliary" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc/const-evalutation-ice" "/checkout/src/test/rustdoc/const-evalutation-ice.rs"
[01:26:29] ------------------------------------------
[01:26:29] 
[01:26:29] ------------------------------------------
[01:26:29] stderr:

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@bors
Copy link
Contributor

bors commented Sep 20, 2018

☀️ Test successful - status-travis
State: approved= try=True

@eddyb
Copy link
Member Author

eddyb commented Sep 20, 2018

@craterbot run start=master#f7f4c500b46603386e940f116b469c7adc043a6d end=try#f1a3f05968ed2a674e09b6efa4ad9df90816e2b6 mode=check-only

@craterbot
Copy link
Collaborator

👌 Experiment pr-54394 created and queued.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 20, 2018
@craterbot
Copy link
Collaborator

🚧 Experiment pr-54394 is now running on agent aws-2.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot
Copy link
Collaborator

🎉 Experiment pr-54394 is completed!
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Sep 21, 2018
@eddyb
Copy link
Member Author

eddyb commented Sep 21, 2018

@nikomatsakis Can you take a look at these results? I'm confused because I expected to not see any errors - maybe the crater run should've been done with cap-lints=warn so #![deny(warnings)] doesn't make the warning an error, but does that explain everything?

EDIT: I looked at a random sample, and only a few were {deny,forbid}(warnings), and the remaining ones were either caused by nalgebra or diesel.
EDIT2: okay, it looks like both nalgebra and diesel only have errors with help: messages indicating they should be lints, but I hadn't changed the respective error logic to support lints.
EDIT3: @lqd helped with combing through the logs, and found a few intended regressions (most of them depend on domain, but there's overall not a lot, so we can do future-compat warnings here).

@XAMPPRocky
Copy link
Member

Triage; @nikomatsakis Hello, are you able to review this PR?

@eddyb
Copy link
Member Author

eddyb commented Sep 26, 2018

@Aaronepower This is not waiting on a review, I still have to finish the PR.

@eddyb eddyb added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 26, 2018
@nikomatsakis
Copy link
Contributor

@eddyb

Can you take a look at these results? I'm confused because I expected to not see any errors

Just saw this, sorry...

@TimNN
Copy link
Contributor

TimNN commented Oct 16, 2018

Ping from triage @eddyb: what is the status of this PR?

@Centril Centril added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. P-high High priority I-nominated labels Oct 19, 2018
@nikomatsakis
Copy link
Contributor

I'm going to close this PR for now. I do plan to start opening some PRs and issuing warnings, per the plan discussed in #55222

@apiraino apiraino removed I-nominated P-high High priority labels Feb 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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.

9 participants