Rollup of 8 pull requests #117228

matthiaskrgr · 2023-10-26T15:45:47Z

Successful merges:

refactor(compiler/resolve): simplify some code #116905 (refactor(compiler/resolve): simplify some code)
Add way to differentiate argument locals from other locals in Stable MIR #117095 (Add way to differentiate argument locals from other locals in Stable MIR)
Avoid unbounded O(n^2) when parsing nested type args #117143 (Avoid unbounded O(n^2) when parsing nested type args)
Minor improvements to rustc_incremental #117194 (Minor improvements to rustc_incremental)
Revert "Remove TaKO8Ki from reviewers" #117202 (Revert "Remove TaKO8Ki from reviewers")
The value of -Cinstrument-coverage= doesn't need to be Option #117207 (The value of -Cinstrument-coverage= doesn't need to be Option)
Quietly fail if an error has already occurred #117214 (Quietly fail if an error has already occurred)
Rename type flag HAS_TY_GENERATOR to HAS_TY_COROUTINE #117221 (Rename type flag HAS_TY_GENERATOR to HAS_TY_COROUTINE)

r? @ghost
@rustbot modify labels: rollup

Removes unnecessary allocates and double-sorting the same vector, makes the code a little nicer.

When encountering code like `f::<f::<f::<f::<f::<f::<f::<f::<...` with unmatched closing angle brackets, add a linear check that avoids the exponential behavior of the parse recovery mechanism. Fix rust-lang#117080.

This field allows SMIR consumers to identify which locals correspond to argument locals. It simply exposes the arg_count field from the MIR representation.

This commit hides the arg_count field in Body and instead exposes more stable and user-friendly methods to get the return and argument locals. As a result, Body instances must now be constructed using the `new` function.

This is particularly helpful for the ui tests, but also could be helpful for Stable MIR users who just want all the locals without needing to concatenate responses

I.e. in the source file where it's used.

The word internal has connotations about information that's not exposed. It's more accurate to say that the remaining locals apply only to the inner part of the function, so I'm renaming them to inner locals.

The latest locals() method in stable MIR returns slices instead of vecs. This commit also includes fixes to the existing tests that previously referenced the private locals field.

Not using this flag is identical to passing `-Cinstrument-coverage=off`, so there's no need to distinguish between `None` and `Some(Off)`.

This reverts commit 8e06b25.

…=petrochenkov refactor(compiler/resolve): simplify some code Removes unnecessary allocate and double-sorting the same vector, makes the code a little nicer.

Add way to differentiate argument locals from other locals in Stable MIR This PR resolves rust-lang/project-stable-mir#47 which request a way to differentiate argument locals in a SMIR `Body` from other locals. Specifically, this PR exposes the `arg_count` field from the MIR `Body`. However, I'm opening this as a draft PR because I think there are a few outstanding questions on how this information should be exposed and described. Namely: - Is exposing `arg_count` the best way to surface this information to SMIR users? Would it be better to leave `arg_count` as a private field and add public methods (e.g. `fn arguments(&self) -> Iter<'_, LocalDecls>`) that may use the underlying `arg_count` info from the MIR body, but expose this information to users in a more convenient form? Or is it best to stick close to the current MIR convention? - If the answer to the above point is to stick with the current MIR convention (`arg_count`), is it reasonable to also commit to sticking to the current MIR convention that the first local is always the return local, while the next `arg_count` locals are always the (in-order) argument locals? - Should `Body` in SMIR only represent function bodies (as implied by the comment I added)? That seems to be the current case in MIR, but should this restriction always be the case for SMIR? r? `@celinval` r? `@oli-obk`

Avoid unbounded O(n^2) when parsing nested type args When encountering code like `f::<f::<f::<f::<f::<f::<f::<f::<...` with unmatched closing angle brackets, add a linear check that avoids the exponential behavior of the parse recovery mechanism. Fix rust-lang#117080, fix rust-lang#115414.

…jgillot Minor improvements to `rustc_incremental` Just some things I spotted while looking at this code. r? `@cjgillot`

…-reviewers, r=Nilstrieb Revert "Remove TaKO8Ki from reviewers" ref rust-lang#116061 It's been a month since this pull request, and I now have some available time for reviews. Would it be okay to revisit it as a reviewer? This reverts commit 8e06b25. r? `@Nilstrieb`

The value of `-Cinstrument-coverage=` doesn't need to be `Option` (Extracted from rust-lang#117199, since this is a purely internal cleanup that can land independently.) Not using this flag is identical to passing `-Cinstrument-coverage=off`, so there's no need to distinguish between `None` and `Some(Off)`.

…ler-errors Quietly fail if an error has already occurred fixes rust-lang#117195

…to-COROUTINE, r=lqd Rename type flag `HAS_TY_GENERATOR` to `HAS_TY_COROUTINE` r? oli-obk

matthiaskrgr · 2023-10-26T15:46:25Z

@bors r+ rollup=never p=8

bors · 2023-10-26T15:46:27Z

📌 Commit a461de7 has been approved by matthiaskrgr

It is now in the queue for this repository.

bors · 2023-10-26T17:08:17Z

⌛ Testing commit a461de7 with merge ef5c51f...

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#116905 (refactor(compiler/resolve): simplify some code) - rust-lang#117095 (Add way to differentiate argument locals from other locals in Stable MIR) - rust-lang#117143 (Avoid unbounded O(n^2) when parsing nested type args) - rust-lang#117194 (Minor improvements to `rustc_incremental`) - rust-lang#117202 (Revert "Remove TaKO8Ki from reviewers") - rust-lang#117207 (The value of `-Cinstrument-coverage=` doesn't need to be `Option`) - rust-lang#117214 (Quietly fail if an error has already occurred) - rust-lang#117221 (Rename type flag `HAS_TY_GENERATOR` to `HAS_TY_COROUTINE`) r? `@ghost` `@rustbot` modify labels: rollup

rust-log-analyzer · 2023-10-26T17:11:00Z

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)


Sending build context to Docker daemon  425.5kB

Step 1/18 : FROM ubuntu:22.04
error parsing HTTP 429 response body: invalid character 'T' looking for beginning of value: "Too Many Requests (HAP429).\n"
DEPRECATED: The legacy builder is deprecated and will be removed in a future release.
            BuildKit is currently disabled; enable it by removing the DOCKER_BUILDKIT=0
            environment-variable.


Sending build context to Docker daemon  425.5kB

Step 1/18 : FROM ubuntu:22.04
error parsing HTTP 429 response body: invalid character 'T' looking for beginning of value: "Too Many Requests (HAP429).\n"
DEPRECATED: The legacy builder is deprecated and will be removed in a future release.
            BuildKit is currently disabled; enable it by removing the DOCKER_BUILDKIT=0
            environment-variable.


Sending build context to Docker daemon  425.5kB

Step 1/18 : FROM ubuntu:22.04
Head "https://registry-1.docker.io/v2/library/ubuntu/manifests/22.04": error parsing HTTP 429 response body: invalid character 'T' looking for beginning of value: "Too Many Requests (HAP429).\n"
DEPRECATED: The legacy builder is deprecated and will be removed in a future release.
            BuildKit is currently disabled; enable it by removing the DOCKER_BUILDKIT=0
            environment-variable.


Sending build context to Docker daemon  425.5kB

Step 1/18 : FROM ubuntu:22.04
Head "https://registry-1.docker.io/v2/library/ubuntu/manifests/22.04": error parsing HTTP 429 response body: invalid character 'T' looking for beginning of value: "Too Many Requests (HAP429).\n"
DEPRECATED: The legacy builder is deprecated and will be removed in a future release.
            BuildKit is currently disabled; enable it by removing the DOCKER_BUILDKIT=0
            environment-variable.


Sending build context to Docker daemon  425.5kB

Step 1/18 : FROM ubuntu:22.04
error parsing HTTP 429 response body: invalid character 'T' looking for beginning of value: "Too Many Requests (HAP429).\n"
##[error]Process completed with exit code 1.
Post job cleanup.

bors · 2023-10-26T17:11:48Z

💔 Test failed - checks-actions

matthiaskrgr · 2023-10-26T17:17:13Z

:(
@bors retry

bors · 2023-10-26T17:35:26Z

⌛ Testing commit a461de7 with merge 8396efe...

bors · 2023-10-26T19:29:20Z

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing 8396efe to master...

rust-timer · 2023-10-26T19:31:36Z

📌 Perf builds for each rolled up PR:

PR#	Message	Perf Build Sha
#116905	refactor(compiler/resolve): simplify some code	`95bc5aaa908d6cfd0406759546ee97d30835d756` (link)
#117095	Add way to differentiate argument locals from other locals …	`0522649e946359d9e387239a455d6030b462d5a4` (link)
#117143	Avoid unbounded O(n^2) when parsing nested type args	`eb98ead696229bd14bb065bffbf5c8c833d496a1` (link)
#117194	Minor improvements to `rustc_incremental`	`a1763b36622ae5433809bdb64c520f0e886b5375` (link)
#117202	Revert "Remove TaKO8Ki from reviewers"	`497206ca93d7e06300e139598b443cc47a490175` (link)
#117207	The value of `-Cinstrument-coverage=` doesn't need to be `O…	`4d9312510e12cbf1807ea24eb5d233626a716cba` (link)
#117214	Quietly fail if an error has already occurred	`6e70db81cfcda1cdbe32457d3da2081bf691f07b` (link)
#117221	Rename type flag `HAS_TY_GENERATOR` to `HAS_TY_COROUTINE`	`49d45403683f3ee8161137baea39385db4f85075` (link)

previous master: 698db856de

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

rust-timer · 2023-10-26T23:19:35Z

Finished benchmarking commit (8396efe): 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
Improvements ✅ (secondary)	-0.8%	[-0.8%, -0.8%]	1
All ❌✅ (primary)	-	-	0

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)	3.9%	[3.9%, 3.9%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.9%	[-1.4%, -0.4%]	6
Improvements ✅ (secondary)	-3.2%	[-3.2%, -3.2%]	1
All ❌✅ (primary)	-0.2%	[-1.4%, 3.9%]	7

Cycles

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

Binary size

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

Bootstrap: 634.359s -> 635.153s (0.13%)
Artifact size: 304.48 MiB -> 304.47 MiB (-0.01%)

NeptuneUltra101 · 2024-01-10T02:02:25Z

compiler/rustc_resolve/src/check_unused.rs

> ``

Fenex and others added 29 commits October 19, 2023 04:45

refactor(compiler/resolve): simplify some code

e68edb8

Removes unnecessary allocates and double-sorting the same vector, makes the code a little nicer.

mv tests

855444e

Avoid unbounded O(n^2) when parsing nested type args

2dec1bc

When encountering code like `f::<f::<f::<f::<f::<f::<f::<f::<...` with unmatched closing angle brackets, add a linear check that avoids the exponential behavior of the parse recovery mechanism. Fix rust-lang#117080.

Add arg_count field to Body in Stable MIR

e4c41b0

This field allows SMIR consumers to identify which locals correspond to argument locals. It simply exposes the arg_count field from the MIR representation.

Replace arg_count in public API with return/arg getters

93d1b3e

This commit hides the arg_count field in Body and instead exposes more stable and user-friendly methods to get the return and argument locals. As a result, Body instances must now be constructed using the `new` function.

Add public API to retrieve internal locals

f4d80a5

Make locals field private

372c533

Add a public API to get all body locals

39b293f

This is particularly helpful for the ui tests, but also could be helpful for Stable MIR users who just want all the locals without needing to concatenate responses

Remove unused never_type feature.

72e8690

Reduce exposure of three functions used only within rustc_incremental.

ca29c27

Move a use to a more sensible spot.

8da1b33

I.e. in the source file where it's used.

Tiny comment fixes.

3cf2a74

Reduce some function exposure.

e0c990e

Rename internal_locals to inner_locals

fe4dfb8

The word internal has connotations about information that's not exposed. It's more accurate to say that the remaining locals apply only to the inner part of the function, so I'm renaming them to inner locals.

Update Place and Operand to take slices

4b23bd4

The latest locals() method in stable MIR returns slices instead of vecs. This commit also includes fixes to the existing tests that previously referenced the private locals field.

Add test for smir locals

bac7d5b

The value of -Cinstrument-coverage= doesn't need to be Option

9f5fc02

Not using this flag is identical to passing `-Cinstrument-coverage=off`, so there's no need to distinguish between `None` and `Some(Off)`.

Revert "Remove TaKO8Ki from reviewers"

ab7f64c

This reverts commit 8e06b25.

Use two slice expressions to save on an offset repetition

d55487d

Quietly fail if an error has already occurred

d572729

Replace type flag HAS_TY_GENERATOR with HAS_TY_COROUTINE

b1b1458

Rollup merge of rust-lang#116905 - Fenex:refactor/compiler/resolve, r…

17fb2f4

…=petrochenkov refactor(compiler/resolve): simplify some code Removes unnecessary allocate and double-sorting the same vector, makes the code a little nicer.

Rollup merge of rust-lang#117194 - nnethercote:rustc_incremental, r=c…

577026c

…jgillot Minor improvements to `rustc_incremental` Just some things I spotted while looking at this code. r? `@cjgillot`

Rollup merge of rust-lang#117214 - oli-obk:error_shenanigans, r=compi…

70a4678

…ler-errors Quietly fail if an error has already occurred fixes rust-lang#117195

Rollup merge of rust-lang#117221 - fmease:TypeFlags-HAS_TY_GENERATOR-…

a461de7

…to-COROUTINE, r=lqd Rename type flag `HAS_TY_GENERATOR` to `HAS_TY_COROUTINE` r? oli-obk

rustbot added the A-meta Area: Issues about the rust-lang/rust repository. label Oct 26, 2023

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. rollup A PR which is a rollup labels Oct 26, 2023

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 Oct 26, 2023

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 26, 2023

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 Oct 26, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 26, 2023

bors merged commit 8396efe into rust-lang:master Oct 26, 2023
12 checks passed

rustbot added this to the 1.75.0 milestone Oct 26, 2023

bors mentioned this pull request Oct 26, 2023

ignore implied bounds with placeholders #112422

Merged

github-actions bot mentioned this pull request Oct 31, 2023

Automated pull from upstream master ferrocene/ferrocene#74

Merged

matthiaskrgr deleted the rollup-23zzepv branch March 16, 2024 18:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rollup of 8 pull requests #117228

Rollup of 8 pull requests #117228

matthiaskrgr commented Oct 26, 2023

matthiaskrgr commented Oct 26, 2023

bors commented Oct 26, 2023

bors commented Oct 26, 2023

rust-log-analyzer commented Oct 26, 2023

bors commented Oct 26, 2023

matthiaskrgr commented Oct 26, 2023

bors commented Oct 26, 2023

bors commented Oct 26, 2023

rust-timer commented Oct 26, 2023

rust-timer commented Oct 26, 2023

NeptuneUltra101 commented Jan 10, 2024

Rollup of 8 pull requests #117228

Rollup of 8 pull requests #117228

Conversation

matthiaskrgr commented Oct 26, 2023

matthiaskrgr commented Oct 26, 2023

bors commented Oct 26, 2023

bors commented Oct 26, 2023

rust-log-analyzer commented Oct 26, 2023

bors commented Oct 26, 2023

matthiaskrgr commented Oct 26, 2023

bors commented Oct 26, 2023

bors commented Oct 26, 2023

rust-timer commented Oct 26, 2023

rust-timer commented Oct 26, 2023

Overall result: ✅ improvements - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

NeptuneUltra101 commented Jan 10, 2024