Always evaluate free constants and statics, even if previous errors occurred #121087

oli-obk · 2024-02-14T15:02:00Z

work towards #79738

We will need to evaluate static items before the definitions.freeze() below, as we will start creating new DefIds (for nested allocations) within the eval_static_initializer query.

But even without that motivation, this is a good change. Hard errors should always be reported and not silenced if other errors happened earlier.

rustbot · 2024-02-14T15:02:10Z

r? @lcnr

rustbot has assigned @lcnr.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2024-02-14T15:02:12Z

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

rustbot · 2024-02-14T16:47:48Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

rustbot · 2024-02-14T20:17:04Z

The Miri subtree was changed

cc @rust-lang/miri

compiler/rustc_middle/src/mir/interpret/queries.rs

compiler/rustc_hir_analysis/src/lib.rs

tests/ui/consts/const-mut-refs/mut_ref_in_final.stderr

lcnr

r=me after nits + CI

compiler/rustc_lint/src/builtin.rs

lcnr · 2024-02-15T07:35:35Z

tests/ui/consts/miri_unleashed/mutable_references.rs

 static BOO: &mut Foo<()> = &mut Foo(());
+//~^ ERROR encountered mutable pointer in final value of static

 // interior mutability is fine


outdated comment, this test generally feels somewhat useless rn, given that its behavior doesn't depend on -Zunleash-the-miri-inside-of-you afaict?

stderr has a bunch of "skipping check that does not even have a feature gate", so I think it does depend on the unleash flag.

oli-obk · 2024-02-15T12:29:46Z

compiler/rustc_const_eval/src/transform/check_consts/check.rs

        let secondary_errors = mem::take(&mut self.secondary_errors);
        if self.error_emitted.is_none() {
            for error in secondary_errors {
-                error.emit();
+                self.error_emitted = Some(error.emit());


@RalfJung found the issue that caused us to const eval even though there were const qualif errors 🙈

bors · 2024-02-15T13:30:53Z

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

bjorn3 · 2024-02-15T16:09:42Z

as we will start creating new DefIds (for nested allocations) within the eval_static_initializer query.

Wouldn't it be possible to represent the id of nested statics as a pair of "def id of containing static" and "index of this nested static"?

oli-obk · 2024-02-15T16:35:15Z

Wouldn't it be possible to represent the id of nested statics as a pair of "def id of containing static" and "index of this nested static"?

yes, but that just makes everything more complex, as then everything needs to handle this special case. I tried it, and it's annoying to get right. Just generating a new static per nested allocation was significantly simpler.

oli-obk · 2024-02-15T16:35:30Z

@bors r=lcnr

bors · 2024-02-15T16:35:32Z

📌 Commit 7770df0 has been approved by lcnr

It is now in the queue for this repository.

rust-timer · 2024-02-20T12:27:54Z

Finished benchmarking commit (cce6a6e): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

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
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.8%	[0.4%, 2.0%]	4
Regressions ❌ (secondary)	1.0%	[0.3%, 5.2%]	11
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.8%	[0.4%, 2.0%]	4

Max RSS (memory usage)

Results

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)	9.9%	[1.5%, 15.6%]	22
Improvements ✅ (primary)	-3.3%	[-4.0%, -2.4%]	3
Improvements ✅ (secondary)	-3.2%	[-5.3%, -1.4%]	3
All ❌✅ (primary)	-3.3%	[-4.0%, -2.4%]	3

Cycles

Results

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)	3.3%	[2.2%, 4.1%]	7
Regressions ❌ (secondary)	3.2%	[2.8%, 3.7%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	3.3%	[2.2%, 4.1%]	7

Binary size

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

Bootstrap: 641.541s -> 639.567s (-0.31%)
Artifact size: 308.58 MiB -> 310.63 MiB (0.66%)

oli-obk · 2024-02-20T12:55:43Z

Looking into the regressions. They are unintended

…ion, r=<try> Avoid some unnecessary query invocations. Specifically this inlines `const_eval_poly` and avoids computing the generic params, the param env, normalizing the param env and erasing lifetimes on everything. should fix the perf regression from rust-lang#121087

oli-obk · 2024-02-23T10:07:55Z

@rustbot label: +perf-regression-triaged

I fixed the libc regression in #121387

The other regressions' detailed view actually shows improvements instead of a regression, and the benchmarks themselves have been flaky on the same scale as they were affected here, so I'm guessing it's not related

…ion, r=<try> Avoid some unnecessary query invocations. Specifically this inlines `const_eval_poly` and avoids computing the generic params, the param env, normalizing the param env and erasing lifetimes on everything. should fix the perf regression from rust-lang#121087

Always evaluate free constants and statics, even if previous errors occurred work towards rust-lang#79738 We will need to evaluate static items before the `definitions.freeze()` below, as we will start creating new `DefId`s (for nested allocations) within the `eval_static_initializer` query. But even without that motivation, this is a good change. Hard errors should always be reported and not silenced if other errors happened earlier.

…ion, r=lcnr Avoid some unnecessary query invocations. Specifically this inlines `const_eval_poly` and avoids computing the generic params, the param env, normalizing the param env and erasing lifetimes on everything. should fix the perf regression from rust-lang#121087

Avoid some unnecessary query invocations. Specifically this inlines `const_eval_poly` and avoids computing the generic params, the param env, normalizing the param env and erasing lifetimes on everything. should fix the perf regression from rust-lang/rust#121087

rustbot assigned lcnr Feb 14, 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. labels Feb 14, 2024

This comment has been minimized.

Sign in to view

oli-obk force-pushed the eager_const_failures branch from 2cdd83a to 17adcb3 Compare February 15, 2024 05:20

This comment has been minimized.

Sign in to view

RalfJung reviewed Feb 15, 2024

View reviewed changes

compiler/rustc_middle/src/mir/interpret/queries.rs Outdated Show resolved Hide resolved

RalfJung reviewed Feb 15, 2024

View reviewed changes

compiler/rustc_hir_analysis/src/lib.rs Show resolved Hide resolved

RalfJung reviewed Feb 15, 2024

View reviewed changes

tests/ui/consts/const-mut-refs/mut_ref_in_final.stderr Outdated Show resolved Hide resolved

lcnr reviewed Feb 15, 2024

View reviewed changes

oli-obk force-pushed the eager_const_failures branch 2 times, most recently from 0bb9b2b to 3f88472 Compare February 15, 2024 11:45

This comment has been minimized.

Sign in to view

oli-obk force-pushed the eager_const_failures branch from 3f88472 to e8e0f86 Compare February 15, 2024 12:29

oli-obk commented Feb 15, 2024

View reviewed changes

oli-obk force-pushed the eager_const_failures branch from e8e0f86 to 39d167c Compare February 15, 2024 15:08

oli-obk mentioned this pull request Feb 15, 2024

Tracking issue for &mut T in const contexts (const_mut_refs) #57349

Closed

8 tasks

This comment has been minimized.

Sign in to view

oli-obk force-pushed the eager_const_failures branch from 39d167c to 7770df0 Compare February 15, 2024 16:19

bors merged commit cce6a6e into rust-lang:master Feb 20, 2024
12 checks passed

rustbot added this to the 1.78.0 milestone Feb 20, 2024

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

oli-obk deleted the eager_const_failures branch February 20, 2024 12:41

oli-obk mentioned this pull request Feb 21, 2024

Avoid some unnecessary query invocations. #121387

Merged

matthiaskrgr mentioned this pull request Feb 22, 2024

ICE: assertion failed: layout.is_sized() #121443

Closed

rustbot added the perf-regression-triaged The performance regression has been triaged. label Feb 23, 2024

matthiaskrgr mentioned this pull request Mar 3, 2024

ICE: effects: Unexpected parameter Type(()) when instantiating in region 'x (index=1) #121957

Closed

This was referenced Mar 14, 2024

ICE: Unexpected unsized type tail: &ReStatic [u8] #122488

Closed

ICE: None, compiler/rustc_const_eval/src/interpret/validity.rs #122548

Closed

This was referenced Mar 26, 2024

ICE: !base.layout().is_sized() #123078

Closed

ICE: write_immediate_to_mplace: invalid Scalar layout: TyAndLayout{ ... #123153

Closed

cushionbadak mentioned this pull request Apr 20, 2024

ICE: assertion failed: layout.is_sized() #124182

Closed

matthiaskrgr mentioned this pull request Apr 28, 2024

ICE: write_immediate_to_mplace: invalid Scalar layout: TyAndLayout #124464

Closed

cushionbadak mentioned this pull request Apr 30, 2024

ICE: type mismatch when copying! with no feature provided #124552

Closed

lqd mentioned this pull request Jun 26, 2024

ICE: assertion left == right failed in rustc_const_eval/src/interpret/cast.rs unsize_into #126982

Open

This was referenced Jul 13, 2024

ICE: primitive read not possible for type #127676

Closed

ICE: failed to resolve instance for ErrorGuaranteed() #127826

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Always evaluate free constants and statics, even if previous errors occurred #121087

Always evaluate free constants and statics, even if previous errors occurred #121087

oli-obk commented Feb 14, 2024

rustbot commented Feb 14, 2024

rustbot commented Feb 14, 2024

This comment has been minimized.

rustbot commented Feb 14, 2024

This comment has been minimized.

This comment has been minimized.

rustbot commented Feb 14, 2024

This comment has been minimized.

lcnr left a comment

lcnr Feb 15, 2024

RalfJung Feb 15, 2024

This comment has been minimized.

oli-obk Feb 15, 2024

bors commented Feb 15, 2024

This comment has been minimized.

bjorn3 commented Feb 15, 2024

oli-obk commented Feb 15, 2024

oli-obk commented Feb 15, 2024

bors commented Feb 15, 2024

rust-timer commented Feb 20, 2024

oli-obk commented Feb 20, 2024

oli-obk commented Feb 23, 2024

Always evaluate free constants and statics, even if previous errors occurred #121087

Always evaluate free constants and statics, even if previous errors occurred #121087

Conversation

oli-obk commented Feb 14, 2024

rustbot commented Feb 14, 2024

rustbot commented Feb 14, 2024

This comment has been minimized.

rustbot commented Feb 14, 2024

This comment has been minimized.

This comment has been minimized.

rustbot commented Feb 14, 2024

This comment has been minimized.

lcnr left a comment

Choose a reason for hiding this comment

lcnr Feb 15, 2024

Choose a reason for hiding this comment

RalfJung Feb 15, 2024

Choose a reason for hiding this comment

This comment has been minimized.

oli-obk Feb 15, 2024

Choose a reason for hiding this comment

bors commented Feb 15, 2024

This comment has been minimized.

bjorn3 commented Feb 15, 2024

oli-obk commented Feb 15, 2024

oli-obk commented Feb 15, 2024

bors commented Feb 15, 2024

rust-timer commented Feb 20, 2024

Overall result: ❌ regressions - ACTION NEEDED

Instruction count

Max RSS (memory usage)

Cycles

Binary size

oli-obk commented Feb 20, 2024

oli-obk commented Feb 23, 2024