continue `ParamEnv` to `TypingEnv` transition #133212

lcnr · 2024-11-19T15:17:38Z

cc #132279

r? @compiler-errors

rustbot · 2024-11-19T15:20:03Z

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri, @rust-lang/wg-const-eval

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred in match checking

cc @Nadrieril

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred to the CTFE machinery

cc @rust-lang/wg-const-eval

lcnr · 2024-11-19T16:18:36Z

cc @BoxyUwU on the last commit

lcnr · 2024-11-19T17:10:18Z

src/librustdoc/clean/mod.rs

-                        // Only anon consts can implicitly capture params.
-                        // FIXME: is this correct behavior?
-                        let typing_env = ty::TypingEnv::from_param_env(cx.tcx.param_env(*def_id));
+                        let typing_env = ty::TypingEnv::post_analysis(cx.tcx, *def_id);
                        cx.tcx.normalize_erasing_regions(typing_env, ct)


we're using normalize_erasing_regions to evaluate the constant, so it's gonna switch to Reveal::All anyways, cc @BoxyUwU

RalfJung · 2024-11-19T17:46:04Z

What about the part where InterpCx stores a TypingEnv?

Currently the InterpCx setup in Miri still seems rather odd to me.

lcnr · 2024-11-19T18:32:24Z

What about the part where InterpCx stores a TypingEnv?

Currently the InterpCx setup in Miri still seems rather odd to me.

I agree, wanted to do this in a separate PR so that this one doesn't grow out of hand 🤔 edit: just gonna do it here as well

RalfJung · 2024-11-19T18:39:54Z

Separate PR is fine, I was just confused since you didn't mention it in the PR description. :)

compiler-errors

r=me when green

lcnr · 2024-11-19T21:49:33Z

@bors r=compiler-errors rollup=iffy

bors · 2024-11-19T21:49:36Z

📌 Commit 002efeb has been approved by compiler-errors

It is now in the queue for this repository.

lcnr · 2024-11-19T21:49:59Z

@bors rollup=never

probably slightly impacts performance again

bors · 2024-11-20T06:22:05Z

⌛ Testing commit 002efeb with merge 70e814b...

bors · 2024-11-20T09:26:48Z

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing 70e814b to master...

RalfJung · 2024-11-20T10:00:34Z

compiler/rustc_const_eval/src/interpret/eval_context.rs

-    pub(crate) param_env: ty::ParamEnv<'tcx>,
+    /// The current context in case we're evaluating in a
+    /// polymorphic context. This always uses `ty::TypingMode::PostAnalysis`
+    pub typing_env: ty::TypingEnv<'tcx>,


I don't think we should make this public, I want to be sure nobody mutates this field... I'll make a PR to privatize this again.

rust-timer · 2024-11-20T10:45:02Z

Finished benchmarking commit (70e814b): comparison URL.

Overall result: ❌ regressions - please read the text below

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

If the regression was expected or you think it can be justified,
please write a comment with sufficient written justification, and add
@rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
If you think that you know of a way to resolve the regression, try to create
a new PR with a fix for the regression.
If you do not understand the regression or you think that it is just noise,
you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
were already notified of this PR).

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	0.4%	[0.3%, 0.5%]	5
Regressions ❌ (secondary)	2.1%	[0.1%, 6.6%]	12
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.4%	[0.3%, 0.5%]	5

Max RSS (memory usage)

Results (primary -1.4%, secondary -2.1%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	4.7%	[4.7%, 4.7%]	1
Improvements ✅ (primary)	-1.4%	[-1.4%, -1.4%]	1
Improvements ✅ (secondary)	-3.7%	[-5.6%, -2.7%]	4
All ❌✅ (primary)	-1.4%	[-1.4%, -1.4%]	1

Cycles

Results (secondary 4.5%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	4.5%	[1.6%, 7.2%]	11
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 793.573s -> 793.177s (-0.05%)
Artifact size: 335.42 MiB -> 335.96 MiB (0.16%)

lcnr · 2024-11-20T11:18:18Z

Passing TypingEnv to the const-eval queries seems to have a bigger perf impact. Will look into reverting this change and making a new TypingEnv from scratch in the query

finish `Reveal` removal After rust-lang#133212 changed the `TypingMode` to be the only source of truth, this entirely rips out `Reveal`. cc rust-lang#132279 r? `@compiler-errors`

RalfJung · 2024-11-20T11:28:15Z

We can also go back to not storing the full env in the Cx, if that helps.

interpret: make typing_env field private This was made public in rust-lang#133212 but IMO it should remain private. (Specifically, this prevents it from being mutated.) r? `@lcnr`

finish `Reveal` removal After rust-lang#133212 changed the `TypingMode` to be the only source of truth, this entirely rips out `Reveal`. cc rust-lang#132279 r? `@compiler-errors`

Rollup merge of rust-lang#133241 - RalfJung:typing-env, r=lcnr interpret: make typing_env field private This was made public in rust-lang#133212 but IMO it should remain private. (Specifically, this prevents it from being mutated.) r? `@lcnr`

interpret: make typing_env field private This was made public in rust-lang/rust#133212 but IMO it should remain private. (Specifically, this prevents it from being mutated.) r? `@lcnr`

rustbot assigned compiler-errors Nov 19, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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 Nov 19, 2024

lcnr force-pushed the questionable-uwu branch from 36fdfa9 to 7937ab3 Compare November 19, 2024 15:20

lcnr changed the title ~~move fn is_item_raw to TypingEnv~~ continue ParamEnv to TypingEnv transition Nov 19, 2024

lcnr added 6 commits November 19, 2024 18:06

move fn is_item_raw to TypingEnv

948cec0

unconditional recursion, yeet TypingEnv::from_param_env

1ec9648

liveness checking, yeet TypingEnv::from_param_env

decf37b

pattern lowering, yeet TypingEnv::from_param_env

07a5272

impl trait overcaptures, yeet TypingMode::from_param_env

ffd7a50

generic_const_exprs: yeet TypingEnv::from_param_env

f74951f

lcnr force-pushed the questionable-uwu branch from 7f67ec1 to ae20d31 Compare November 19, 2024 17:08

lcnr commented Nov 19, 2024

View reviewed changes

rustdoc: yeet TypingEnv::from_param_env

4813fda

lcnr force-pushed the questionable-uwu branch from cd994f4 to 4813fda Compare November 19, 2024 17:35

remove TypingMode::from_param_env in clippy

d667dd5

TypingMode::from_param_env begone

b9dea31

This comment has been minimized.

Sign in to view

lcnr added 3 commits November 19, 2024 21:36

InterpCx store TypingEnv instead of a ParamEnv

7a90e84

resolve_instance: stop relying on Reveal

d61effe

additional TypingEnv cleanups

002efeb

lcnr force-pushed the questionable-uwu branch from 617a554 to 002efeb Compare November 19, 2024 20:36

compiler-errors approved these changes Nov 19, 2024

View reviewed changes

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 Nov 19, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 20, 2024

bors merged commit 70e814b into rust-lang:master Nov 20, 2024
7 checks passed

rustbot added this to the 1.84.0 milestone Nov 20, 2024

This was referenced Nov 20, 2024

Implement the unsafe-fields RFC. #132915

Open

Detect const in pattern with typo #132658

Open

lint: change help for pointers to dyn types in FFI #131669

Open

Add context to "const in pattern" errors #133233

Open

RalfJung reviewed Nov 20, 2024

View reviewed changes

lcnr deleted the questionable-uwu branch November 20, 2024 10:04

RalfJung mentioned this pull request Nov 20, 2024

interpret: make typing_env field private #133241

Merged

rustbot added the perf-regression Performance regression. label Nov 20, 2024

lcnr mentioned this pull request Nov 20, 2024

finish Reveal removal #133242

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

continue `ParamEnv` to `TypingEnv` transition #133212

continue `ParamEnv` to `TypingEnv` transition #133212

lcnr commented Nov 19, 2024 •

edited

Loading

rustbot commented Nov 19, 2024

lcnr commented Nov 19, 2024

lcnr Nov 19, 2024

RalfJung commented Nov 19, 2024

lcnr commented Nov 19, 2024 •

edited

Loading

RalfJung commented Nov 19, 2024

This comment has been minimized.

compiler-errors left a comment

lcnr commented Nov 19, 2024

bors commented Nov 19, 2024

lcnr commented Nov 19, 2024

bors commented Nov 20, 2024

bors commented Nov 20, 2024

RalfJung Nov 20, 2024

rust-timer commented Nov 20, 2024

lcnr commented Nov 20, 2024

RalfJung commented Nov 20, 2024 via email

continue ParamEnv to TypingEnv transition #133212

continue ParamEnv to TypingEnv transition #133212

Conversation

lcnr commented Nov 19, 2024 • edited Loading

rustbot commented Nov 19, 2024

lcnr commented Nov 19, 2024

lcnr Nov 19, 2024

Choose a reason for hiding this comment

RalfJung commented Nov 19, 2024

lcnr commented Nov 19, 2024 • edited Loading

RalfJung commented Nov 19, 2024

This comment has been minimized.

compiler-errors left a comment

Choose a reason for hiding this comment

lcnr commented Nov 19, 2024

bors commented Nov 19, 2024

lcnr commented Nov 19, 2024

bors commented Nov 20, 2024

bors commented Nov 20, 2024

RalfJung Nov 20, 2024

Choose a reason for hiding this comment

rust-timer commented Nov 20, 2024

Overall result: ❌ regressions - please read the text below

Instruction count

Max RSS (memory usage)

Cycles

Binary size

lcnr commented Nov 20, 2024

RalfJung commented Nov 20, 2024 via email

continue `ParamEnv` to `TypingEnv` transition #133212

continue `ParamEnv` to `TypingEnv` transition #133212

lcnr commented Nov 19, 2024 •

edited

Loading

lcnr commented Nov 19, 2024 •

edited

Loading