Introduce mir::Unevaluated by b-naber · Pull Request #102056 · rust-lang/rust

b-naber · 2022-09-20T15:41:57Z

Previously the distinction between unevaluated constants in the type-system and in mir was not explicit and a little confusing. Probably better to introduce its own type for that.

r? @lcnr

rustbot · 2022-09-20T15:42:01Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred in const_evaluatable.rs

cc @lcnr

lcnr

nice, small nits, then r=me

compiler/rustc_middle/src/mir/mod.rs

compiler/rustc_middle/src/ty/consts/kind.rs

lcnr · 2022-09-20T15:57:10Z

compiler/rustc_middle/src/mir/mod.rs

yeah, once we've removed visit_mir_constant we should be able to remove this, probably makes sense to do that in yet another pr 😁

bors · 2022-09-20T22:51:18Z

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

compiler/rustc_middle/src/mir/mod.rs

lcnr

final nit, then r=me

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

lcnr · 2022-09-22T12:14:32Z

@bors delegate+

bors · 2022-09-22T12:14:34Z

✌️ @b-naber can now approve this pull request

lcnr · 2022-09-23T13:12:12Z

compiler/rustc_mir_transform/src/inline.rs

                    callee_body.required_consts.iter().copied().filter(|&ct| match ct.literal {
                        ConstantKind::Ty(_) => {
-                            bug!("should never encounter ty::Unevaluated in `required_consts`")
+                            bug!("should never encounter ty::UnevaluatedConst in `required_consts`")


this should be "unexpected ty::Const"

lcnr · 2022-09-23T13:15:00Z

@bors r+ rollup=never

maybe perf relevant, probably not 🤷 the queue is pretty short rn

bors · 2022-09-23T13:15:02Z

📌 Commit a705e65 has been approved by lcnr

It is now in the queue for this repository.

bors · 2022-09-23T13:39:19Z

⌛ Testing commit a705e65 with merge 9a963e3...

bors · 2022-09-23T16:43:59Z

☀️ Test successful - checks-actions
Approved by: lcnr
Pushing 9a963e3 to master...

rust-timer · 2022-09-23T18:01:18Z

Finished benchmarking commit (9a963e3): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

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)	2.4%	[2.0%, 2.9%]	2
Improvements ✅ (primary)	-3.5%	[-3.5%, -3.5%]	1
Improvements ✅ (secondary)	-1.7%	[-1.7%, -1.7%]	1
All ❌✅ (primary)	-3.5%	[-3.5%, -3.5%]	1

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)	-	-	0
Regressions ❌ (secondary)	4.0%	[4.0%, 4.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.2%	[-4.5%, -2.6%]	4
All ❌✅ (primary)	-	-	0

the arithmetic mean of the percent change ↩ ↩²
number of relevant changes ↩ ↩²

Introduce mir::Unevaluated Previously the distinction between unevaluated constants in the type-system and in mir was not explicit and a little confusing. Probably better to introduce its own type for that. r? `@lcnr`

rust-highfive assigned lcnr Sep 20, 2022

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

lcnr reviewed Sep 20, 2022

View reviewed changes

compiler/rustc_middle/src/mir/mod.rs Outdated Show resolved Hide resolved

compiler/rustc_middle/src/ty/consts/kind.rs Outdated Show resolved Hide resolved

lcnr reviewed Sep 20, 2022

View reviewed changes

lcnr 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 Sep 21, 2022

b-naber force-pushed the unevaluated branch from 6f42f1a to 85aa94f Compare September 21, 2022 17:02

RalfJung reviewed Sep 21, 2022

View reviewed changes

compiler/rustc_middle/src/mir/mod.rs Outdated Show resolved Hide resolved

b-naber added 3 commits September 22, 2022 12:35

introduce mir::Unevaluated

9f3784d

bless tests

db7df59

address review

efb9089

b-naber force-pushed the unevaluated branch 2 times, most recently from 23d044f to fad44e0 Compare September 22, 2022 10:39

lcnr reviewed Sep 22, 2022

View reviewed changes

compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs Outdated Show resolved Hide resolved

b-naber force-pushed the unevaluated branch from fad44e0 to 43ddb7f Compare September 23, 2022 12:12

rename Unevaluated to UnevaluatedConst

a705e65

b-naber force-pushed the unevaluated branch from 43ddb7f to a705e65 Compare September 23, 2022 12:27

lcnr reviewed Sep 23, 2022

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 23, 2022

bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 23, 2022

bors merged commit 9a963e3 into rust-lang:master Sep 23, 2022

rustbot added this to the 1.66.0 milestone Sep 23, 2022

This was referenced Sep 23, 2022

make const_err a hard error #102091

Merged

reimplement ~const Trait bounds via a fourth kind of generic param #101900

Closed

Add ConstKind::Expr #99798

Merged

lcnr added the A-const-generics Area: const generics (parameters and arguments) label Sep 23, 2022

Uh oh!

Conversation

b-naber commented Sep 20, 2022

Uh oh!

rustbot commented Sep 20, 2022

Uh oh!

lcnr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lcnr Sep 20, 2022

Choose a reason for hiding this comment

Uh oh!

bors commented Sep 20, 2022

Uh oh!

Uh oh!

lcnr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lcnr commented Sep 22, 2022

Uh oh!

bors commented Sep 22, 2022

Uh oh!

lcnr Sep 23, 2022

Choose a reason for hiding this comment

Uh oh!

lcnr commented Sep 23, 2022

Uh oh!

bors commented Sep 23, 2022

Uh oh!

bors commented Sep 23, 2022

Uh oh!

bors commented Sep 23, 2022

Uh oh!

rust-timer commented Sep 23, 2022

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Footnotes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants