Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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
@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
# for free to join this conversation on GitHub. Already have an account? # 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.