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

Subtree sync for rustc_codegen_cranelift #119470

Merged
merged 21 commits into from
Dec 31, 2023

Conversation

bjorn3
Copy link
Member

@bjorn3 bjorn3 commented Dec 31, 2023

The main highlight this time is a fix for rust-lang/rustc_codegen_cranelift#1437.

r? @ghost

@rustbot label +A-codegen +A-cranelift +T-compiler

nnethercote and others added 21 commits December 24, 2023 08:05
Also add some `dcx` methods to types that wrap `TyCtxt`, for easier
access.
…ion, r=compiler-errors

Remove `DiagCtxt` API duplication

`DiagCtxt` defines the internal API for creating and emitting diagnostics: methods like `struct_err`, `struct_span_warn`, `note`, `create_fatal`, `emit_bug`. There are over 50 methods.

Some of these methods are then duplicated across several other types: `Session`, `ParseSess`, `Parser`, `ExtCtxt`, and `MirBorrowckCtxt`. `Session` duplicates the most, though half the ones it does are unused. Each duplicated method just calls forward to the corresponding method in `DiagCtxt`. So this duplication exists to (in the best case) shorten chains like `ecx.tcx.sess.parse_sess.dcx.emit_err()` to `ecx.emit_err()`.

This API duplication is ugly and has been bugging me for a while. And it's inconsistent: there's no real logic about which methods are duplicated, and the use of `#[rustc_lint_diagnostic]` and `#[track_caller]` attributes vary across the duplicates.

This PR removes the duplicated API methods and makes all diagnostic creation and emission go through `DiagCtxt`. It also adds `dcx` getter methods to several types to shorten chains. This approach scales *much* better than API duplication; indeed, the PR adds `dcx()` to numerous types that didn't have API duplication: `TyCtxt`, `LoweringCtxt`, `ConstCx`, `FnCtxt`, `TypeErrCtxt`, `InferCtxt`, `CrateLoader`, `CheckAttrVisitor`, and `Resolver`. These result in a lot of changes from `foo.tcx.sess.emit_err()` to `foo.dcx().emit_err()`. (You could do this with more types, but it gets into diminishing returns territory for types that don't emit many diagnostics.)

After all these changes, some call sites are more verbose, some are less verbose, and many are the same. The total number of lines is reduced, mostly because of the removed API duplication. And consistency is increased, because calls to `emit_err` and friends are always preceded with `.dcx()` or `.dcx`.

r? `@compiler-errors`
Slightly reduce CI runtime and reduce log verbosity
@rustbot 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. has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 31, 2023
@rustbot

This comment was marked as resolved.

@rustbot rustbot added A-codegen Area: Code generation A-cranelift Things relevant to the [future] cranelift backend labels Dec 31, 2023
@bjorn3 bjorn3 removed A-codegen Area: Code generation A-cranelift Things relevant to the [future] cranelift backend labels Dec 31, 2023
@bjorn3 bjorn3 removed the has-merge-commits PR has merge commits, merge with caution. label Dec 31, 2023
@bjorn3
Copy link
Member Author

bjorn3 commented Dec 31, 2023

@bors r+ subtree sync

@bors
Copy link
Contributor

bors commented Dec 31, 2023

📌 Commit d1d134e has been approved by bjorn3

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. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 31, 2023
@bors
Copy link
Contributor

bors commented Dec 31, 2023

⌛ Testing commit d1d134e with merge 1c20462...

@bors
Copy link
Contributor

bors commented Dec 31, 2023

☀️ Test successful - checks-actions
Approved by: bjorn3
Pushing 1c20462 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 31, 2023
@bors bors merged commit 1c20462 into rust-lang:master Dec 31, 2023
12 checks passed
@rustbot rustbot added this to the 1.77.0 milestone Dec 31, 2023
@bjorn3 bjorn3 deleted the sync_cg_clif-2023-12-31 branch December 31, 2023 16:08
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (1c20462): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

Max RSS (memory usage)

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

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: 667.255s -> 666.618s (-0.10%)
Artifact size: 311.75 MiB -> 311.73 MiB (-0.01%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. 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