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

Replace all &DiagCtxt with a DiagCtxtHandle<'_> wrapper type #126623

Merged
merged 4 commits into from
Jun 18, 2024

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Jun 18, 2024

r? @davidtwco

This paves the way for tracking more state (e.g. error tainting) in the diagnostic context handle

Basically I will add a field to the DiagCtxtHandle that refers back to the InferCtxt's (and others) Option<ErrorHandled>, allowing us to immediately taint these contexts when emitting an error and not needing manual tainting anymore (which is easy to forget and we don't do in general anyway)

@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jun 18, 2024
@rustbot
Copy link
Collaborator

rustbot commented Jun 18, 2024

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

rustc_macros::diagnostics was changed

cc @davidtwco, @compiler-errors, @TaKO8Ki

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

@oli-obk oli-obk changed the title Do not count errors Replace all &DiagCtxt with a DiagCtxtHandle<'_> wrapper type Jun 18, 2024
@rust-log-analyzer

This comment has been minimized.

@rustbot
Copy link
Collaborator

rustbot commented Jun 18, 2024

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred in match checking

cc @Nadrieril

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Jun 18, 2024

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

@davidtwco
Copy link
Member

r=me once CI passes

@rust-log-analyzer

This comment has been minimized.

This paves the way for tracking more state (e.g. error tainting) in the diagnostic context handle
@oli-obk
Copy link
Contributor Author

oli-obk commented Jun 18, 2024

@bors r=davidtwco p=1 bitrotty

@bors
Copy link
Contributor

bors commented Jun 18, 2024

📌 Commit 3f34196 has been approved by davidtwco

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 Jun 18, 2024
@jieyouxu
Copy link
Member

@bors p=5 (very bitrotty)

@bors
Copy link
Contributor

bors commented Jun 18, 2024

⌛ Testing commit 3f34196 with merge dd104ef...

@bors
Copy link
Contributor

bors commented Jun 18, 2024

☀️ Test successful - checks-actions
Approved by: davidtwco
Pushing dd104ef to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 18, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (dd104ef): comparison URL.

Overall result: ❌ regressions - 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.3% [0.3%, 0.4%] 5
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary 0.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)
2.3% [2.1%, 2.4%] 2
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-4.2% [-4.2%, -4.2%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.1% [-4.2%, 2.4%] 3

Cycles

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

Binary size

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

Bootstrap: 671.639s -> 671.341s (-0.04%)
Artifact size: 320.44 MiB -> 320.42 MiB (-0.01%)

celinval added a commit to celinval/kani-dev that referenced this pull request Jun 21, 2024
celinval added a commit to model-checking/kani that referenced this pull request Jun 21, 2024
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Jun 24, 2024
…piler-errors

Split the lifetimes of `MirBorrowckCtxt`

These lifetimes are sometimes too general and will link things together that are independent. These are a blocker for actually finishing tracking more state (e.g. error tainting) in the diagnostic context handle, and I'd rather land it in its own PR instead of together with functional changes.

Also changes a bunch of named lifetimes to `'_` where they were irrelevant

follow-up to rust-lang#126623
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 24, 2024
Rollup merge of rust-lang#126911 - oli-obk:do_not_count_errors, r=compiler-errors

Split the lifetimes of `MirBorrowckCtxt`

These lifetimes are sometimes too general and will link things together that are independent. These are a blocker for actually finishing tracking more state (e.g. error tainting) in the diagnostic context handle, and I'd rather land it in its own PR instead of together with functional changes.

Also changes a bunch of named lifetimes to `'_` where they were irrelevant

follow-up to rust-lang#126623
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 13, 2024
…rrors

Add set_dcx to ParseSess

After [this](rust-lang#126623) PR was merged, it is no longer possible to inject one's own `Emitter` in the way [described in the Compiler Development Guide](https://rustc-dev-guide.rust-lang.org/rustc-driver-getting-diagnostics.html). The reason is that the `dcx` field in `ParseSess` is no longer public, so it is not possible to update the `dcx` field with a `DiagCtxt` that contains one's own `Emitter` in the `psess_created` callback in `rustc_interface::Config`. The only way I have found to insert my own `DiagCtxt` is by creating an entirely new `ParseSess` and replacing the old one. This is not a good solution as the original `ParseSess` contains fields I would like to keep. (In my case the problem is that I lose the `cfg` and `check-cfg` fields of the original.)

The solution proposed in this PR is to add a `set_dcx` method to `ParseSess`. Per my limited understanding of the rustc codebase this should be fine as `set_dcx` requires a mutable reference to `ParseSess`, which is as far as I know only available in the `psess_created` callback (outside of `rustc_interface::run_compiler`).

If this PR is accepted, I will create a new PR to update the aforementioned example in the Compiler Development Guide.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Sep 13, 2024
Rollup merge of rust-lang#130299 - vetleras:add_set_dcx, r=compiler-errors

Add set_dcx to ParseSess

After [this](rust-lang#126623) PR was merged, it is no longer possible to inject one's own `Emitter` in the way [described in the Compiler Development Guide](https://rustc-dev-guide.rust-lang.org/rustc-driver-getting-diagnostics.html). The reason is that the `dcx` field in `ParseSess` is no longer public, so it is not possible to update the `dcx` field with a `DiagCtxt` that contains one's own `Emitter` in the `psess_created` callback in `rustc_interface::Config`. The only way I have found to insert my own `DiagCtxt` is by creating an entirely new `ParseSess` and replacing the old one. This is not a good solution as the original `ParseSess` contains fields I would like to keep. (In my case the problem is that I lose the `cfg` and `check-cfg` fields of the original.)

The solution proposed in this PR is to add a `set_dcx` method to `ParseSess`. Per my limited understanding of the rustc codebase this should be fine as `set_dcx` requires a mutable reference to `ParseSess`, which is as far as I know only available in the `psess_created` callback (outside of `rustc_interface::run_compiler`).

If this PR is accepted, I will create a new PR to update the aforementioned example in the Compiler Development Guide.
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Sep 14, 2024
Add set_dcx to ParseSess

After [this](rust-lang/rust#126623) PR was merged, it is no longer possible to inject one's own `Emitter` in the way [described in the Compiler Development Guide](https://rustc-dev-guide.rust-lang.org/rustc-driver-getting-diagnostics.html). The reason is that the `dcx` field in `ParseSess` is no longer public, so it is not possible to update the `dcx` field with a `DiagCtxt` that contains one's own `Emitter` in the `psess_created` callback in `rustc_interface::Config`. The only way I have found to insert my own `DiagCtxt` is by creating an entirely new `ParseSess` and replacing the old one. This is not a good solution as the original `ParseSess` contains fields I would like to keep. (In my case the problem is that I lose the `cfg` and `check-cfg` fields of the original.)

The solution proposed in this PR is to add a `set_dcx` method to `ParseSess`. Per my limited understanding of the rustc codebase this should be fine as `set_dcx` requires a mutable reference to `ParseSess`, which is as far as I know only available in the `psess_created` callback (outside of `rustc_interface::run_compiler`).

If this PR is accepted, I will create a new PR to update the aforementioned example in the Compiler Development Guide.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

7 participants