Skip to content

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

dianne and others added 29 commits February 10, 2025 04:08
Because of an ambiguity with const closures, the parser needs to ensure
that for a const item, the `const` keyword isn't followed by a `move` or
`static` keyword, as that would indicate a const closure:

```rust
fn main() {
  const move // ...
}
```

This check did not take raw identifiers into account, therefore being
unable to distinguish between `const move` and `const r#move`. The
latter is obviously not a const closure, so it should be allowed as a
const item.

This fixes the check in the parser to only treat `const ...` as a const
closure if it's followed by the *proper keyword*, and not a raw
identifier.

Additionally, this adds a large test that tests for all raw identifiers in
all kinds of positions, including `const`, to prevent issues like this
one from occurring again.
It's similar to the other limits, e.g. obtained via `get_limit`. So it
makes sense to handle it consistently with the other limits. We now use
`Limit`/`usize` in most places instead of `Option<usize>`, so we use
`Limit::new(usize::MAX)`/`usize::MAX` to emulate how `None` used to work.

The commit also adds `Limit::unlimited`.
Thanks to the previous commit, they no longer need to be separate.
It's always good to make `rustc_middle` smaller. `rustc_interface` is
the best destination, because it's the only crate that calls
`get_recursive_limit`.
For consistency with `recursion_limit`, `move_size_limit`, and
`type_length_limit`.
The `Map` trait is there for cases where `tcx` isn't available. This
isn't one of those cases, so it's simpler to just call through `tcx`
directly.
All the methods are unreachable, so make that clearer.
There is no need for the extra subdirectory, and this makes the `map`
module consistent with its sibling modules `nested_filter` and `place`.
The end goal is to eliminate `Map` altogether.

I added a `hir_` prefix to all of them, that seemed simplest. The
exceptions are `module_items` which became `hir_module_free_items` because
there was already a `hir_module_items`, and `items` which became
`hir_free_items` for consistency with `hir_module_free_items`.
First of all, note that `Map` has three different relevant meanings.
- The `intravisit::Map` trait.
- The `map::Map` struct.
- The `NestedFilter::Map` associated type.

The `intravisit::Map` trait is impl'd twice.
- For `!`, where the methods are all unreachable.
- For `map::Map`, which gets HIR stuff from the `TyCtxt`.

As part of getting rid of `map::Map`, this commit changes `impl
intravisit::Map for map::Map` to `impl intravisit::Map for TyCtxt`. It's
fairly straightforward except various things are renamed, because the
existing names would no longer have made sense.

- `trait intravisit::Map` becomes `trait intravisit::HirTyCtxt`, so named
  because it gets some HIR stuff from a `TyCtxt`.
- `NestedFilter::Map` assoc type becomes `NestedFilter::MaybeTyCtxt`,
  because it's always `!` or `TyCtxt`.
- `Visitor::nested_visit_map` becomes `Visitor::maybe_tcx`.

I deliberately made the new trait and associated type names different to
avoid the old `type Map: Map` situation, which I found confusing. We now
have `type MaybeTyCtxt: HirTyCtxt`.
It's a trivial wrapper around the `hir_crate` query with a small number
of uses.
…cjgillot

Start removing `rustc_middle::hir::map::Map`

`rustc_middle::hir::map::Map` is now just a low-value wrapper around `TyCtxt`. This PR starts removing it.

r? `@cjgillot`
…eril

Overhaul `rustc_middle::limits`

In particular, to make `pattern_complexity` work more like other limits, which then enables some other simplifications.

r? ``@Nadrieril``
…tion, r=Nadrieril

Pattern Migration 2024: clean up and comment

This follows up on rust-lang#136577 by moving the pattern migration logic to its own module, removing a bit of unnecessary complexity, and adding comments. Since there's quite a bit of pattern migration logic now (and potentially more in rust-lang#136496), I think it makes sense to keep it separate from THIR construction, at least as much as is convenient.

r? ``@Nadrieril``
…sDenton

Use `const_error!` when possible

Replace usages of `io::Error::new(io::ErrorKind::Variant, "constant string")` with `io::const_error!(io::ErrorKind::Variant, "constant string")` to avoid allocations when possible. Additionally, fix `&&str` error messages in SGX and missing/misplaced trailing commas in `const_error!`.
bootstrap: add more tracing to compiler/std/llvm flows

- Add more tracing to compiler/std/llvm flows.
- Two drive-by nits:
    1. Take `TargetSelection` by-value for `builder.is_builder_target()`. Noticed while adding tracing; follow-up to rust-lang#136767.
    2. Coalesce enzyme build logic into one branch.
- Document `COMPILER{,_FOR}` tracing targets for rust-lang#96176.
- No functional changes.

### Testing

You can play with the tracing locally with:

```
$ BOOTSTRAP_TRACING=bootstrap=debug ./x build library
$ BOOTSTRAP_TRACING=bootstrap=trace ./x build library
$ BOOTSTRAP_TRACING=bootstrap=trace,COMPILER=trace,COMPILER_FOR=trace ./x build library
```

### Previews

```
$ BOOTSTRAP_TRACING=bootstrap=debug ./x build library
```

![Screenshot 2025-02-15 230824](https://github.com/user-attachments/assets/c3b02b62-d52e-4c03-a00a-da0d95618989)

```
$ BOOTSTRAP_TRACING=bootstrap=trace,COMPILER=trace,COMPILER_FOR=trace ./x build library
```

![Screenshot 2025-02-15 233859](https://github.com/user-attachments/assets/842e4ece-4c26-4191-acbb-5f93e42de4dc)

r? ``@onur-ozkan`` (or reroll)
…Urgau

`invalid_from_utf8[_unchecked]`: also lint inherent methods

Addressing rust-lang#131114 (comment)

Also corrected a typo: "_an_ invalid literal", not "_a_ invalid literal".
…ompiler-errors

Fix const items not being allowed to be called `r#move` or `r#static`

Because of an ambiguity with const closures, the parser needs to ensure that for a const item, the `const` keyword isn't followed by a `move` or `static` keyword, as that would indicate a const closure:

```rust
fn main() {
  const move // ...
}
```

This check did not take raw identifiers into account, therefore being unable to distinguish between `const move` and `const r#move`. The latter is obviously not a const closure, so it should be allowed as a const item.

This fixes the check in the parser to only treat `const ...` as a const closure if it's followed by the *proper keyword*, and not a raw identifier.

Additionally, this adds a large test that tests for all raw identifiers in all kinds of positions, including `const`, to prevent issues like this one from occurring again.

fixes rust-lang#137128
@rustbot rustbot added the A-attributes Area: Attributes (`#[…]`, `#![…]`) label Feb 17, 2025
@rustbot rustbot added O-wasi Operating system: Wasi, Webassembly System Interface O-windows Operating system: Windows S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output. rollup A PR which is a rollup labels Feb 17, 2025
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=5

@bors
Copy link
Collaborator

bors commented Feb 17, 2025

📌 Commit f071099 has been approved by matthiaskrgr

It is now in the queue for this repository.

@bors 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 Feb 17, 2025
@bors
Copy link
Collaborator

bors commented Feb 17, 2025

⌛ Testing commit f071099 with merge 273465e...

@bors
Copy link
Collaborator

bors commented Feb 17, 2025

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing 273465e to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 17, 2025
@bors bors merged commit 273465e into rust-lang:master Feb 17, 2025
7 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Feb 17, 2025
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#136466 Start removing rustc_middle::hir::map::Map 24b5ecef3da30f0cb5d4d82427072f8ac80efeee (link)
#136671 Overhaul rustc_middle::limits 7159f0b7e6d9c1a4d640ee02321cdde33598efc1 (link)
#136817 Pattern Migration 2024: clean up and comment 3ec5d8bef5e0366545f6e8f15716d81d0a4ff260 (link)
#136844 Use const_error! when possible 6e5dfcce47fac3b8be892f420243b91cd6cafc3d (link)
#137080 bootstrap: add more tracing to compiler/std/llvm flows d96c7a8f45924a7e87d281bed4f3a920f234e4b9 (link)
#137101 invalid_from_utf8[_unchecked]: also lint inherent methods 9cd5a37d86e4d52fd34d2e27bf77f6eedb136b41 (link)
#137140 Fix const items not being allowed to be called r#move or … 964a73c43e3a81b9a9659d86b10fc7f3691b2605 (link)

previous master: d5eb31c934

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
Copy link
Collaborator

Finished benchmarking commit (273465e): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

  • If the regression was expected or you think it can be justified,
    please write a comment with sufficient written justification, and add
    @rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
  • If you think that you know of a way to resolve the regression, try to create
    a new PR with a fix for the regression.
  • If you do not understand the regression or you think that it is just noise,
    you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
    were already notified of this PR).

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.1% [0.1%, 0.1%] 1
Improvements ✅
(primary)
-0.3% [-0.3%, -0.3%] 2
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.3% [-0.3%, -0.3%] 2

Max RSS (memory usage)

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

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: 789.735s -> 789.915s (0.02%)
Artifact size: 350.22 MiB -> 350.21 MiB (-0.00%)

@rustbot rustbot added the perf-regression Performance regression. label Feb 17, 2025
@Kobzol
Copy link
Member

Kobzol commented Feb 18, 2025

Three tiny changes, the only regression is on an incr-unchanged run of a parser stress test, I don't think that this warrants further investigation.

@rustbot label: +perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Feb 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) A-rustc-dev-guide Area: rustc-dev-guide A-rustdoc-json Area: Rustdoc JSON backend A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. O-hermit Operating System: Hermit O-solid Operating System: SOLID O-unix Operating system: Unix-like O-wasi Operating system: Wasi, Webassembly System Interface O-windows Operating system: Windows perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. rollup A PR which is a rollup S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output.
Projects
None yet
Development

Successfully merging this pull request may close these issues.