Cleanup: Eliminate ConstnessAnd #91354

fee1-dead · 2021-11-29T13:25:18Z

This is almost a behaviour-free change and purely a refactoring. "almost" because we appear to be using the wrong ParamEnv somewhere already, and this is now exposed by failing a test using the unstable ~const feature.

We most definitely need to review all without_const and at some point should probably get rid of many of them by using TraitPredicate instead of TraitRef.

This is a continuation of #90274.

r? @oli-obk

cc @spastorino @ecstatic-morse

This now causes a lot of queries to be executed twice, as reveal_all forces NotConst

…ension for it This breaks a ~const test that will be fixed in a follow up commit of this PR

…the same information

…se TraitPredicate instead

Nothing else makes sense, and there is no "danger" in doing so, as it only does something if there are const bounds, which are unstable. This used to happen implicitly via the inferctxt before, which was much more fragile.

rust-highfive · 2021-11-29T13:25:21Z

Some changes occurred in src/tools/clippy.

cc @rust-lang/clippy

Some changes occured to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occured to the CTFE / Miri engine

cc @rust-lang/miri

fee1-dead · 2021-11-29T13:25:52Z

@bors try @rust-timer queue

rust-timer · 2021-11-29T13:25:53Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2021-11-29T13:26:00Z

⌛ Trying commit 8710a2e with merge 3bde14d5d8c5c093944947c2dfb90ea047842a0b...

oli-obk · 2021-11-29T13:55:20Z

r? @spastorino

Can't review a PR made up of mostly my own commits

oli-obk · 2021-11-29T14:01:18Z

Tho... essentially @fee1-dead can review this, you only rebased it and fixed some tools. I guess r=oli-obk,fee1-dead if you are alright with my commits

fee1-dead · 2021-11-29T14:20:42Z

I don't think I can do that, as this PR requires some work (fixing the assoc type ui test), also improving caching of queries that do not care about constness if the performance had gotten worse.

The ui test fix is quite simple. constness_for_typeck should return Constness::Const when the item is ImplItem::Fn | ImplItem::Const in a const impl. I had to remove the function from rustc_hir as the crate doesn't know what TyCtxt is and therefore cannot query the parent of a node easily.

fee1-dead · 2021-12-02T03:56:59Z

@bors r=spastorino

bors · 2021-12-02T03:57:00Z

📌 Commit 5ebc99e has been approved by spastorino

bors · 2021-12-02T11:49:01Z

⌛ Testing commit 5ebc99e with merge 18bb8c6...

spastorino · 2021-12-02T12:46:44Z

I'm not sure how perf labelling works really but seems like we have perf-regression left from the first perf run, but the second shows mixed results.
Should we manually remove perf-regression label? if that's the case maybe it's nice if the perf tooling automatically does that in the second run.

cc @rust-lang/wg-compiler-performance

fee1-dead · 2021-12-02T14:07:16Z

rustbot label: +S-waiting-on-review -S-waiting-on-perf +perf-regression

perfbot always marks PRs with regressions as regressions. It automatically removes the label when there are no regressions, which is not the case for this PR.

bors · 2021-12-02T14:53:14Z

☀️ Test successful - checks-actions
Approved by: spastorino
Pushing 18bb8c6 to master...

rust-timer · 2021-12-02T16:23:34Z

Finished benchmarking commit (18bb8c6): comparison url.

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

Small improvement in instruction counts (up to -0.6% on full builds of deeply-nested)
Moderate regression in instruction counts (up to 1.1% on incr-patched: b9b3e592dd cherry picked builds of style-servo)

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

oli-obk · 2021-12-02T19:22:12Z

I have an idea that could address the remaining regressions: https://rust-lang.zulipchat.com/#narrow/stream/146212-t-compiler.2Fconst-eval/topic/ParamEnv.20and.20constness/near/263477812

sdroege · 2021-12-03T11:52:41Z

This seems to cause a regression, see #91489

oli-obk · 2021-12-03T12:13:59Z

Oof. That's quite odd. Let's revert this PR and go with a smaller subset of the changes first?

…rino" This reverts commit 18bb8c6, reversing changes made to d9baa36.

Revert "Auto merge of rust-lang#91354 - fee1-dead:const_env, r=spastorino" This reverts commit 18bb8c6, reversing changes made to d9baa36. Reverts rust-lang#91354 in order to address rust-lang#91489. We would need to place this changes in a more granular way and would also be nice to address the small perf regression that was also introduced. r? `@oli-obk` cc `@fee1-dead`

Cleanup: Eliminate ConstnessAnd This is almost a behaviour-free change and purely a refactoring. "almost" because we appear to be using the wrong ParamEnv somewhere already, and this is now exposed by failing a test using the unstable `~const` feature. We most definitely need to review all `without_const` and at some point should probably get rid of many of them by using `TraitPredicate` instead of `TraitRef`. This is a continuation of rust-lang#90274. r? `@oli-obk` cc `@spastorino` `@ecstatic-morse`

…rino" This reverts commit 18bb8c6, reversing changes made to d9baa36.

oli-obk and others added 14 commits November 29, 2021 21:19

Prepare for more ParamEnv flags

22eeff7

Add constness to ParamEnv

721ffd1

This now causes a lot of queries to be executed twice, as reveal_all forces NotConst

Static items are also const

1761d88

Re-use constness_for_typeck instead of rolling it ourselves

e37947f

Fixup add const to param env

19f2101

Use the constness from the param env instead of having a separate dim…

d51068c

…ension for it This breaks a ~const test that will be fixed in a follow up commit of this PR

Fixup

40f39e6

Prefer TraitPredicate over ConstnessAnd<TraitRef>

b16c811

Replace ConstnessAnd<TraitRef> with TraitPredicate which conveys …

d161cc2

…the same information

Avoid storing the ImplPolarity and Constness next to a TraitRef and u…

a848c4b

…se TraitPredicate instead

Completely remove ConstnessAnd

a9a79f6

Always use const param envs for const eval.

f394bb5

Nothing else makes sense, and there is no "danger" in doing so, as it only does something if there are const bounds, which are unstable. This used to happen implicitly via the inferctxt before, which was much more fragile.

Fix tools

fdf5322

Reformat everything

8710a2e

rust-highfive assigned oli-obk Nov 29, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 29, 2021

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Nov 29, 2021

This comment has been minimized.

Sign in to view

rust-highfive assigned spastorino and unassigned oli-obk Nov 29, 2021

ParamEnv should be const when ImplItem is within a const impl.

87cd1ce

fee1-dead marked this pull request as draft November 29, 2021 14:41

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 2, 2021

bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 2, 2021

bors merged commit 18bb8c6 into rust-lang:master Dec 2, 2021

rustbot added this to the 1.59.0 milestone Dec 2, 2021

fee1-dead deleted the const_env branch December 3, 2021 10:33

sdroege mentioned this pull request Dec 3, 2021

nightly-2021-12-02 to nightly-2021-12-03 regression (PR #91354): auto-deref on Cow does not seem to work anymore in all circumstances #91489

Closed

spastorino added a commit to spastorino/rust that referenced this pull request Dec 3, 2021

Revert "Auto merge of rust-lang#91354 - fee1-dead:const_env, r=spasto…

85b723c

…rino" This reverts commit 18bb8c6, reversing changes made to d9baa36.

spastorino mentioned this pull request Dec 3, 2021

Revert "Auto merge of #91354 - fee1-dead:const_env, r=spastorino" #91491

Merged

fee1-dead restored the const_env branch December 3, 2021 16:45

flip1995 pushed a commit to flip1995/rust that referenced this pull request Dec 17, 2021

Revert "Auto merge of rust-lang#91354 - fee1-dead:const_env, r=spasto…

23f752f

…rino" This reverts commit 18bb8c6, reversing changes made to d9baa36.

fee1-dead added the F-const_trait_impl `#![feature(const_trait_impl)]` label Jul 20, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cleanup: Eliminate ConstnessAnd #91354

Cleanup: Eliminate ConstnessAnd #91354

fee1-dead commented Nov 29, 2021

rust-highfive commented Nov 29, 2021

fee1-dead commented Nov 29, 2021

rust-timer commented Nov 29, 2021

bors commented Nov 29, 2021

This comment has been minimized.

oli-obk commented Nov 29, 2021

oli-obk commented Nov 29, 2021

fee1-dead commented Nov 29, 2021

fee1-dead commented Dec 2, 2021

bors commented Dec 2, 2021

bors commented Dec 2, 2021

spastorino commented Dec 2, 2021

fee1-dead commented Dec 2, 2021

bors commented Dec 2, 2021

rust-timer commented Dec 2, 2021

oli-obk commented Dec 2, 2021

sdroege commented Dec 3, 2021

oli-obk commented Dec 3, 2021

Cleanup: Eliminate ConstnessAnd #91354

Cleanup: Eliminate ConstnessAnd #91354

Conversation

fee1-dead commented Nov 29, 2021

rust-highfive commented Nov 29, 2021

fee1-dead commented Nov 29, 2021

rust-timer commented Nov 29, 2021

bors commented Nov 29, 2021

This comment has been minimized.

oli-obk commented Nov 29, 2021

oli-obk commented Nov 29, 2021

fee1-dead commented Nov 29, 2021

fee1-dead commented Dec 2, 2021

bors commented Dec 2, 2021

bors commented Dec 2, 2021

spastorino commented Dec 2, 2021

fee1-dead commented Dec 2, 2021

bors commented Dec 2, 2021

rust-timer commented Dec 2, 2021

oli-obk commented Dec 2, 2021

sdroege commented Dec 3, 2021

oli-obk commented Dec 3, 2021