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

Resolve long compile times when evaluating always valid constants #67667

Merged

Conversation

wesleywiser
Copy link
Member

This extends the existing logic which skips validating every integer or
floating point number type to also skip validating empty structs because
they are also trivially valid.

Fixes #67539

r? @oli-obk
cc @RalfJung @spastorino

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 27, 2019
@wesleywiser wesleywiser force-pushed the speed_up_trivially_valid_constants branch from 8a45951 to dce8826 Compare December 28, 2019 01:24
@wesleywiser
Copy link
Member Author

Force pushed

@wesleywiser wesleywiser force-pushed the speed_up_trivially_valid_constants branch from dce8826 to 19834f3 Compare December 28, 2019 01:34
src/librustc_mir/interpret/validity.rs Outdated Show resolved Hide resolved
src/librustc_mir/interpret/validity.rs Outdated Show resolved Hide resolved
@wesleywiser
Copy link
Member Author

Force pushed. We're back to the original version with an added //FIXME to document that we could eventually handle more cases with a more complete analysis.

const FOO: [Empty; 3] = [foo(); 3];

#[warn(const_err)]
const BAR: [Empty; 3] = [unsafe { std::mem::transmute(()) }; 3];
Copy link
Contributor

Choose a reason for hiding this comment

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

side note: if you remove this constant and its use, you do get a hard error on the use of FOO, but BAR is what we are actually interested in testing here, because it causes validation to error instead of const eval.

@oli-obk
Copy link
Contributor

oli-obk commented Dec 29, 2019

@bors r+

@bors
Copy link
Contributor

bors commented Dec 29, 2019

📌 Commit 31e1c41 has been approved by oli-obk

@bors bors 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 Dec 29, 2019
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 29, 2019
…d_constants, r=oli-obk

Resolve long compile times when evaluating always valid constants

This extends the existing logic which skips validating every integer or
floating point number type to also skip validating empty structs because
they are also trivially valid.

Fixes rust-lang#67539

r? @oli-obk
cc @RalfJung @spastorino
@JohnTitor
Copy link
Member

Seems failed in #67704 (comment)
@bors r-

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 29, 2019
This extends the existing logic which skips validating every integer or
floating point number type to also skip validating empty structs because
they are also trivially valid.

Fixes rust-lang#67539
@wesleywiser wesleywiser force-pushed the speed_up_trivially_valid_constants branch from 31e1c41 to 5ca3a1b Compare December 29, 2019 21:46
@wesleywiser
Copy link
Member Author

Ignored 32-bit platforms in the huge-values test I added.

@bors r=oli-obk

@bors
Copy link
Contributor

bors commented Dec 29, 2019

📌 Commit 5ca3a1b has been approved by oli-obk

@bors bors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 29, 2019
@bors
Copy link
Contributor

bors commented Dec 30, 2019

⌛ Testing commit 5ca3a1b with merge 0fb4380...

bors added a commit that referenced this pull request Dec 30, 2019
…, r=oli-obk

Resolve long compile times when evaluating always valid constants

This extends the existing logic which skips validating every integer or
floating point number type to also skip validating empty structs because
they are also trivially valid.

Fixes #67539

r? @oli-obk
cc @RalfJung @spastorino
@bors
Copy link
Contributor

bors commented Dec 30, 2019

☀️ Test successful - checks-azure
Approved by: oli-obk
Pushing 0fb4380 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 30, 2019
@bors bors merged commit 5ca3a1b into rust-lang:master Dec 30, 2019
@Mark-Simulacrum Mark-Simulacrum added the stable-nominated Nominated for backporting to the compiler in the stable channel. label Feb 6, 2020
@Mark-Simulacrum
Copy link
Member

Stable nominating per #68873.

@Mark-Simulacrum Mark-Simulacrum added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Feb 6, 2020
@pnkfelix
Copy link
Member

pnkfelix commented Feb 6, 2020

discussed at T-compiler meeting. stable-accepted.

@pnkfelix pnkfelix added the stable-accepted Accepted for backporting to the compiler in the stable channel. label Feb 6, 2020
@Mark-Simulacrum Mark-Simulacrum removed the stable-nominated Nominated for backporting to the compiler in the stable channel. label Feb 21, 2020
bors added a commit that referenced this pull request Feb 22, 2020
[stable] 1.41.1 release

This backports the following PRs:
 * Revert "Remove `checked_add` in `Layout::repeat`" #69241
 * Do not ICE when encountering `yield` inside `async` block #69175
 * Correct ICE caused by macros generating invalid spans. #68611
 * Changelog: Demonstrate final build-override syntax #68603
 * Resolve long compile times when evaluating always valid constants #67667
 * Fix MIR typeck soundness holes #69145

This also includes a commit which rustfmt's files which the latter commits touched (and perhaps a bit more) to make rebasing the PRs go more smoothly (thankfully, this should be the last time we need to do so).

I have removed stable-nominated tags from PRs successfully backported.
bors added a commit that referenced this pull request Feb 24, 2020
[stable] 1.41.1 release

This backports the following PRs:
 * Revert "Remove `checked_add` in `Layout::repeat`" #69241
 * Do not ICE when encountering `yield` inside `async` block #69175
 * Correct ICE caused by macros generating invalid spans. #68611
 * Changelog: Demonstrate final build-override syntax #68603
 * Resolve long compile times when evaluating always valid constants #67667
 * Fix MIR typeck soundness holes #69145

This also includes a commit which rustfmt's files which the latter commits touched (and perhaps a bit more) to make rebasing the PRs go more smoothly (thankfully, this should be the last time we need to do so).

I have removed stable-nominated tags from PRs successfully backported.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. stable-accepted Accepted for backporting to the compiler in the stable channel. 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.

rustc endless loop on some constant casts
10 participants