Track where diagnostics were created. #103217

mejrs · 2022-10-18T22:31:24Z

This implements the -Ztrack-diagnostics flag, which uses #[track_caller] to track where diagnostics are created. It is meant as a debugging tool much like -Ztreat-err-as-bug.

For example, the following code...

struct A;
struct B;

fn main(){
    let _: A = B;
}

...now emits the following error message:

error[E0308]: mismatched types
 --> src\main.rs:5:16
  |
5 |     let _: A = B;
  |            -   ^ expected struct `A`, found struct `B`
  |            |
  |            expected due to this
-Ztrack-diagnostics: created at compiler\rustc_infer\src\infer\error_reporting\mod.rs:2275:31

rustbot · 2022-10-18T22:31:27Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

rust-highfive · 2022-10-18T22:31:27Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @eholk (or someone else) soon.

Please see the contribution instructions for more information.

jyn514 · 2022-10-18T22:37:14Z

compiler/rustc_driver/src/lib.rs

@@ -1211,6 +1211,7 @@ pub fn report_ice(info: &panic::PanicInfo<'_>, bug_report_url: &str) {
        false,
        None,
        false,
+        false,


this is getting to be a lot of arguments, maybe add a Config struct? Doesn't need to block this PR though

compiler/rustc_errors/src/emitter.rs

src/librustdoc/core.rs

src/test/ui/track-diagnostics/track.rs

jyn514 · 2022-10-18T22:43:21Z

src/tools/clippy/clippy_lints/src/doc.rs

@@ -691,6 +691,7 @@ fn check_code(cx: &LateContext<'_>, text: &str, edition: Edition, span: Span) {
                    false,
                    None,
                    false,
+                    false,


Same for clippy and rustfmt.

compiler-errors · 2022-10-18T22:51:14Z

Not sure about -Z flags, but does this need an MCP?

compiler/rustc_session/src/options.rs

jyn514 · 2022-10-18T23:05:14Z

compiler/rustc_session/src/session.rs

+            fallback_bundle,
+            None,
+            false,
+            false,


I think if you don't set this for JSON output, it won't show up when using cargo.

I'm not sure what you mean with that. Afaik most code that calls this function doesn't have the sessions options available to it (because it's creating it). So i'm not sure what to do here.

Oh, this is in early_error, I misread. Ok, seems fine to skip for now. I think it shouldn't be too hard to add it though, nearly every place that calls this has access to Options and those that don't can still call matches.opt_get("track-diagnostics") before calling the error code.

davidtwco · 2022-10-21T08:31:00Z

Not sure about -Z flags, but does this need an MCP?

-Z flags don't need an MCP to the best of my knowledge

compiler/rustc_errors/src/diagnostic.rs

src/test/rustdoc-ui/track-diagnostics.rs

eholk · 2022-10-25T21:43:02Z

I'm starting to look over this PR. One general question, is adding #[track_caller] to several functions going to cause issues if these ever panic or do Option::unwrap() or something like that? Will this make debugging those issues trickier?

eholk · 2022-10-25T22:12:08Z

I think this looks pretty good! The main thing left is to make sure all of @jyn514's comments get addressed (it looks like the hardcoding changes for clippy and rustfmt are the main ones).

camsteffen · 2022-10-29T13:59:12Z

When would this be used rather than treat-err-as-bug? I think in my experience I always just want to know where one specific error comes from, and with a whole stack trace.

jyn514 · 2022-10-29T14:46:37Z

@camsteffen this can be used in a compiler that has debug symbols disabled, and without reading through 200 lines of stack trace from rustc_query_impl.

rustbot · 2022-10-31T14:57:32Z

rustc_macros::diagnostics was changed

cc @davidtwco, @compiler-errors, @JohnTitor, @estebank, @TaKO8Ki

compiler/rustc_session/src/options.rs

mejrs · 2022-10-31T15:34:45Z

@jyn514 have I addressed all of your comments? I sort of lost track because I bit off small pieces here and there over the last week and I'm not totally sure I understood everything completely.

I'm starting to look over this PR. One general question, is adding #[track_caller] to several functions going to cause issues if these ever panic or do Option::unwrap() or something like that? Will this make debugging those issues trickier?

That could be a bit annoying if it happens but I don't think it's an issue. Most of the functions that have it now just call into_diagnostic or call something that does. Given that most of these impls are macro generated rather than hand written I don't expect mistakes to be made in them.

jyn514 · 2022-10-31T16:02:22Z

@jyn514 have I addressed all of your comments? I sort of lost track because I bit off small pieces here and there over the last week and I'm not totally sure I understood everything completely.

Hmm, the only one I had left was #103217 (comment), but it looks like clippy is emitting -Ztrack-diagnostics notes even though you always pass in false? Not sure what's going on there.

mejrs · 2022-11-01T18:09:25Z

it looks like clippy is emitting -Ztrack-diagnostics notes even though you always pass in false? Not sure what's going on there.

It looks like clippy uses the compiler's machinery for that, and just provides a callback to add its configuration? The change I made in clippy driver just passes in false if an ice is reported.

jyn514 · 2022-11-01T19:04:12Z

Ok, great. I'm still not sure rustfmt is handling the option properly, but we can always add it later if someone complains.

I think this looks pretty good! The main thing left is to make sure all of @jyn514's comments get addressed (it looks like the hardcoding changes for clippy and rustfmt are the main ones).

@bors r=eholk rollup=iffy

bors · 2022-11-01T19:04:15Z

📌 Commit cbeb244 has been approved by eholk

It is now in the queue for this repository.

bors · 2022-11-01T21:09:49Z

⌛ Testing commit cbeb244 with merge 11ebe65...

bors · 2022-11-01T23:57:52Z

☀️ Test successful - checks-actions
Approved by: eholk
Pushing 11ebe65 to master...

rust-timer · 2022-11-02T01:17:06Z

Finished benchmarking commit (11ebe65): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

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)	2.2%	[1.6%, 2.8%]	6
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
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)	4.2%	[4.2%, 4.2%]	1
Improvements ✅ (primary)	-0.5%	[-0.5%, -0.5%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.5%	[-0.5%, -0.5%]	1

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

lqd · 2022-11-02T08:36:31Z

I think deeply-nested-multi may be becoming noisy.

rylev · 2022-11-02T14:17:20Z

I think the likelihood that this is noise is pretty high as you can see from the end of this graph. I think we should assume noise in this case. If it's true regression it's small enough to not be a big deal.

@rustbot label: +perf-regression-triaged

calebcartwright · 2022-11-02T15:16:26Z

Ok, great. I'm still not sure rustfmt is handling the option properly, but we can always add it later if someone complains.

Possibly easier to chat in Zulip, but could you help me understand what you mean by this @jyn514? Admit I've only done a cursory pass through this PR but it's not clear to me

jyn514 · 2022-11-02T15:53:41Z

@calebcartwright I mean I'm not sure that running rustfmt -Ztrack-diagnostics will print the new message @mejrs added, since it's passing in false unconditionally. It should be something that's easy to test, I just haven't had time.

calebcartwright · 2022-11-08T21:17:28Z

@calebcartwright I mean I'm not sure that running rustfmt -Ztrack-diagnostics will print the new message @mejrs added, since it's passing in false unconditionally. It should be something that's easy to test, I just haven't had time.

Gotcha. I can't conjure up a persona that I think would need that right now, so I'm happy to punt this to a future caleb on the off chance anyone ever does come up with a need

Track where diagnostics were created. This implements the `-Ztrack-diagnostics` flag, which uses `#[track_caller]` to track where diagnostics are created. It is meant as a debugging tool much like `-Ztreat-err-as-bug`. For example, the following code... ```rust struct A; struct B; fn main(){ let _: A = B; } ``` ...now emits the following error message: ``` error[E0308]: mismatched types --> src\main.rs:5:16 | 5 | let _: A = B; | - ^ expected struct `A`, found struct `B` | | | expected due to this -Ztrack-diagnostics: created at compiler\rustc_infer\src\infer\error_reporting\mod.rs:2275:31 ```

Implement -Ztrack-diagnostics

406e1dc

rustbot added 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 Oct 18, 2022

rust-highfive assigned eholk Oct 18, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 18, 2022

jyn514 reviewed Oct 18, 2022

View reviewed changes

This comment has been minimized.

Sign in to view

compiler-errors reviewed Oct 18, 2022

View reviewed changes

compiler/rustc_session/src/options.rs Outdated Show resolved Hide resolved

jyn514 reviewed Oct 18, 2022

View reviewed changes

This comment has been minimized.

Sign in to view

Address some comments

854b316

This comment has been minimized.

Sign in to view

Add more normalization and tests

447d57f

This comment has been minimized.

Sign in to view

WaffleLapkin reviewed Oct 25, 2022

View reviewed changes

compiler/rustc_errors/src/diagnostic.rs Show resolved Hide resolved

src/test/rustdoc-ui/track-diagnostics.rs Outdated Show resolved Hide resolved

mejrs added 2 commits October 26, 2022 13:41

Adjust normalization

60dbffb

fix tracking hash test

fae0be5

Add parser test

0f35c0c

rustbot added the A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic label Oct 31, 2022

jyn514 reviewed Oct 31, 2022

View reviewed changes

compiler/rustc_session/src/options.rs Outdated Show resolved Hide resolved

Add more track_caller

cbeb244

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 1, 2022

bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 1, 2022

bors merged commit 11ebe65 into rust-lang:master Nov 1, 2022

rustbot added this to the 1.67.0 milestone Nov 1, 2022

bors mentioned this pull request Nov 2, 2022

incremental: migrate diagnostics #100754

Merged

rustbot added the perf-regression Performance regression. label Nov 2, 2022

rustbot added the perf-regression-triaged The performance regression has been triaged. label Nov 2, 2022

mejrs mentioned this pull request Nov 5, 2022

Add -Ztrack-diagnostics information rust-lang/rustc-dev-guide#1506

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Track where diagnostics were created. #103217

Track where diagnostics were created. #103217

mejrs commented Oct 18, 2022

rustbot commented Oct 18, 2022

rust-highfive commented Oct 18, 2022

jyn514 Oct 18, 2022

jyn514 Oct 18, 2022

This comment has been minimized.

compiler-errors commented Oct 18, 2022

jyn514 Oct 18, 2022

mejrs Oct 30, 2022

jyn514 Oct 30, 2022

davidtwco commented Oct 21, 2022

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

eholk commented Oct 25, 2022

eholk commented Oct 25, 2022

camsteffen commented Oct 29, 2022

jyn514 commented Oct 29, 2022

rustbot commented Oct 31, 2022

mejrs commented Oct 31, 2022 •

edited

Loading

jyn514 commented Oct 31, 2022 •

edited

Loading

mejrs commented Nov 1, 2022

jyn514 commented Nov 1, 2022

bors commented Nov 1, 2022

bors commented Nov 1, 2022

bors commented Nov 1, 2022

rust-timer commented Nov 2, 2022

lqd commented Nov 2, 2022

rylev commented Nov 2, 2022

calebcartwright commented Nov 2, 2022

jyn514 commented Nov 2, 2022

calebcartwright commented Nov 8, 2022

Track where diagnostics were created. #103217

Track where diagnostics were created. #103217

Conversation

mejrs commented Oct 18, 2022

rustbot commented Oct 18, 2022

rust-highfive commented Oct 18, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

compiler-errors commented Oct 18, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidtwco commented Oct 21, 2022

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

eholk commented Oct 25, 2022

eholk commented Oct 25, 2022

camsteffen commented Oct 29, 2022

jyn514 commented Oct 29, 2022

rustbot commented Oct 31, 2022

mejrs commented Oct 31, 2022 • edited Loading

jyn514 commented Oct 31, 2022 • edited Loading

mejrs commented Nov 1, 2022

jyn514 commented Nov 1, 2022

bors commented Nov 1, 2022

bors commented Nov 1, 2022

bors commented Nov 1, 2022

rust-timer commented Nov 2, 2022

Overall result: ❌ regressions - ACTION NEEDED

lqd commented Nov 2, 2022

rylev commented Nov 2, 2022

calebcartwright commented Nov 2, 2022

jyn514 commented Nov 2, 2022

calebcartwright commented Nov 8, 2022

mejrs commented Oct 31, 2022 •

edited

Loading

jyn514 commented Oct 31, 2022 •

edited

Loading