Create const block DefIds in typeck instead of ast lowering by oli-obk · Pull Request #124650 · rust-lang/rust

oli-obk · 2024-05-03T09:32:34Z

this is a prerequisite for cleaning up pattern types and the range pattern HIR nodes in general. Right now they contain expressions, but they are supposed to only contain constants. In order to generate the anonymous constants lazily during typeck, we need to support generating new items with bodies during typeck in general. Transforming const blocks was the simplest change I could find to allow us to do that (everything else is much more invasive if we haven't already done it for const blocks).

rustbot · 2024-05-03T09:32:38Z

Some changes occurred in match checking

cc @Nadrieril

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Changes to the size of AST and/or HIR nodes.

cc @nnethercote

oli-obk · 2024-05-03T09:32:45Z

@bors try @rust-timer queue
w

Create const block DefIds in typeck instead of ast lowering r? `@ghost` this is a prerequisite for cleaning up pattern types and the range pattern HIR nodes in general. Right now they contain expressions, but they are supposed to only contain constants. In order to generate the anonymous constants lazily during typeck, we need to support generating new items with bodies during typeck in general. Transforming const blocks was the simplest change I could find to allow us to do that (everything else is much more invasive if we haven't already done it for const blocks).

bors · 2024-05-03T09:33:55Z

⌛ Trying commit edb59c9 with merge eeec347...

bors · 2024-05-03T09:47:50Z

💔 Test failed - checks-actions

bors · 2024-05-04T03:32:30Z

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

…-params, r=spastorino Pattern types: Prohibit generic args on const params Addresses https://github.com/rust-lang/rust/pull/123689/files#r1562676629. NB: Technically speaking, *not* prohibiting generics args on const params is not a bug as `pattern_types` is an *internal* feature and as such any uncaught misuses of it are considered to be the fault of the user. However, permitting this makes me slightly uncomfortable esp. since we might want to make pattern types available to the public at some point and I don't want this oversight to be able to slip into the language (for comparison, ICEs triggered by the use of internal features are like super fine). Furthermore, this is an ad hoc fix. A more general fix would be changing the representation of the pattern part of pattern types in such a way that it can reuse preexisting lowering routines for exprs / anon consts. See also this [Zulip discussion](https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/pattern.20type.20HIR.20nodes/near/432410768) and rust-lang#124650. Also note that we currently don't properly typeck the pattern of pat tys. This however is out of scope for this PR. cc `@oli-obk` r? `@spastorino` as discussed

rustbot · 2024-05-22T10:30:41Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

oli-obk · 2024-05-22T10:43:17Z

@bors try @rust-timer queue

bors · 2024-05-22T10:44:28Z

⌛ Trying commit 8d59f8b with merge bbdee60...

Create const block DefIds in typeck instead of ast lowering r? `@ghost` this is a prerequisite for cleaning up pattern types and the range pattern HIR nodes in general. Right now they contain expressions, but they are supposed to only contain constants. In order to generate the anonymous constants lazily during typeck, we need to support generating new items with bodies during typeck in general. Transforming const blocks was the simplest change I could find to allow us to do that (everything else is much more invasive if we haven't already done it for const blocks).

bors · 2024-05-22T12:18:25Z

☀️ Try build successful - checks-actions
Build commit: bbdee60 (bbdee6062d573cdaa8edbd300234a18adee59de9)

rust-timer · 2024-05-22T15:47:49Z

Finished benchmarking commit (bbdee60): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

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-perf +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)	1.4%	[0.2%, 4.6%]	21
Regressions ❌ (secondary)	3.4%	[0.7%, 8.5%]	5
Improvements ✅ (primary)	-0.5%	[-0.6%, -0.4%]	6
Improvements ✅ (secondary)	-0.3%	[-0.3%, -0.3%]	1
All ❌✅ (primary)	1.0%	[-0.6%, 4.6%]	27

Max RSS (memory usage)

Results (primary 1.4%)

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)	1.8%	[1.2%, 3.5%]	14
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-5.0%	[-5.0%, -5.0%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.4%	[-5.0%, 3.5%]	15

Cycles

Results (primary 1.8%, secondary 4.9%)

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)	2.5%	[1.1%, 4.0%]	6
Regressions ❌ (secondary)	4.9%	[1.8%, 8.9%]	3
Improvements ✅ (primary)	-2.2%	[-2.2%, -2.2%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.8%	[-2.2%, 4.0%]	7

Binary size

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

Bootstrap: 672.666s -> 672.469s (-0.03%)
Artifact size: 315.47 MiB -> 315.47 MiB (-0.00%)

bors · 2024-05-28T13:39:47Z

📌 Commit ac7e836 has been approved by nnethercote

It is now in the queue for this repository.

bors · 2024-05-28T13:48:31Z

⌛ Testing commit ac7e836 with merge 7717a30...

bors · 2024-05-28T15:57:08Z

☀️ Test successful - checks-actions
Approved by: nnethercote
Pushing 7717a30 to master...

rust-timer · 2024-05-28T17:11:29Z

Finished benchmarking commit (7717a30): comparison URL.

Overall result: ❌✅ regressions and improvements - 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.2%	[0.2%, 0.2%]	2
Regressions ❌ (secondary)	0.2%	[0.2%, 0.2%]	3
Improvements ✅ (primary)	-0.5%	[-0.6%, -0.3%]	6
Improvements ✅ (secondary)	-0.3%	[-0.3%, -0.3%]	1
All ❌✅ (primary)	-0.3%	[-0.6%, 0.2%]	8

Max RSS (memory usage)

Results (primary 1.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)	1.1%	[0.6%, 1.6%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.1%	[0.6%, 1.6%]	2

Cycles

Results (primary -1.7%, secondary 2.6%)

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.6%	[2.6%, 2.6%]	1
Improvements ✅ (primary)	-1.7%	[-1.7%, -1.7%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-1.7%	[-1.7%, -1.7%]	1

Binary size

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

Bootstrap: 669.563s -> 669.344s (-0.03%)
Artifact size: 318.39 MiB -> 318.39 MiB (0.00%)

lqd · 2024-05-28T18:44:23Z

Tiny changes, and overall more gains than losses, probably not worth investigation effort imho.

…r-errors Reintroduce name resolution check for trying to access locals from an inline const fixesrust-lang#125676 I removed this without replacement in rust-lang#124650 without considering the consequences

Rollup merge of rust-lang#125705 - oli-obk:const_block_ice, r=compiler-errors Reintroduce name resolution check for trying to access locals from an inline const fixesrust-lang#125676 I removed this without replacement in rust-lang#124650 without considering the consequences

…r-errors Revert: create const block bodies in typeck via query feeding as per the discussion in rust-lang#125806 (comment) It was a mistake to try to shoehorn const blocks and some specific anon consts into the same box and feed them during typeck. It turned out not simplifying anything (my hope was that we could feed `type_of` to start avoiding the huge HIR matcher, but that didn't work out), but instead making a few things more fragile. reverts the const-block-specific parts of rust-lang#124650 `@bors` rollup=never had a small perf impact previously fixesrust-lang#125846 r? `@compiler-errors`

…r-errors Revert: create const block bodies in typeck via query feeding as per the discussion in rust-lang#125806 (comment) It was a mistake to try to shoehorn const blocks and some specific anon consts into the same box and feed them during typeck. It turned out not simplifying anything (my hope was that we could feed `type_of` to start avoiding the huge HIR matcher, but that didn't work out), but instead making a few things more fragile. reverts the const-block-specific parts of rust-lang#124650 ``@bors`` rollup=never had a small perf impact previously fixesrust-lang#125846 r? ``@compiler-errors``

pnkfelix · 2024-06-04T13:34:06Z

as previously noted by @lqd : "Tiny changes, and overall more gains than losses, probably not worth investigation effort imho."
marking as triaged

@rustbot label: +perf-regression-triaged

…r-errors Revert: create const block bodies in typeck via query feeding as per the discussion in rust-lang#125806 (comment) It was a mistake to try to shoehorn const blocks and some specific anon consts into the same box and feed them during typeck. It turned out not simplifying anything (my hope was that we could feed `type_of` to start avoiding the huge HIR matcher, but that didn't work out), but instead making a few things more fragile. reverts the const-block-specific parts of rust-lang#124650 ```@bors``` rollup=never had a small perf impact previously fixesrust-lang#125846 r? ```@compiler-errors```

…errors Revert: create const block bodies in typeck via query feeding as per the discussion in rust-lang#125806 (comment) It was a mistake to try to shoehorn const blocks and some specific anon consts into the same box and feed them during typeck. It turned out not simplifying anything (my hope was that we could feed `type_of` to start avoiding the huge HIR matcher, but that didn't work out), but instead making a few things more fragile. reverts the const-block-specific parts of rust-lang#124650 `@bors` rollup=never had a small perf impact previously fixesrust-lang#125846 r? `@compiler-errors`

Revert: create const block bodies in typeck via query feeding as per the discussion in rust-lang/rust#125806 (comment) It was a mistake to try to shoehorn const blocks and some specific anon consts into the same box and feed them during typeck. It turned out not simplifying anything (my hope was that we could feed `type_of` to start avoiding the huge HIR matcher, but that didn't work out), but instead making a few things more fragile. reverts the const-block-specific parts of rust-lang/rust#124650 `@bors` rollup=never had a small perf impact previously fixesrust-lang/rust#125846 r? `@compiler-errors`

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 May 3, 2024

This comment has been minimized.
Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 3, 2024

oli-obk commented May 3, 2024
View reviewed changes

Comment threadcompiler/rustc_const_eval/src/const_eval/machine.rs

This comment has been minimized.
Sign in to view

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 May 3, 2024

fmease mentioned this pull request May 11, 2024
Pattern types: Prohibit generic args on const params #125015
Merged

oli-obk force-pushed the pattern_types_syntax branch from edb59c9 to 96380e0Compare May 22, 2024 10:30

oli-obk force-pushed the pattern_types_syntax branch from 96380e0 to 8d59f8bCompare May 22, 2024 10:41

This comment has been minimized.
Sign in to view

oli-obk force-pushed the pattern_types_syntax branch from 8d59f8b to 520c8e7Compare May 22, 2024 15:08

This comment has been minimized.
Sign in to view

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 May 28, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label May 28, 2024

bors merged commit 7717a30 into rust-lang:masterMay 28, 2024

rustbot added this to the 1.80.0 milestone May 28, 2024

lqd mentioned this pull request May 28, 2024
ICE: mir_build: no entry found for key#125676
Closed

oli-obk mentioned this pull request May 29, 2024
Reintroduce name resolution check for trying to access locals from an inline const #125705
Merged

oli-obk mentioned this pull request May 31, 2024
Create const blocks on the fly during typeck instead of waiting until writeback. #125806
Closed

oli-obk deleted the pattern_types_syntax branch May 31, 2024 15:02

This was referenced Jun 1, 2024
ICE: no successor#125846
Closed
ICE: cannot convert ReLateParam to a region vid#125873
Closed

oli-obk mentioned this pull request Jun 3, 2024
Revert: create const block bodies in typeck via query feeding #125918
Merged

rustbot added the perf-regression-triaged The performance regression has been triaged. label Jun 4, 2024

GrigorenkoPV mentioned this pull request Jun 19, 2024
ICE: mir_const_qualif: index out of bounds: the len is 0 but the index is 0#125837
Closed

Uh oh!

Uh oh!

Conversation

oli-obk commented May 3, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented May 3, 2024

Uh oh!

oli-obk commented May 3, 2024

Uh oh!

This comment has been minimized.

bors commented May 3, 2024

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

bors commented May 3, 2024

Uh oh!

bors commented May 4, 2024

Uh oh!

rustbot commented May 22, 2024

Uh oh!

oli-obk commented May 22, 2024

Uh oh!

This comment has been minimized.

bors commented May 22, 2024

Uh oh!

This comment has been minimized.

bors commented May 22, 2024

Uh oh!

This comment has been minimized.

This comment has been minimized.

rust-timer commented May 22, 2024

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Uh oh!

bors commented May 28, 2024

Uh oh!

bors commented May 28, 2024

Uh oh!

bors commented May 28, 2024

Uh oh!

rust-timer commented May 28, 2024

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Uh oh!

lqd commented May 28, 2024

Uh oh!

pnkfelix commented Jun 4, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

oli-obk commented May 3, 2024•
edited
Loading