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.

Sign up for GitHub

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

Jump to bottom

Diagnostic API fixes #119751

Merged
merged 7 commits into from
Jan 10, 2024
Merged

Diagnostic API fixes #119751

merged 7 commits into from
Jan 10, 2024

Conversation

nnethercote
Copy link
Contributor

Some improvements to diagnostic APIs: improve some naming, use shortcuts in more places, and add a couple of missing methods.

r? @compiler-errors

@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) 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 Jan 8, 2024
@rustbot
Copy link
Collaborator

rustbot commented Jan 8, 2024

Type relation code was changed

cc @compiler-errors, @lcnr

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

The Miri subtree was changed

cc @rust-lang/miri

Some changes might have occurred in exhaustiveness checking

cc @Nadrieril

Some changes occurred in const_evaluatable.rs

cc @BoxyUwU

@nnethercote
Copy link
Contributor Author

@oli-obk: The Rename consuming chaining methods on DiagnosticBuilder commit renames the foo_mv methods as with_foo, because that's an established naming convention for such methods.

@bors
Copy link
Contributor

bors commented Jan 9, 2024

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

@nnethercote
Fix incorrect comment.
99b1b0f
@nnethercote
Rename struct_span_err! as struct_span_code_err!.
4864cb8 
Because it takes an error code after the span. This avoids the confusing
overlap with the `DiagCtxt::struct_span_err` method, which doesn't take
an error code.
@nnethercote
Shorten some error invocations.
ff40ad4 
- `struct_foo` + `emit` -> `foo`
- `create_foo` + `emit` -> `emit_foo`

I have made recent commits in other PRs that have removed some of these
shortcuts for combinations with few uses, e.g.
`struct_span_err_with_code`. But for the remaining combinations that
have high levels of use, we might as well use them wherever possible.
@nnethercote
Rename {create,emit}_warning as {create,emit}_warn.
3c4f1d8 
For consistency with `warn`/`struct_warn`, and also `{create,emit}_err`,
all of which use an abbreviated form.
@nnethercote
Add DiagCtxt::delayed_bug.
2ea7a37 
We have `span_delayed_bug` and often pass it a `DUMMY_SP`. This commit
adds `delayed_bug`, which matches pairs like `err`/`span_err` and
`warn`/`span_warn`.
@nnethercote
Rename consuming chaining methods on DiagnosticBuilder.
ed76b0b 
In rust-lang#119606 I added them and used a `_mv` suffix, but that wasn't great.

A `with_` prefix has three different existing uses.
- Constructors, e.g. `Vec::with_capacity`.
- Wrappers that provide an environment to execute some code, e.g.
  `with_session_globals`.
- Consuming chaining methods, e.g. `Span::with_{lo,hi,ctxt}`.

The third case is exactly what we want, so this commit changes
`DiagnosticBuilder::foo_mv` to `DiagnosticBuilder::with_foo`.

Thanks to @compiler-errors for the suggestion.
@nnethercote
Add missing DiagnosticBuilder::eager_diagnostic method.
700a396 
This lets us avoid the use of `DiagnosticBuilder::into_diagnostic` in
miri, when then means that `DiagnosticBuilder::into_diagnostic` can
become private, being now only used by `stash` and `buffer`.
@compiler-errors
Copy link
Member

r? oli-obk

kind of busy today so i want to offload some currently assigned reviews

@rustbot rustbot assigned oli-obk and unassigned compiler-errors Jan 10, 2024
@oli-obk
Copy link
Contributor

oli-obk commented Jan 10, 2024

@bors r+ p=1 bitrotty

@bors
Copy link
Contributor

bors commented Jan 10, 2024

📌 Commit 700a396 has been approved by oli-obk

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 Jan 10, 2024
@bors
Copy link
Contributor

bors commented Jan 10, 2024

⌛ Testing commit 700a396 with merge a2d9d73...

@bors
Copy link
Contributor

bors commented Jan 10, 2024

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing a2d9d73 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 10, 2024
@bors bors merged commit a2d9d73 into rust-lang:master Jan 10, 2024
12 checks passed
@rustbot rustbot added this to the 1.77.0 milestone Jan 10, 2024
This was referenced Jan 10, 2024
Closed
Closed
@nnethercote nnethercote deleted the error-api-fixes branch January 10, 2024 20:51
This was referenced Jan 10, 2024
Merged
Merged
Merged
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (a2d9d73): 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.7% [-0.7%, -0.7%] 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)		 0.8% [0.4%, 1.1%] 2
Regressions ❌
(secondary)		 1.3% [1.1%, 1.5%] 2
Improvements ✅
(primary)		 -0.5% [-0.5%, -0.5%] 1
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) 0.4% [-0.5%, 1.1%] 3

Cycles

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

Binary size

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

Bootstrap: 666.26s -> 667.319s (0.16%)
Artifact size: 308.39 MiB -> 308.43 MiB (0.01%)

@celinval celinval mentioned this pull request Jan 18, 2024
Merged
feliperodri pushed a commit to model-checking/kani that referenced this pull request Jan 18, 2024
@celinval
Upgrade toolchain to nightly-2024-01-17 (#2976)
b349137 
Fixes were done to address the following upstream changes:

- rust-lang/rust#119606
- rust-lang/rust#119751
- rust-lang/rust#120025
- rust-lang/rust#116520

Resolves #2971 

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 and MIT licenses.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RalfJung RalfJung RalfJung left review comments

Assignees

@oli-obk oli-obk

Labels
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. 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
None yet
Milestone
1.77.0
Development

Successfully merging this pull request may close these issues.
7 participants
@nnethercote @rustbot @bors @compiler-errors @oli-obk @rust-timer @RalfJung