const_generics: correctly deal with bound variables #98900

lcnr · 2022-07-04T17:20:07Z

removes the hack in resolve which was needed because we evaluated constants without caring about their bound variables.

Each commit should be fairly self-contained, even if they build on each other

r? @jackh726

rustbot · 2022-07-04T17:20:10Z

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

lcnr · 2022-07-04T17:20:20Z

@bors try @rust-timer queue

rust-timer · 2022-07-04T17:20:21Z

Awaiting bors try build completion.

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

bors · 2022-07-04T17:20:28Z

⌛ Trying commit 70f11da4fe0c07822290824ae21fa266d9e19f5e with merge 05fe25a300b6473d232e4b32b3f240dbba9f3c3f...

RalfJung · 2022-07-04T18:03:48Z

compiler/rustc_const_eval/src/interpret/intrinsics.rs

            | ty::Placeholder(_)
            | ty::Infer(_) => throw_inval!(TooGeneric),
+            ty::Bound(_, _) => bug!("bound ty during ctfe"),


Curious to have this ICE only for variant_count?
Though this might be the only exhaustive type match in the interpreter 🤷 it just won't give a lot of coverage.

^^ yeah, I also thought that we would have this in more places. ctfe mostly doesn't care about ty::Const. Stuff like type name looks into types, so there bound stuff is allowed, e.g. for for<'a> fn(&'a ()) 'ashould be bound.

The only other place I found was const_to_op which I've also changed

bors · 2022-07-04T18:56:34Z

☀️ Try build successful - checks-actions
Build commit: 05fe25a300b6473d232e4b32b3f240dbba9f3c3f (05fe25a300b6473d232e4b32b3f240dbba9f3c3f)

rust-timer · 2022-07-04T18:56:35Z

Queued 05fe25a300b6473d232e4b32b3f240dbba9f3c3f with parent d2074cb, future comparison URL.

rust-timer · 2022-07-05T08:24:22Z

Finished benchmarking commit (05fe25a300b6473d232e4b32b3f240dbba9f3c3f): comparison url.

Instruction count

Primary benchmarks: 😿 relevant regressions found
Secondary benchmarks: mixed results

	mean¹	max	count²
Regressions 😿 (primary)	0.7%	1.0%	9
Regressions 😿 (secondary)	1.2%	2.4%	14
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	-0.7%	-0.9%	6
All 😿🎉 (primary)	0.7%	1.0%	9

Max RSS (memory usage)

Results

Primary benchmarks: 🎉 relevant improvement found
Secondary benchmarks: no relevant changes found

	mean¹	max	count²
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	-0.2%	-0.2%	1
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	-0.2%	-0.2%	1

Cycles

Results

Primary benchmarks: no relevant changes found
Secondary benchmarks: 😿 relevant regressions found

	mean¹	max	count²
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	4.0%	5.6%	9
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	-2.1%	-2.1%	1
All 😿🎉 (primary)	N/A	N/A	0

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf +perf-regression

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

lcnr · 2022-07-05T15:22:55Z

@bors try @rust-timer queue

rust-timer · 2022-07-05T15:22:57Z

Awaiting bors try build completion.

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

bors · 2022-07-05T15:23:05Z

⌛ Trying commit 0632724d09aedd7b2942590ff73b6c0c2b7b4465 with merge 8fdfdbe7af0ae6247b009133b3477c4d4c75ab42...

lcnr · 2022-07-05T15:23:38Z

@bors try @rust-timer queue

lcnr · 2022-07-05T15:24:41Z

@bors clean

lcnr · 2022-07-05T15:24:58Z

@bors try @rust-timer queue

rust-timer · 2022-07-05T15:25:02Z

Awaiting bors try build completion.

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

bors · 2022-07-05T15:25:09Z

⌛ Trying commit 2d7a649be740f9819be48b87cbbed9842b6f4ec4 with merge 3553e445517920c619c2dae2a58199d8276c96ea...

bors · 2022-07-05T16:55:18Z

☀️ Try build successful - checks-actions
Build commit: 3553e445517920c619c2dae2a58199d8276c96ea (3553e445517920c619c2dae2a58199d8276c96ea)

rust-timer · 2022-07-05T16:55:20Z

Queued 3553e445517920c619c2dae2a58199d8276c96ea with parent 54f79ba, future comparison URL.

rust-timer · 2022-07-05T20:37:58Z

Finished benchmarking commit (3553e445517920c619c2dae2a58199d8276c96ea): comparison url.

Instruction count

Primary benchmarks: 🎉 relevant improvements found
Secondary benchmarks: 🎉 relevant improvements found

	mean¹	max	count²
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	-0.6%	-1.0%	12
Improvements 🎉 (secondary)	-1.2%	-2.3%	18
All 😿🎉 (primary)	-0.6%	-1.0%	12

Max RSS (memory usage)

Results

Primary benchmarks: no relevant changes found
Secondary benchmarks: 🎉 relevant improvements found

	mean¹	max	count²
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	-3.5%	-4.8%	2
All 😿🎉 (primary)	N/A	N/A	0

Cycles

Results

Primary benchmarks: no relevant changes found
Secondary benchmarks: 🎉 relevant improvements found

	mean¹	max	count²
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	-2.4%	-2.9%	4
All 😿🎉 (primary)	N/A	N/A	0

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

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

jackh726

Comment needed then r=me

jackh726 · 2022-09-08T00:50:20Z

compiler/rustc_trait_selection/src/traits/project.rs

@@ -637,6 +642,29 @@ pub struct BoundVarReplacer<'me, 'tcx> {
    universe_indices: &'me mut Vec<Option<ty::UniverseIndex>>,
 }

+pub fn with_replaced_escaping_bound_vars<'a, 'tcx, T: TypeFoldable<'tcx>, R: TypeFoldable<'tcx>>(


Can you add a comment and maybe a FIXME that we should be more careful to not leak placeholders?

lcnr · 2022-09-08T09:39:27Z

@bors r=jackh726 rollup=never

bors · 2022-09-08T09:39:33Z

📌 Commit 5824746aa28d906f2072d7b9c2acd4aca373be5b has been approved by jackh726

It is now in the queue for this repository.

lcnr · 2022-09-08T09:41:18Z

@bors r=jackh726 rollup=never

bors · 2022-09-08T09:41:20Z

📌 Commit e666032 has been approved by jackh726

It is now in the queue for this repository.

bors · 2022-09-08T10:30:05Z

⌛ Testing commit e666032 with merge ccb5595...

bors · 2022-09-08T12:59:15Z

☀️ Test successful - checks-actions
Approved by: jackh726
Pushing ccb5595 to master...

rust-timer · 2022-09-08T14:33:07Z

Finished benchmarking commit (ccb5595): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

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
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.9%	[-1.3%, -0.8%]	8
Improvements ✅ (secondary)	-3.3%	[-3.8%, -2.9%]	6
All ❌✅ (primary)	-0.9%	[-1.3%, -0.8%]	8

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.0%	[0.7%, 3.1%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.2%	[-2.2%, -2.2%]	1
All ❌✅ (primary)	-	-	0

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)	2.5%	[0.5%, 5.0%]	5
Improvements ✅ (primary)	-3.1%	[-3.1%, -3.1%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-3.1%	[-3.1%, -3.1%]	1

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

remove bound var hack in `resolve` somehow dropped that change from rust-lang#98900. r? `@jackh726`

riking · 2022-10-19T22:52:24Z

This PR is being blamed for a regression in #103243 .

rust-highfive assigned jackh726 Jul 4, 2022

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 4, 2022

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

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 4, 2022

RalfJung reviewed Jul 4, 2022

View reviewed changes

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jul 5, 2022

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 5, 2022

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 5, 2022

rustbot removed S-waiting-on-perf Status: Waiting on a perf run to be completed. perf-regression Performance regression. labels Aug 19, 2022

lcnr removed the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Aug 19, 2022

jackh726 requested changes Sep 8, 2022

View reviewed changes

jackh726 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 8, 2022

lcnr added 2 commits September 8, 2022 11:14

generalize: no need to cache errors

060f3e0

stop evaluating constants in Relate

01adb7e

lcnr force-pushed the region-stuff branch from 40a5e95 to 5824746 Compare September 8, 2022 09:34

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 8, 2022

lcnr added 2 commits September 8, 2022 11:41

don't evaluate with escaping bound vars

d15b00a

bound variables during ctfe are a bug

e666032

lcnr force-pushed the region-stuff branch from 5824746 to e666032 Compare September 8, 2022 09:41

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

bors merged commit ccb5595 into rust-lang:master Sep 8, 2022

rustbot added this to the 1.65.0 milestone Sep 8, 2022

lcnr deleted the region-stuff branch September 8, 2022 13:00

lcnr mentioned this pull request Sep 8, 2022

remove bound var hack in resolve #101578

Merged

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Sep 10, 2022

Rollup merge of rust-lang#101578 - lcnr:resolve-hack, r=jackh726

2386ed9

remove bound var hack in `resolve` somehow dropped that change from rust-lang#98900. r? `@jackh726`

boulanlo mentioned this pull request Oct 19, 2022

regression: associated consts cannot be referenced in patterns #103242

Closed

compiler-errors mentioned this pull request Oct 19, 2022

regression: trait bound not satisfied #103243

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

const_generics: correctly deal with bound variables #98900

const_generics: correctly deal with bound variables #98900

lcnr commented Jul 4, 2022 •

edited

Loading

rustbot commented Jul 4, 2022

lcnr commented Jul 4, 2022

rust-timer commented Jul 4, 2022

bors commented Jul 4, 2022

RalfJung Jul 4, 2022 •

edited

Loading

lcnr Jul 4, 2022

bors commented Jul 4, 2022

rust-timer commented Jul 4, 2022

rust-timer commented Jul 5, 2022

lcnr commented Jul 5, 2022

rust-timer commented Jul 5, 2022

bors commented Jul 5, 2022

lcnr commented Jul 5, 2022

lcnr commented Jul 5, 2022

lcnr commented Jul 5, 2022

rust-timer commented Jul 5, 2022

bors commented Jul 5, 2022

bors commented Jul 5, 2022

rust-timer commented Jul 5, 2022

rust-timer commented Jul 5, 2022

jackh726 left a comment

jackh726 Sep 8, 2022

lcnr commented Sep 8, 2022

bors commented Sep 8, 2022

lcnr commented Sep 8, 2022

bors commented Sep 8, 2022

bors commented Sep 8, 2022

bors commented Sep 8, 2022

rust-timer commented Sep 8, 2022

riking commented Oct 19, 2022

const_generics: correctly deal with bound variables #98900

const_generics: correctly deal with bound variables #98900

Conversation

lcnr commented Jul 4, 2022 • edited Loading

rustbot commented Jul 4, 2022

lcnr commented Jul 4, 2022

rust-timer commented Jul 4, 2022

bors commented Jul 4, 2022

RalfJung Jul 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bors commented Jul 4, 2022

rust-timer commented Jul 4, 2022

rust-timer commented Jul 5, 2022

Footnotes

lcnr commented Jul 5, 2022

rust-timer commented Jul 5, 2022

bors commented Jul 5, 2022

lcnr commented Jul 5, 2022

lcnr commented Jul 5, 2022

lcnr commented Jul 5, 2022

rust-timer commented Jul 5, 2022

bors commented Jul 5, 2022

bors commented Jul 5, 2022

rust-timer commented Jul 5, 2022

rust-timer commented Jul 5, 2022

Footnotes

jackh726 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lcnr commented Sep 8, 2022

bors commented Sep 8, 2022

lcnr commented Sep 8, 2022

bors commented Sep 8, 2022

bors commented Sep 8, 2022

bors commented Sep 8, 2022

rust-timer commented Sep 8, 2022

Overall result: ✅ improvements - no action needed

Footnotes

riking commented Oct 19, 2022

lcnr commented Jul 4, 2022 •

edited

Loading

RalfJung Jul 4, 2022 •

edited

Loading