Cleanup: Eliminate ConstnessAnd #90274

oli-obk · 2021-10-25T16:52: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. I am looking into that, but not sure if I'll get to it this week.

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 pull request is best reviewed commit by commit, as it works on each commit (modulo some ui changes that will go away once I squash 764aac2 into 8e84aa9

r? @fee1-dead

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-10-25T16:52:21Z

Some changes occured to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occured to the CTFE / Miri engine

cc @rust-lang/miri

oli-obk · 2021-10-25T16:55:58Z

@bors try @rust-timer queue

rust-timer · 2021-10-25T16:55:59Z

Awaiting bors try build completion.

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

bors · 2021-10-25T16:56:06Z

⌛ Trying commit 764aac2 with merge 5149eb5f1733b5aeadfdd0a2e8a18c34cc023b71...

rust-log-analyzer · 2021-10-25T16:56:45Z

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

    Checking tracing-tree v0.1.9
    Checking rustdoc-json-types v0.1.0 (/checkout/src/rustdoc-json-types)
    Checking tera v1.10.0
    Checking rustdoc v0.0.0 (/checkout/src/librustdoc)
error[E0432]: unresolved import `rustc_middle::ty::WithConstness`
 --> src/librustdoc/clean/blanket_impl.rs:6:37
  |
6 | use rustc_middle::ty::{ToPredicate, WithConstness};
  |                                     ^^^^^^^^^^^^^ no `WithConstness` in `ty`
For more information about this error, try `rustc --explain E0432`.
error: could not compile `rustdoc` due to previous error
Build completed unsuccessfully in 0:02:33

bors · 2021-10-25T18:13:18Z

💔 Test failed - checks-actions

rust-log-analyzer · 2021-10-25T18:13:22Z

The job dist-x86_64-apple failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

Updating files:  98% (30537/31041)
Updating files:  99% (30731/31041)
Updating files: 100% (31041/31041)
Updating files: 100% (31041/31041), done.
Switched to a new branch 'try'
Branch 'try' set up to track remote branch 'try' from 'origin'.
[command]/usr/local/bin/git log -1 --format='%H'
'5149eb5f1733b5aeadfdd0a2e8a18c34cc023b71'
##[group]Run src/ci/scripts/setup-environment.sh
src/ci/scripts/setup-environment.sh
---
   Compiling tera v1.10.0
[RUSTC-TIMING] rustdoc_json_types test:false 4.575
[RUSTC-TIMING] tracing_subscriber test:false 15.049
   Compiling rustdoc v0.0.0 (/Users/runner/work/rust/rust/src/librustdoc)
error[E0432]: unresolved import `rustc_middle::ty::WithConstness`
 --> src/librustdoc/clean/blanket_impl.rs:6:37
  |
6 | use rustc_middle::ty::{ToPredicate, WithConstness};
  |                                     ^^^^^^^^^^^^^ no `WithConstness` in `ty`
For more information about this error, try `rustc --explain E0432`.
[RUSTC-TIMING] rustdoc test:false 6.018
error: could not compile `rustdoc` due to previous error
warning: build failed, waiting for other jobs to finish...

bors · 2021-10-26T01:09:00Z

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

fee1-dead

Some queries do not care about if we are in a const context. Maybe we could always change the ParamEnv to be NonConst before caching?

I also think that we need a test for reproducing the error caused by the bad caching system.

fee1-dead · 2021-10-26T04:28:20Z

compiler/rustc_infer/src/traits/mod.rs

+impl TraitObligation<'tcx> {
+    /// Returns `true` if the trait predicate is considered `const` in its ParamEnv.
+    pub fn is_const(&self) -> bool {
+        match (self.predicate.skip_binder().constness, self.param_env.constness()) {
+            (ty::BoundConstness::ConstIfConst, hir::Constness::Const) => true,
+            _ => false,
+        }
+    }
+}
+


I think this can hurt performance by making cache misses more common. Before it is just the trait and the constness, now it is the predicate's constness AND the ParamEnv's constness.

NonConst predicate inside a NonConst context is very common.

But what about a NonConst predicate inside a Const context? For example associated consts in traits. These don't use the same cache.

Perhaps we need a function BoundConstness.and(hir::Constness) -> hir::Constness and use this to change the ParamEnv's constness to the result, not storing the BoundConstness in the cache.

We could make ParamEnv::and go through its anded value and check whether it needs constness, and remove said constness from itself if not

oli-obk · 2021-10-26T07:24:06Z

Maybe we could always change the ParamEnv to be NonConst before caching?

How would we handle the queries that do require constness? We could add a new query modifier stating whether constness is relevant and if not, automatically adjust the paramenv in its arguments

fee1-dead · 2021-10-26T11:58:35Z

We could add a new query modifier stating whether constness is relevant and if not, automatically adjust the paramenv in its arguments

Yes, that is what I meant. I used "always" changing, but I forgot to say to change those "that do not require constness"

spastorino · 2021-10-26T12:39:01Z

librustdoc is failing to compile, it has references to WithConstness

fee1-dead · 2021-11-28T17:35:36Z

@oli-obk: do you mind if I take this PR and update it? You seem quite busy with other stuff and I really want this to land. I tried rebasing the PR a while back and I couldn't push to your repository.

oli-obk · 2021-11-28T17:54:52Z

Yea, feel free to take over. I recommend splitting it into parts though, as the non-param-env parts should be perf and behaviour neutral

spastorino · 2021-11-29T12:47:24Z

@fee1-dead ohh great. I've promised @oli-obk that I was going to take this over but had some stuff on my plate and couldn't yet. If you're going ahead please do, otherwise I'd take it over probably this week.

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`

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/rust#90274. r? `@oli-obk` cc `@spastorino` `@ecstatic-morse`

oli-obk · 2022-01-12T14:20:48Z

Thanks @fee1-dead for carrying this over the finish line

oli-obk added 12 commits October 25, 2021 11:18

Prepare for more ParamEnv flags

b966d1c

Add constness to ParamEnv

8e84aa9

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

Static items are also const

8c0edca

Re-use constness_for_typeck instead of rolling it ourselves

ffd46d0

Fixup add const to param env

fce93cb

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

3193cf7

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

Fixup

104593f

Prefer TraitPredicate over ConstnessAnd<TraitRef>

cde1f6a

Replace ConstnessAnd<TraitRef> with TraitPredicate which conveys …

63ce5c1

…the same information

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

3be7786

…se TraitPredicate instead

Completely remove ConstnessAnd

afbac75

Always use const param envs for const eval.

764aac2

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 assigned fee1-dead Oct 25, 2021

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

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

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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 25, 2021

fee1-dead reviewed Oct 26, 2021

View reviewed changes

fee1-dead removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 26, 2021

fee1-dead mentioned this pull request Nov 29, 2021

Cleanup: Eliminate ConstnessAnd #91354

Merged

oli-obk closed this Jan 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cleanup: Eliminate ConstnessAnd #90274

Cleanup: Eliminate ConstnessAnd #90274

oli-obk commented Oct 25, 2021 •

edited

Loading

rust-highfive commented Oct 25, 2021

oli-obk commented Oct 25, 2021

rust-timer commented Oct 25, 2021

bors commented Oct 25, 2021

rust-log-analyzer commented Oct 25, 2021

bors commented Oct 25, 2021

rust-log-analyzer commented Oct 25, 2021

bors commented Oct 26, 2021

fee1-dead left a comment

fee1-dead Oct 26, 2021

oli-obk Oct 26, 2021

oli-obk commented Oct 26, 2021

fee1-dead commented Oct 26, 2021

spastorino commented Oct 26, 2021 •

edited

Loading

fee1-dead commented Nov 28, 2021 •

edited

Loading

oli-obk commented Nov 28, 2021

spastorino commented Nov 29, 2021

oli-obk commented Jan 12, 2022

Cleanup: Eliminate ConstnessAnd #90274

Cleanup: Eliminate ConstnessAnd #90274

Conversation

oli-obk commented Oct 25, 2021 • edited Loading

rust-highfive commented Oct 25, 2021

oli-obk commented Oct 25, 2021

rust-timer commented Oct 25, 2021

bors commented Oct 25, 2021

rust-log-analyzer commented Oct 25, 2021

bors commented Oct 25, 2021

rust-log-analyzer commented Oct 25, 2021

bors commented Oct 26, 2021

fee1-dead left a comment

Choose a reason for hiding this comment

fee1-dead Oct 26, 2021

Choose a reason for hiding this comment

oli-obk Oct 26, 2021

Choose a reason for hiding this comment

oli-obk commented Oct 26, 2021

fee1-dead commented Oct 26, 2021

spastorino commented Oct 26, 2021 • edited Loading

fee1-dead commented Nov 28, 2021 • edited Loading

oli-obk commented Nov 28, 2021

spastorino commented Nov 29, 2021

oli-obk commented Jan 12, 2022

oli-obk commented Oct 25, 2021 •

edited

Loading

spastorino commented Oct 26, 2021 •

edited

Loading

fee1-dead commented Nov 28, 2021 •

edited

Loading