Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove NtItem and NtStmt #138083

Merged
merged 3 commits into from
Mar 12, 2025
Merged

Remove NtItem and NtStmt #138083

merged 3 commits into from
Mar 12, 2025

Conversation

nnethercote
Copy link
Contributor

Another piece of #124141.

r? @petrochenkov

@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) PG-exploit-mitigations Project group: Exploit mitigations 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 Mar 6, 2025
@rustbot
Copy link
Collaborator

rustbot commented Mar 6, 2025

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

Some changes occurred in compiler/rustc_sanitizers

cc @rust-lang/project-exploit-mitigations, @rcvalle

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

@petrochenkov
Copy link
Contributor

The rustdoc overflows are probably from the extra passes that rustdoc runs to determine the full set of auto trait implementations to document.
Although I'm still not sure why is the recursion level increased by this change.
@rustbot author

@rustbot rustbot 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 Mar 6, 2025
@nnethercote
Copy link
Contributor Author

I added the cfg_attr conditions to the recursion_limit increases. I left everything else unchanged because I think it's necessary.

This involves replacing `nt_pretty_printing_compatibility_hack` with
`stream_pretty_printing_compatibility_hack`.

The handling of statements in `transcribe` is slightly different to
other nonterminal kinds, due to the lack of `from_ast` implementation
for empty statements.

Notable test changes:
- `tests/ui/proc-macro/expand-to-derive.rs`: the diff looks large but
  the only difference is the insertion of a single invisible-delimited
  group around a metavar.
This is temporarily needed for `x doc compiler` to work. They can be
removed once the `Nonterminal` is removed (rust-lang#124141).
This time when converting them to proc-macro `Group` form.
@nnethercote
Copy link
Contributor Author

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 7, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 7, 2025
Remove `NtItem` and `NtStmt`

Another piece of rust-lang#124141.

r? `@petrochenkov`
@bors
Copy link
Contributor

bors commented Mar 7, 2025

⌛ Trying commit 3378ee1 with merge 17de919...

@bors
Copy link
Contributor

bors commented Mar 7, 2025

☀️ Try build successful - checks-actions
Build commit: 17de919 (17de9196568986e11f218c0c5a9a7c6c608f1cd3)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (17de919): comparison URL.

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

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 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)
2.2% [0.2%, 3.1%] 14
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.3% [-0.4%, -0.2%] 6
All ❌✅ (primary) 2.2% [0.2%, 3.1%] 14

Max RSS (memory usage)

Results (primary -0.7%, secondary 2.2%)

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.3% [3.3%, 3.3%] 1
Regressions ❌
(secondary)
2.2% [2.2%, 2.2%] 1
Improvements ✅
(primary)
-2.7% [-3.0%, -2.3%] 2
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.7% [-3.0%, 3.3%] 3

Cycles

Results (primary 2.7%, secondary 4.0%)

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.7% [2.1%, 3.8%] 9
Regressions ❌
(secondary)
4.0% [4.0%, 4.0%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 2.7% [2.1%, 3.8%] 9

Binary size

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

Bootstrap: 765.289s -> 764.767s (-0.07%)
Artifact size: 362.07 MiB -> 362.08 MiB (0.00%)

@rustbot rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Mar 7, 2025
@petrochenkov
Copy link
Contributor

r=me unless you want to check why compiling libc is slow.

It it's due to stream_pretty_printing_compatibility_hack, then maybe a couple of tokens of look ahead will help to filter away everything that is not relevant to rental hack and avoid the Parser creation and item reparsing in most case.

@nnethercote
Copy link
Contributor Author

From what I remember, stream_pretty_printing_compatibility_hack wasn't relevant to perf. Anyway, I think it makes more sense to dig into perf once Nonterminal is fully removed. We're currently at an awkward halfway state. E.g. making TokenKind impl Copy is a perf win.

@rustbot label: +perf-regression-triaged
@bors r=petrochenkov

@bors
Copy link
Contributor

bors commented Mar 7, 2025

📌 Commit 3378ee1 has been approved by petrochenkov

It is now in the queue for this repository.

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Mar 7, 2025
@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 7, 2025
@bors
Copy link
Contributor

bors commented Mar 12, 2025

⌛ Testing commit 3378ee1 with merge aaa2d47...

@bors
Copy link
Contributor

bors commented Mar 12, 2025

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing aaa2d47 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 12, 2025
@bors bors merged commit aaa2d47 into rust-lang:master Mar 12, 2025
7 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Mar 12, 2025
Copy link

Post-merge analysis result

Test differences

  • x86_64-gnu
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_impl (line 563): [missing] -> ignore
    • compiler/rustc_mir_transform/src/lib.rs - declare_passes (line 65): [missing] -> ignore
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_foreign_mod (line 1205): [missing] -> ignore
    • compiler/rustc_parse/src/parser/expr.rs - parser::expr::CondChecker (line 3913): [missing] -> ignore
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 496): [missing] -> ignore
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 501): [missing] -> pass
    • [ui] tests/ui/macros/macro-stmt-2.rs: [missing] -> pass
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 492): [missing] -> pass
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_extern_crate (line 1149): [missing] -> ignore
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_impl (line 560): ignore -> [missing]
    • (and 7 additional testss)
  • x86_64-gnu-nopt
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_foreign_mod (line 1205): [missing] -> ignore
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 501): [missing] -> pass
    • compiler/rustc_parse/src/parser/expr.rs - parser::expr::CondChecker (line 3913): [missing] -> ignore
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 496): [missing] -> ignore
    • compiler/rustc_mir_transform/src/lib.rs - declare_passes (line 65): [missing] -> ignore
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_impl (line 563): [missing] -> ignore
    • [ui] tests/ui/macros/macro-stmt-2.rs: [missing] -> pass
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 492): [missing] -> pass
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_extern_crate (line 1149): [missing] -> ignore
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 491): pass -> [missing]
    • (and 7 additional testss)
  • x86_64-gnu-llvm-19-3
    • compiler/rustc_mir_transform/src/lib.rs - declare_passes (line 65): [missing] -> ignore
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 501): [missing] -> pass
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_impl (line 563): [missing] -> ignore
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_foreign_mod (line 1205): [missing] -> ignore
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_extern_crate (line 1149): [missing] -> ignore
    • compiler/rustc_parse/src/parser/expr.rs - parser::expr::CondChecker (line 3913): [missing] -> ignore
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 492): [missing] -> pass
    • [ui] tests/ui/macros/macro-stmt-2.rs: [missing] -> pass
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 496): [missing] -> ignore
    • compiler/rustc_mir_transform/src/lib.rs - declare_passes (line 64): ignore -> [missing]
    • (and 7 additional testss)
  • x86_64-gnu-stable
    • [ui] tests/ui/macros/macro-stmt-2.rs: [missing] -> pass
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 501): [missing] -> pass
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 496): [missing] -> ignore
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_extern_crate (line 1149): [missing] -> ignore
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_impl (line 563): [missing] -> ignore
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_foreign_mod (line 1205): [missing] -> ignore
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 492): [missing] -> pass
    • compiler/rustc_parse/src/parser/expr.rs - parser::expr::CondChecker (line 3913): [missing] -> ignore
    • compiler/rustc_mir_transform/src/lib.rs - declare_passes (line 65): [missing] -> ignore
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_foreign_mod (line 1202): ignore -> [missing]
    • (and 7 additional testss)
  • aarch64-gnu
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 492): [missing] -> pass
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_impl (line 563): [missing] -> ignore
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 496): [missing] -> ignore
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_extern_crate (line 1149): [missing] -> ignore
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_foreign_mod (line 1205): [missing] -> ignore
    • compiler/rustc_mir_transform/src/lib.rs - declare_passes (line 65): [missing] -> ignore
    • compiler/rustc_parse/src/parser/expr.rs - parser::expr::CondChecker (line 3913): [missing] -> ignore
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 501): [missing] -> pass
    • [ui] tests/ui/macros/macro-stmt-2.rs: [missing] -> pass
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_impl (line 560): ignore -> [missing]
    • (and 7 additional testss)
  • x86_64-gnu-llvm-18-3
    • compiler/rustc_mir_transform/src/lib.rs - declare_passes (line 65): [missing] -> ignore
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_impl (line 563): [missing] -> ignore
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 496): [missing] -> ignore
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 501): [missing] -> pass
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_foreign_mod (line 1205): [missing] -> ignore
    • compiler/rustc_parse/src/parser/expr.rs - parser::expr::CondChecker (line 3913): [missing] -> ignore
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_extern_crate (line 1149): [missing] -> ignore
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 492): [missing] -> pass
    • [ui] tests/ui/macros/macro-stmt-2.rs: [missing] -> pass
    • compiler/rustc_parse/src/parser/expr.rs - parser::expr::CondChecker (line 3914): ignore -> [missing]
    • (and 7 additional testss)
  • aarch64-apple
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_extern_crate (line 1149): [missing] -> ignore
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 496): [missing] -> ignore
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_impl (line 563): [missing] -> ignore
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_foreign_mod (line 1205): [missing] -> ignore
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 501): [missing] -> pass
    • compiler/rustc_parse/src/parser/expr.rs - parser::expr::CondChecker (line 3913): [missing] -> ignore
    • compiler/rustc_mir_transform/src/lib.rs - declare_passes (line 65): [missing] -> ignore
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 492): [missing] -> pass
    • [ui] tests/ui/macros/macro-stmt-2.rs: [missing] -> pass
    • compiler/rustc_parse/src/parser/expr.rs - parser::expr::CondChecker (line 3914): ignore -> [missing]
    • (and 7 additional testss)
  • i686-mingw-2
    • compiler/rustc_mir_transform/src/lib.rs - declare_passes (line 65): [missing] -> ignore
    • compiler/rustc_parse/src/parser/expr.rs - parser::expr::CondChecker (line 3913): [missing] -> ignore
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_impl (line 563): [missing] -> ignore
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 496): [missing] -> ignore
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 492): [missing] -> pass
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 501): [missing] -> pass
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_extern_crate (line 1149): [missing] -> ignore
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_foreign_mod (line 1205): [missing] -> ignore
    • compiler/rustc_mir_transform/src/lib.rs - declare_passes (line 64): ignore -> [missing]
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_impl (line 560): ignore -> [missing]
    • (and 6 additional testss)
  • i686-mingw-3
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_extern_crate (line 1149): [missing] -> ignore
    • compiler/rustc_parse/src/parser/expr.rs - parser::expr::CondChecker (line 3913): [missing] -> ignore
    • compiler/rustc_mir_transform/src/lib.rs - declare_passes (line 65): [missing] -> ignore
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 492): [missing] -> pass
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 496): [missing] -> ignore
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 501): [missing] -> pass
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_foreign_mod (line 1205): [missing] -> ignore
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_impl (line 563): [missing] -> ignore
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 495): ignore -> [missing]
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_extern_crate (line 1146): ignore -> [missing]
    • (and 6 additional testss)
  • i686-gnu-2
    • compiler/rustc_mir_transform/src/lib.rs - declare_passes (line 65): [missing] -> ignore
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 496): [missing] -> ignore
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_impl (line 563): [missing] -> ignore
    • compiler/rustc_parse/src/parser/expr.rs - parser::expr::CondChecker (line 3913): [missing] -> ignore
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_foreign_mod (line 1205): [missing] -> ignore
    • compiler/rustc_parse/src/parser/item.rs - parser::item::Parser<'a>::parse_item_extern_crate (line 1149): [missing] -> ignore
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 492): [missing] -> pass
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 501): [missing] -> pass
    • compiler/rustc_hir_typeck/src/lib.rs - TupleArgumentsFlag (line 500): pass -> [missing]
    • compiler/rustc_mir_transform/src/lib.rs - declare_passes (line 64): ignore -> [missing]
    • (and 6 additional testss)

(and 6 additional diffs)

This was referenced Mar 12, 2025
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (aaa2d47): comparison URL.

Overall result: ❌ regressions - 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)
2.2% [0.2%, 3.1%] 14
Regressions ❌
(secondary)
0.2% [0.2%, 0.2%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 2.2% [0.2%, 3.1%] 14

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

Cycles

Results (primary 2.4%, secondary -3.5%)

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.4% [1.8%, 2.9%] 8
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.5% [-4.0%, -3.0%] 3
All ❌✅ (primary) 2.4% [1.8%, 2.9%] 8

Binary size

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

Bootstrap: 779.667s -> 779.458s (-0.03%)
Artifact size: 365.24 MiB -> 365.27 MiB (0.01%)

@nnethercote nnethercote deleted the rm-NtItem-NtStmt branch March 12, 2025 22:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. PG-exploit-mitigations Project group: Exploit mitigations S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants