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

Add $message_type field to distinguish json diagnostic outputs #115691

Merged
merged 8 commits into from
Nov 21, 2023

Conversation

jsgf
Copy link
Contributor

@jsgf jsgf commented Sep 9, 2023

Currently the json-formatted outputs have no way to unambiguously determine which kind of message is being output. A consumer can look for specific fields in the json object (eg "message"), but there's no guarantee that in future some other kind of output will have a field of the same name.

This PR adds a "type" field to add json outputs which can be used to unambiguously determine which kind of output it is. The mapping is:

diagnostic: regular compiler diagnostics
artifact: artifact notifications
future_incompat: Future incompatibility report
unused_extern: Unused crate warnings/errors

This matches the "internally tagged" representation for serde enums.

@rustbot
Copy link
Collaborator

rustbot commented Sep 9, 2023

r? @b-naber

(rustbot has picked a reviewer for you, use r? to override)

@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. labels Sep 9, 2023
@rust-log-analyzer

This comment has been minimized.

@jsgf
Copy link
Contributor Author

jsgf commented Sep 9, 2023

cc @JakobDegen

@rust-log-analyzer

This comment has been minimized.

@JakobDegen

This comment was marked as outdated.

@JakobDegen
Copy link
Contributor

Oh actually @jsgf , the right process here is to file a compiler MCP over at rust-lang/compiler-team . That'll be where the FCP happens.

You should be able to just more or less copy your summary.

@jsgf
Copy link
Contributor Author

jsgf commented Sep 9, 2023

@ehuss
Copy link
Contributor

ehuss commented Sep 9, 2023

Can you please also update the documentation at https://github.com/rust-lang/rust/blob/master/src/doc/rustc/src/json.md?

@jsgf
Copy link
Contributor Author

jsgf commented Sep 9, 2023

@ehuss Is it time to stabilize --json=unused-externs? I forgot that it wasn't and was wondering why it was missing from json.md. (cc @est31)

Edit: rust-lang/compiler-team#674

@est31
Copy link
Member

est31 commented Sep 10, 2023

I think this is a great idea, as it lets one distinguish the various possible json messages without having to try to deserialize each one.

@jsgf jsgf changed the title Add type field to json diagnostic outputs Add type field to distinguish json diagnostic outputs Sep 11, 2023
@bors
Copy link
Contributor

bors commented Sep 19, 2023

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

jsgf and others added 5 commits September 19, 2023 14:17
Currently the json-formatted outputs have no way to unambiguously
determine which kind of message is being output. A consumer can look for
specific fields in the json object (eg "message"), but there's no
guarantee that in future some other kind of output will have a field of
the same name.

This PR adds a `"type"` field to add json outputs which can be used to
unambiguously determine which kind of output it is. The mapping is:

diagnostic: regular compiler diagnostics
artifact: artifact notifications
future_incompat: Report of future incompatibility
unused_extern: Unused crate warnings/errors

This matches the "internally tagged" representation for serde enums.
Avoids an unnecessary intermediate string.
`type` turned out to be controversial.
@jsgf jsgf changed the title Add type field to distinguish json diagnostic outputs Add $message_type field to distinguish json diagnostic outputs Oct 14, 2023
@dtolnay dtolnay mentioned this pull request Nov 20, 2023
@dtolnay
Copy link
Member

dtolnay commented Nov 20, 2023

This will be ready to land again after #118101 merges. A rebase shouldn't be needed.

@rustbot label +S-blocked -S-waiting-on-author

@rustbot rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 20, 2023
@dtolnay dtolnay removed the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Nov 21, 2023
@dtolnay
Copy link
Member

dtolnay commented Nov 21, 2023

@bors r=est31,dtolnay

@bors
Copy link
Contributor

bors commented Nov 21, 2023

📌 Commit fe50c53 has been approved by est31,dtolnay

It is now in the queue for this repository.

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Nov 21, 2023
@bors
Copy link
Contributor

bors commented Nov 21, 2023

⌛ Testing commit fe50c53 with merge 698e691...

bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 21, 2023
Add `$message_type` field to distinguish json diagnostic outputs

Currently the json-formatted outputs have no way to unambiguously determine which kind of message is being output. A consumer can look for specific fields in the json object (eg "message"), but there's no guarantee that in future some other kind of output will have a field of the same name.

This PR adds a `"type"` field to add json outputs which can be used to unambiguously determine which kind of output it is. The mapping is:

`diagnostic`: regular compiler diagnostics
`artifact`: artifact notifications
`future_incompat`: Future incompatibility report
`unused_extern`: Unused crate warnings/errors

This matches the "internally tagged" representation for serde enums.
@rust-log-analyzer
Copy link
Collaborator

The job armhf-gnu failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@bors
Copy link
Contributor

bors commented Nov 21, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 21, 2023
@dtolnay
Copy link
Member

dtolnay commented Nov 21, 2023

failures:

---- [ui] tests/ui/issues/issue-39808.rs stdout ----

error: test run failed!
status: exit status: 101
command: RUST_TEST_THREADS="8" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/remote-test-client" "run" "0" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/issues/issue-39808/a"
--- stdout -------------------------------
uploaded "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/issues/issue-39808/a", waiting for result
------------------------------------------
--- stderr -------------------------------
thread 'main' panicked at src/tools/remote-test-client/src/main.rs:310:9:
client.read_exact(&mut header) failed with Connection reset by peer (os error 104)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Sounds unrelated to me.

@bors retry

@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 Nov 21, 2023
@jsgf
Copy link
Contributor Author

jsgf commented Nov 21, 2023

@dtolnay Yeah looks spurious.

@bors
Copy link
Contributor

bors commented Nov 21, 2023

⌛ Testing commit fe50c53 with merge 85c42b7...

@bors
Copy link
Contributor

bors commented Nov 21, 2023

☀️ Test successful - checks-actions
Approved by: est31,dtolnay
Pushing 85c42b7 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 21, 2023
@bors bors merged commit 85c42b7 into rust-lang:master Nov 21, 2023
12 checks passed
@rustbot rustbot added this to the 1.76.0 milestone Nov 21, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (85c42b7): 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.3% [-0.3%, -0.3%] 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
Regressions ❌
(secondary)
1.8% [0.8%, 2.8%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-1.7% [-3.6%, -0.7%] 4
All ❌✅ (primary) - - 0

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: 674.879s -> 675.345s (0.07%)
Artifact size: 313.82 MiB -> 313.77 MiB (-0.01%)

@jsgf jsgf deleted the typed-json-diags branch November 22, 2023 07:07
wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request Feb 18, 2024
Pkgsrc changes:
 * Adapt checksums and patches.

Upstream chnages:

Version 1.76.0 (2024-02-08)
==========================

Language
--------
- [Document Rust ABI compatibility between various types]
  (rust-lang/rust#115476)
- [Also: guarantee that char and u32 are ABI-compatible]
  (rust-lang/rust#118032)
- [Warn against ambiguous wide pointer comparisons]
  (rust-lang/rust#117758)

Compiler
--------
- [Lint pinned `#[must_use]` pointers (in particular, `Box<T>`
  where `T` is `#[must_use]`) in `unused_must_use`.]
  (rust-lang/rust#118054)
- [Soundness fix: fix computing the offset of an unsized field in
  a packed struct]
  (rust-lang/rust#118540)
- [Soundness fix: fix dynamic size/align computation logic for
  packed types with dyn Trait tail]
  (rust-lang/rust#118538)
- [Add `$message_type` field to distinguish json diagnostic outputs]
  (rust-lang/rust#115691)
- [Enable Rust to use the EHCont security feature of Windows]
  (rust-lang/rust#118013)
- [Add tier 3 {x86_64,i686}-win7-windows-msvc targets]
  (rust-lang/rust#118150)
- [Add tier 3 aarch64-apple-watchos target]
  (rust-lang/rust#119074)
- [Add tier 3 arm64e-apple-ios & arm64e-apple-darwin targets]
  (rust-lang/rust#115526)

Refer to Rust's [platform support page][platform-support-doc]
for more information on Rust's tiered platform support.

Libraries
---------
- [Add a column number to `dbg!()`]
  (rust-lang/rust#114962)
- [Add `std::hash::{DefaultHasher, RandomState}` exports]
  (rust-lang/rust#115694)
- [Fix rounding issue with exponents in fmt]
  (rust-lang/rust#116301)
- [Add T: ?Sized to `RwLockReadGuard` and `RwLockWriteGuard`'s Debug impls.]
  (rust-lang/rust#117138)
- [Windows: Allow `File::create` to work on hidden files]
  (rust-lang/rust#116438)

Stabilized APIs
---------------
- [`Arc::unwrap_or_clone`]
  (https://doc.rust-lang.org/stable/std/sync/struct.Arc.html#method.unwrap_or_clone)
- [`Rc::unwrap_or_clone`]
  (https://doc.rust-lang.org/stable/std/rc/struct.Rc.html#method.unwrap_or_clone)
- [`Result::inspect`]
  (https://doc.rust-lang.org/stable/std/result/enum.Result.html#method.inspect)
- [`Result::inspect_err`]
  (https://doc.rust-lang.org/stable/std/result/enum.Result.html#method.inspect_err)
- [`Option::inspect`]
  (https://doc.rust-lang.org/stable/std/option/enum.Option.html#method.inspect)
- [`type_name_of_val`]
  (https://doc.rust-lang.org/stable/std/any/fn.type_name_of_val.html)
- [`std::hash::{DefaultHasher, RandomState}`]
  (https://doc.rust-lang.org/stable/std/hash/index.html#structs)
  These were previously available only through `std::collections::hash_map`.
- [`ptr::{from_ref, from_mut}`]
  (https://doc.rust-lang.org/stable/std/ptr/fn.from_ref.html)
- [`ptr::addr_eq`](https://doc.rust-lang.org/stable/std/ptr/fn.addr_eq.html)

Cargo
-----

See [Cargo release notes]
(https://github.com/rust-lang/cargo/blob/master/CHANGELOG.md#cargo-176-2024-02-08).

Rustdoc
-------
- [Don't merge cfg and doc(cfg) attributes for re-exports]
  (rust-lang/rust#113091)
- [rustdoc: allow resizing the sidebar / hiding the top bar]
  (rust-lang/rust#115660)
- [rustdoc-search: add support for traits and associated types]
  (rust-lang/rust#116085)
- [rustdoc: Add highlighting for comments in items declaration]
  (rust-lang/rust#117869)

Compatibility Notes
-------------------
- [Add allow-by-default lint for unit bindings]
  (rust-lang/rust#112380)
  This is expected to be upgraded to a warning by default in a future Rust
  release. Some macros emit bindings with type `()` with user-provided spans,
  which means that this lint will warn for user code.
- [Remove x86_64-sun-solaris target.]
  (rust-lang/rust#118091)
- [Remove asmjs-unknown-emscripten target]
  (rust-lang/rust#117338)
- [Report errors in jobserver inherited through environment variables]
  (rust-lang/rust#113730)
  This [may warn](rust-lang/rust#120515)
  on benign problems too.
- [Update the minimum external LLVM to 16.]
  (rust-lang/rust#117947)
- [Improve `print_tts`](rust-lang/rust#114571)
  This change can break some naive manual parsing of token trees
  in proc macro code which expect a particular structure after
  `.to_string()`, rather than just arbitrary Rust code.
- [Make `IMPLIED_BOUNDS_ENTAILMENT` into a hard error from a lint]
  (rust-lang/rust#117984)
- [Vec's allocation behavior was changed when collecting some iterators]
  (rust-lang/rust#110353)
  Allocation behavior is currently not specified, nevertheless
  changes can be surprising.
  See [`impl FromIterator for Vec`]
  (https://doc.rust-lang.org/nightly/std/vec/struct.Vec.html#impl-FromIterator%3CT%3E-for-Vec%3CT%3E)
  for more details.
- [Properly reject `default` on free const items]
  (rust-lang/rust#117818)
@dtolnay dtolnay self-assigned this Mar 24, 2024
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.