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.

Sign up for GitHub

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

Jump to bottom

[WIP] rustc: check that type alias where clauses are well-formed. #54352

Closed
wants to merge 1 commit into from
Closed

[WIP] rustc: check that type alias where clauses are well-formed. #54352

wants to merge 1 commit into from

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented Sep 19, 2018 •
edited
Loading

Unlike #54033 (comment) / #49441 (comment), I suspect that "where clauses well-formedness" holds much more commonly than "enough bounds for the aliased type" (as the latter is by omission).
What that means is that the bounds themselves, and type parameter defaults, are checked.

TODO: provide proper examples and tests.

I'm primarily opening this PR to do a crater run, hoping for 0 regressions. If the number of regressions is non-0 but small, I'll include it in #54090, as that PR is likely wrong for non-WF bounds.

r? @nikomatsakis cc @Centril

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 19, 2018
@eddyb
Copy link
Member Author

eddyb commented Sep 19, 2018

@bors try

@bors
Copy link
Contributor

bors commented Sep 19, 2018

⌛ Trying commit 62f3343 with merge 135b4cd023a753409d01e43b0f23d22a899feeeb...

@bors bors mentioned this pull request Sep 19, 2018
Merged
@bors
Copy link
Contributor

bors commented Sep 19, 2018

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

@eddyb
Copy link
Member Author

eddyb commented Sep 19, 2018

@craterbot run start=master#1e21c9a297a9fe668d62887a3a6a4add8e717b17 end=try#135b4cd023a753409d01e43b0f23d22a899feeeb mode=check-only

@Mark-Simulacrum
Copy link
Member

@craterbot run start=master#1e21c9a297a9fe668d62887a3a6a4add8e717b17 end=try#135b4cd023a753409d01e43b0f23d22a899feeeb mode=check-only

@craterbot
Copy link
Collaborator

👌 Experiment pr-54352 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 19, 2018
@craterbot
Copy link
Collaborator

🚧 Experiment pr-54352 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-54352 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 20, 2018
@eddyb
Copy link
Member Author

eddyb commented Sep 20, 2018

Only two regressions were found:

Nominating for discussion, since I'm worried we might not want to have "implied bounds" for type aliases include bounds derived from WF-ness of written bounds/defaults.

@eddyb eddyb added I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 20, 2018
@pnkfelix
Copy link
Member

discussed at T-compiler meeting

Here is how I summarized the conclusion: We can do future-compat warning now, and potentially change edition-2015 behavior in a future release. And we make it hard error for edtion-2018, as planned

@TimNN TimNN added A-allocators Area: Custom and system allocators and removed A-allocators Area: Custom and system allocators labels Sep 25, 2018
@TimNN
Copy link
Contributor

TimNN commented Oct 16, 2018

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

@TimNN TimNN 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 Oct 16, 2018
@Centril Centril added P-high High priority I-nominated labels Oct 19, 2018
@Centril Centril mentioned this pull request Oct 20, 2018
Closed
@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
Reviewers
No reviews
Assignees

@nikomatsakis nikomatsakis

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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
10 participants
@eddyb @bors @Mark-Simulacrum @craterbot @pnkfelix @TimNN @nikomatsakis @Centril @apiraino @rust-highfive