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

Emitter cleanups #119601

Merged
merged 5 commits into from
Jan 5, 2024
Merged

Emitter cleanups #119601

merged 5 commits into from
Jan 5, 2024

Conversation

nnethercote
Copy link
Contributor

Some improvements I found while looking at this code.

r? @oli-obk

For consistency with other `Emitter` impls, such as `JsonEmitter`,
`SilentEmitter`, `SharedEmitter`, etc.
For consistency with other `Emitter` impls.
There are three functions only used for the JSON format.
PR rust-lang#82639 changed UI tests so `-Z unstable-options` aren't passed to UI
tests by default. This completely broke `use_suggestion_json.rs`, which
uses the unstable `--error-format=pretty-json` option. The expected
output went from 400 lines of pretty JSON error messages to a single
JSON error saying "`--error-format=pretty-json` is unstable"!

This commit adds `-Z unstable-options` back and reinstates the old
expected output, with some minor changes to account for shifted spans
and slightly JSON output changes since then.
Currently for these two errors we go to the effort of switching to a
standard JSON emitter, for no obvious reason, and unlike any other
errors. This behaviour was added for `pretty-json` in rust-lang#45737, and then
`human-annotate-rs` copied it some time later when it was added.

This commit changes things to just using the requested emitter, which is
simpler and consistent with other errors.

Old output:
```
$ rustc --error-format pretty-json
{"$message_type":"diagnostic","message":"`--error-format=pretty-json` is unstable","code":null,"level":"error","spans":[],"children":[],"rendered":"error: `--error-format=pretty-json` is unstable\n\n"}

$ rustc --error-format human-annotate-rs
{"$message_type":"diagnostic","message":"`--error-format=human-annotate-rs` is unstable","code":null,"level":"error","spans":[],"children":[],"rendered":"error: `--error-format=human-annotate-rs` is unstable\n\n"}
```

New output:
```
$ rustc --error-format pretty-json
{
  "$message_type": "diagnostic",
  "message": "`--error-format=pretty-json` is unstable",
  "code": null,
  "level": "error",
  "spans": [],
  "children": [],
  "rendered": "error: `--error-format=pretty-json` is unstable\n\n"
}

$ rustc --error-format human-annotate-rs
error: `--error-format=human-annotate-rs` is unstable
```
@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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jan 5, 2024
@rustbot
Copy link
Collaborator

rustbot commented Jan 5, 2024

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

@nnethercote
Copy link
Contributor Author

Best reviewed one commit at a time.

cc @jyn514 for the "Unbreak tests/ui/lint/use_suggestion_json.rs" commit.

@oli-obk
Copy link
Contributor

oli-obk commented Jan 5, 2024

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jan 5, 2024

📌 Commit 8843223 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 5, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 5, 2024
…mpiler-errors

Rollup of 10 pull requests

Successful merges:

 - rust-lang#119034 (Allow coverage tests to ignore test modes, and to enable color in coverage reports)
 - rust-lang#119148 (Tweak suggestions for bare trait used as a type)
 - rust-lang#119538 (Cleanup error handlers: round 5)
 - rust-lang#119566 (Remove `-Zdump-mir-spanview`)
 - rust-lang#119567 (Remove `-Zreport-delayed-bugs`.)
 - rust-lang#119577 (Migrate memory overlap check from validator to lint)
 - rust-lang#119583 (Make `intrinsics::assume` const stable)
 - rust-lang#119586 ([rustdoc] Fix invalid handling for static method calls in jump to definition feature)
 - rust-lang#119588 (Move `i586-unknown-netbsd` from tier 2 to tier 3 platform support table)
 - rust-lang#119601 (`Emitter` cleanups)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit da700b3 into rust-lang:master Jan 5, 2024
11 checks passed
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 5, 2024
Rollup merge of rust-lang#119601 - nnethercote:Emitter-cleanups, r=oli-obk

`Emitter` cleanups

Some improvements I found while looking at this code.

r? `@oli-obk`
@rustbot rustbot added this to the 1.77.0 milestone Jan 5, 2024
@nnethercote nnethercote deleted the Emitter-cleanups branch January 7, 2024 20:08
zhassan-aws added a commit to model-checking/kani that referenced this pull request Jan 8, 2024
Relevant PRs:
- rust-lang/rust#119601
- rust-lang/rust#119146
- rust-lang/rust#119174

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 and MIT licenses.
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jan 11, 2024
…i-obk

`Emitter` cleanups

Some improvements I found while looking at this code.

r? `@oli-obk`
calebcartwright pushed a commit to calebcartwright/rust that referenced this pull request Jun 22, 2024
…i-obk

`Emitter` cleanups

Some improvements I found while looking at this code.

r? `@oli-obk`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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
Development

Successfully merging this pull request may close these issues.

None yet

4 participants