partially revertish `lazily "compute" anon const default substs` by BoxyUwU · Pull Request #92805 · rust-lang/rust

BoxyUwU · 2022-01-12T03:26:01Z

reverts #87280 except for some of the changes around ty::Unevaluated having a visitor and a generic for promoted
why revert: #92805 (comment)

r? @lcnr

rust-highfive · 2022-01-12T03:26:04Z

Some changes occured to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occured to rustc_codegen_cranelift

cc @bjorn3

Some changes occured to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred in src/tools/clippy.

cc @rust-lang/clippy

oli-obk · 2022-01-12T07:31:11Z

Why is this getting reverted?

BoxyUwU · 2022-01-12T07:36:07Z

I knew I forgot something when making this PR.... will add a description explaining the "why" 😅

lcnr · 2022-01-12T07:44:31Z

I think we want visit_unevaluated_const to stick around idk how useful that is by itself 🤔 the part which I want to remove again is the TypeFlags change as that one caused some bugs in the past (#90266 and #90375) and is generally just quite confusing.

I never got to filtering the generic params (which is also more difficult than originally assumed) and we will probably implement a different solution for the "unused substs" problem in the future, so keeping these changes doesn't seem too practical for feature(generic_const_exprs).

And finally, we don't need any substs filtering for #![feature(min_generic_const_expr)] which is what i want to focus on first

oli-obk · 2022-01-12T13:37:57Z

@bors r+

bors · 2022-01-12T13:37:58Z

📌 Commit e24b05e122b134496aa76e6b2333b1cbb5b618af has been approved by oli-obk

lcnr · 2022-01-12T14:06:23Z

@bors rollup=never

this is perf sensitive

lcnr · 2022-01-12T14:33:18Z

I would like to keep the Unevaluated split wrt promoteds and keep using visit_unevaluated_const.
That also means that we don't have to revert #88557 which is quite nice imo.

Should have been more clear when talking about #87280 and what i actually want to revert '^^

@bors r- for now

compiler/rustc_trait_selection/src/traits/object_safety.rs

RalfJung · 2022-01-12T17:21:59Z

Also some of these commit messages look like they should be fixed before merging.^^

lcnr · 2022-01-13T08:03:08Z

compiler/rustc_trait_selection/src/traits/wf.rs

we can remove that assert

lcnr

r=me afterwards

thanks @BoxyUwU :3

compiler/rustc_typeck/src/check/check.rs

src/tools/clippy/clippy_lints/src/escape.rs

src/tools/clippy/clippy_lints/src/non_copy_const.rs

src/tools/clippy/clippy_utils/src/consts.rs

lcnr · 2022-01-13T09:30:20Z

@bors r+ rollup=never

bors · 2022-01-13T09:30:22Z

📌 Commit 6899064be3ddaee785686a614179d9ccfd0d66a8 has been approved by lcnr

bors · 2022-01-14T21:12:39Z

☔ The latest upstream changes (presumably #92883) made this pull request unmergeable. Please resolve the merge conflicts.

BoxyUwU · 2022-01-15T03:44:26Z

@bors r=lcnr rollup=never

bors · 2022-01-15T03:44:27Z

📌 Commit 3f3a10f has been approved by lcnr

bors · 2022-01-16T11:19:25Z

⌛ Testing commit 3f3a10f with merge 7be8693...

bors · 2022-01-16T14:25:19Z

☀️ Test successful - checks-actions
Approved by: lcnr
Pushing 7be8693 to master...

rust-timer · 2022-01-16T17:10:07Z

Finished benchmarking commit (7be8693): comparison url.

Summary: This change led to moderate relevant mixed results 🤷 in compiler performance.

Moderate improvement in instruction counts (up to -1.4% on full builds of deeply-nested-async)
Small regression in instruction counts (up to 0.9% on incr-unchanged builds of clap-rs)

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression

lcnr · 2022-01-17T06:40:40Z

seems to mostly mirror the perf results from #87280 https://perf.rust-lang.org/compare.html?start=3b5df014390dcef66cc35f968fe51e9558e6ca13&end=22a1bd761de8b0ac85d9ecaae5f262d5798a4409

a bit surprised that the ctfe tests stayed about the same even though they regressed by about 4% in the original pr. it might make some sense to look a bit more into the impact of using `ty::Unevaluated' during type folding. Might get some more perf back in stress tests by adding some inlines there.

partially revertish `lazily "compute" anon const default substs` reverts #87280 except for some of the changes around `ty::Unevaluated` having a visitor and a generic for promoted why revert: <rust-lang/rust#92805 (comment)> r? `@lcnr`

Needed to adjust stdout for trait_impls, due to new allocator generic parameter for `Vec` and `Box`, decided it wasn't worth a separate commit.

partially revertish `lazily "compute" anon const default substs` reverts #87280 except for some of the changes around `ty::Unevaluated` having a visitor and a generic for promoted why revert: <rust-lang/rust#92805 (comment)> r? `@lcnr`

rust-highfive assigned lcnr Jan 12, 2022

rustbot added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jan 12, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 12, 2022

This comment has been minimized.

Sign in to view

BoxyUwU added the A-const-generics Area: const generics (parameters and arguments) label Jan 12, 2022

This comment has been minimized.

Sign in to view

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 Jan 12, 2022

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 Jan 12, 2022

lcnr reviewed Jan 12, 2022

View reviewed changes

compiler/rustc_trait_selection/src/traits/object_safety.rs Outdated Show resolved Hide resolved

BoxyUwU force-pushed the revert-lazy-anon-const-substs branch from 1facfa2 to 1312f61 Compare January 12, 2022 23:38

BoxyUwU changed the title ~~revert lazily "compute" anon const default substs~~ partially revertish lazily "compute" anon const default substs Jan 12, 2022

BoxyUwU force-pushed the revert-lazy-anon-const-substs branch from 1312f61 to 7e2cf3b Compare January 13, 2022 01:14

lcnr reviewed Jan 13, 2022

View reviewed changes

compiler/rustc_trait_selection/src/traits/wf.rs Outdated

Copy link

Contributor

lcnr Jan 13, 2022

Choose a reason for hiding this comment

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

we can remove that assert

lcnr reviewed Jan 13, 2022

View reviewed changes

BoxyUwU force-pushed the revert-lazy-anon-const-substs branch from 9d2a60e to 6899064 Compare January 13, 2022 09:27

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 Jan 14, 2022

BoxyUwU added 4 commits January 15, 2022 01:16

initial revert

71bbb60

attempt to re-add ty::Unevaluated visitor and friends

dec8ed4

unrevert # 88557

408a086

nyahggdshjjghsdfhgsf

3f3a10f

BoxyUwU force-pushed the revert-lazy-anon-const-substs branch from 6899064 to 3f3a10f Compare January 15, 2022 01:20

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 Jan 15, 2022

bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 16, 2022

bors merged commit 7be8693 into rust-lang:master Jan 16, 2022

rustbot added this to the 1.60.0 milestone Jan 16, 2022

This was referenced Jan 16, 2022

Enable MIR inlining #91743

Merged

Remove PartialOrd, Ord from DefId #90749

Closed

Override rustc version in ui and mir-opt tests to get stable hashes #92363

Merged

rustbot added the perf-regression Performance regression. label Jan 16, 2022

Alexendoo mentioned this pull request Jan 17, 2022

#![feature(generic_const_exprs)] ICE on evaluating constans both for type of generic const parameter and trait bound at the same type definition #90847

Closed

lcnr mentioned this pull request Mar 21, 2022

update section for type system constants rust-lang/rustc-dev-guide#1329

Merged

dario23 added a commit to dario23/rust-semverver that referenced this pull request May 4, 2022

Rustup to rust-lang/rust#92805

1ed57b8

dario23 added a commit to dario23/rust-semverver that referenced this pull request May 6, 2022

Rustup to rust-lang/rust#92805

8e92282

Needed to adjust stdout for trait_impls, due to new allocator generic parameter for `Vec` and `Box`, decided it wasn't worth a separate commit.

Uh oh!

Conversation

BoxyUwU commented Jan 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-highfive commented Jan 12, 2022

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

oli-obk commented Jan 12, 2022

Uh oh!

BoxyUwU commented Jan 12, 2022

Uh oh!

lcnr commented Jan 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oli-obk commented Jan 12, 2022

Uh oh!

bors commented Jan 12, 2022

Uh oh!

lcnr commented Jan 12, 2022

Uh oh!

lcnr commented Jan 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

RalfJung commented Jan 12, 2022

Uh oh!

lcnr Jan 13, 2022

Choose a reason for hiding this comment

Uh oh!

lcnr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lcnr commented Jan 13, 2022

Uh oh!

bors commented Jan 13, 2022

Uh oh!

bors commented Jan 14, 2022

Uh oh!

BoxyUwU commented Jan 15, 2022

Uh oh!

bors commented Jan 15, 2022

Uh oh!

bors commented Jan 16, 2022

Uh oh!

bors commented Jan 16, 2022

Uh oh!

rust-timer commented Jan 16, 2022

Uh oh!

lcnr commented Jan 17, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

BoxyUwU commented Jan 12, 2022 •

edited

Loading

lcnr commented Jan 12, 2022 •

edited

Loading

lcnr commented Jan 12, 2022 •

edited

Loading