Add `-Z teach` flag to provide extended diagnostic help #47652

estebank · 2018-01-22T05:30:03Z

First step to have a way for the compiler to provide as much information as the --explain E0XXX flag does, but providing the text in the context of the users' code.

r? @nikomatsakis

estebank · 2018-01-22T05:39:36Z

nikomatsakis · 2018-01-22T16:46:24Z

Maybe we need a different name (for back-compat).

Here are some synonyms:

--expound, --say-more, --details, --teach, --elucidate

I think I like --teach.

nikomatsakis · 2018-01-22T16:47:34Z

Also, long term, do you envision us having this text etc in the code like this, or separated into some form of "template"?

estebank · 2018-01-22T17:36:22Z

At the very least I would separate the place where the error is being emitted from the error being prepared for presentation. Using a template is a good possibility, but I feel it can be approached independently from the introduction of the flag.

The way to use it now is rustc -Z unstable-options -Z explain file.rs. I feel that using the same flag shouldn't as now shouldn't be a problem, as in both cases the intention is the same, give me more context on what I'm seeing. Also, the content should be, of not the same, equivalent for both, to the point where explain E0XXX should look like diagnostic output too.

estebank · 2018-01-22T17:37:14Z

Having said that, teach is a nice option.

nikomatsakis · 2018-01-22T17:54:46Z

At the very least I would separate the place where the error is being emitted from the error being prepared for presentation. Using a template is a good possibility, but I feel it can be approached independently from the introduction of the flag.

Yes, it can be, I just wanted to clarify. In particular, I don't know whether we will want to be writing a lot of such explanations in this style, or whether we'd prefer to sketch out how a template would look.

Personally, I think we should move towards a templating engine, but I am also appreciate of the fact that it'd be nice to get started now and not block on things.

The way to use it now is rustc -Z unstable-options -Z explain file.rs

First off, you should not need -Zunstable-options, -Z itself already implies unstable.

This is also why I don't think we should use -Zexplain -- because, once we stabilize, we are going to want a stable name, and --explain is already taken. (Unless we want to make it something like --explain all, but I'd rather just deprecate --explain entirely.)

(Hmm, I suppose we could move towards a model where --explain E123 is like a "filter" on where you get long vs short form explanations. That is, --explain E123 would display errors as short unless their code is E123, and then it would give the full details; then --explain all might show everything long.)

In any case, I'm ok with -Zexplain for now also, but it's something we should decide eventually.

nikomatsakis · 2018-01-22T17:56:48Z

Do we have a tracking issue for this feature? If so, we ought to start documenting these questions.

zackmdavis · 2018-01-22T18:00:58Z

src/librustc_typeck/check/cast.rs

+the additional information the fat pointer holds is their size.");
+                    err.note("to fix this error, don't try to cast directly between thin and fat \
+                              pointers");
+                    err.help("for more information about casts, take a look at


missing backslash? (notice enormous margin before "The Book" in the screenshot)

zackmdavis · 2018-01-22T18:14:20Z

Using a template is a good possibility, but I feel it can be approached independently from the introduction of the flag.

We should keep in mind that deferring work to the future is only a good idea when the minimal PR leaves the code base in a better state than it was previously (even if it's not optimal); we don't want giant explanations cluttering up our code forever if no one gets around to implementing templates in a timely manner.

estebank · 2018-01-23T02:12:00Z

9b4960e is an early mockup of what StructuredDiagnostics would look like. Right now I'm not too happy with how it looks, as it is slightly more verbose than the previous code, but I think it works as a starting point for talking and iterating upon. Having all the diagnostic errors that could use templates in one place would make it easier to migrate them once such a solution is written.

nikomatsakis · 2018-01-23T17:39:37Z

The more I think about it, the more I like --teach. I think we should call this -Zteach for now =)

nikomatsakis · 2018-01-23T17:43:05Z

src/librustc_typeck/check/cast.rs

-                }
-                err.emit();
+                use structured_errors::{SizedUnsizedCastError, StructuredDiagnostic};
+                SizedUnsizedCastError::new(&fcx.tcx.sess,


Personally, I'd rather something with labels I think, like:

SizedUnsizedCastError { span: self.span, thin_ty: self.expr_ty, fat_ty: fcx.ty_to_string(self.cast_ty) // although if we could do `self.cast_ty` it'd be even better }.emit(fcx.tcx);

These labels would then also appear in the templating logic.

I'm thinking that the templating could look something like the following:

diagnostic_template!( Any independent text will be notes. Such as this one and the previous one. Here is where a span would appear: $secondary_span the label for the span immediately after I haven't thought about how to deal with suggestions. )

error[E0XXX]: error message outside of template --> $FILE:21:9 | 21 | foo(); | ^^^^^ = note: Any independent text will be notes. Such as this one and the previous one. note: Here is where a span would appear: --> $FILE:21:9 | 21 | foo(); | ^^^^^ the label for the span immediately after = note: I haven't thought about how to deal with suggestions.

In the background it would convert the template string into calls to err.*().

How does that sound?

Interesting. Quite different than what I had in mind, but I like it as a start, and maybe more practical than some of my more grandiose ideas =)

I think one key question is whether the --teach output would still have the "basic structure" of the messages that we've been giving (as in this PR), or whether it would change to something more essay like.

I think I had initially imagined that, in --teach mode, we would change "our look" somewhat drastically. e.g (using E0010 as an example):

error[E0010]: boxed values cannot be used in statics --> $FILE:21:9 | 21 | const CON : Box<i32> = box 0; | ^^^ The value of statics and constants must be known at compile time, and they live for the entire lifetime of a program. Creating a boxed value allocates memory on the heap at runtime, and therefore cannot be done at compile time.

For more complex examples, there might be multiple snippets:

error[E0010]: shadowed constant `CON` --> $FILE:21:9 | 21 | const CON : Box<i32> = box 0; | ^^^ This code is illegal. The problem is that the const CON is shadowed by another constant declared earlier, also named `CON`: --> $FILE:18:9 | 18 | const CON : Box<i32> = box 22; | ^^^

The only reason I wasn't considering moving in that direction is because I though we wouldn't want to present the same explanation over and over again, so mixing the current diagnostics with the teach diagnostics would be a bit jarring. But if we don't mind either mixing both styles or repeating walls of text, then I don't see why we wouldn't do it.

Also, I think that the differences in output could act like a stylesheet, where the DiagnosticBuilder is still created in the same way, but when emitting it to the output we change how things are presented depending on wether we're in teach mode or not, by removing the left margin on notes, line wrapping, how we deal with suggestions, maybe even highlighting code based on this.

Create the concept of an `StructuredDiagnostic` that is self-contained with enough knowledge of all variables to create a `DiagnosticBuilder`, including different possible versions (one line output and expanded explanations).

nikomatsakis

OK, good start. I just left a nit that I think a comment would be good (not that this is a particularly confusing case, just that I think we should start to comment more)

r=me with that

nikomatsakis · 2018-01-25T18:12:23Z

src/librustc_errors/lib.rs

@@ -244,6 +244,7 @@ pub struct Handler {
    continue_after_error: Cell<bool>,
    delayed_span_bug: RefCell<Option<Diagnostic>>,
    tracked_diagnostics: RefCell<Option<Vec<Diagnostic>>>,
+    tracked_diagnostic_codes: RefCell<FxHashSet<DiagnosticId>>,


can we put a comment on this field explaining what it's for?

nikomatsakis · 2018-01-25T18:13:02Z

src/librustc_errors/lib.rs

@@ -575,13 +577,21 @@ impl Handler {
        (ret, diagnostics)
    }

+    pub fn code_emitted(&self, code: &DiagnosticId) -> bool {


comment (short one would suffice)

e.g., /// True if a diagnostic with this code has already been emitted. Used to suppress duplicate messages with -Zteach.

nikomatsakis · 2018-01-25T18:15:28Z

I think however we should think about some kind of public document laying out the desired design here at some point. We could probably get a bunch of people involved in making this happen if we do it right!

estebank · 2018-01-25T21:53:16Z

@bors r=nikomatsakis

bors · 2018-01-25T21:53:17Z

📌 Commit 2b73733 has been approved by nikomatsakis

Add `-Zteach` documentation Add extra inline documentation to E0019, E0016, E0013, E0396, E0017, E0018, E0010, E0022, E0030, E0029, E0033, E0026 and E0027. Follow up to #47652.

rust-highfive assigned nikomatsakis Jan 22, 2018

kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 22, 2018

zackmdavis reviewed Jan 22, 2018

View reviewed changes

estebank force-pushed the explain branch 4 times, most recently from 4988c46 to 3f3198c Compare January 22, 2018 22:53

estebank added 7 commits January 22, 2018 15:46

Add --explain for extended error explanations

9adf2b2

Mark --explain as unstable

4e68e2a

Add explanation for E0607

fdfb9a2

Fix test for variadic error change

e76d3f6

Only emit expanded diagnostic information once

864f6d1

Fix test redux

ffb827a

Don't make it necessary to enable unstable-options

7d41cba

estebank force-pushed the explain branch from 3f3198c to 19d1637 Compare January 23, 2018 02:09

estebank force-pushed the explain branch 2 times, most recently from 16f7f68 to 9b4960e Compare January 23, 2018 02:35

nikomatsakis reviewed Jan 23, 2018

View reviewed changes

Create StructuredDiagnostic

3dac0f5

Create the concept of an `StructuredDiagnostic` that is self-contained with enough knowledge of all variables to create a `DiagnosticBuilder`, including different possible versions (one line output and expanded explanations).

estebank force-pushed the explain branch from 9b4960e to 3dac0f5 Compare January 23, 2018 19:01

Rename -Z explain to -Z teach

482f7f1

estebank changed the title ~~Add -Z explain flag to provide extended diagnostic help~~ Add -Z teach flag to provide extended diagnostic help Jan 23, 2018

nikomatsakis requested changes Jan 25, 2018

View reviewed changes

nikomatsakis added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 25, 2018

Add description to field and method

2b73733

estebank 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 25, 2018

bors merged commit 2b73733 into rust-lang:master Jan 26, 2018

estebank mentioned this pull request Jan 29, 2018

Add -Zteach documentation #47843

Merged

estebank mentioned this pull request Feb 6, 2018

Put back "try with rustc --explain E0XXX" #48041

Closed

estebank mentioned this pull request Jul 4, 2018

Handling of primary vs. secondary spans in checkers rust-lang/rust.vim#174

Open

estebank deleted the explain branch November 9, 2023 05:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `-Z teach` flag to provide extended diagnostic help #47652

Add `-Z teach` flag to provide extended diagnostic help #47652

estebank commented Jan 22, 2018

estebank commented Jan 22, 2018

nikomatsakis commented Jan 22, 2018

nikomatsakis commented Jan 22, 2018

estebank commented Jan 22, 2018

estebank commented Jan 22, 2018

nikomatsakis commented Jan 22, 2018

nikomatsakis commented Jan 22, 2018

zackmdavis Jan 22, 2018 •

edited

Loading

zackmdavis commented Jan 22, 2018

estebank commented Jan 23, 2018 •

edited

Loading

nikomatsakis commented Jan 23, 2018

nikomatsakis Jan 23, 2018

estebank Jan 23, 2018

nikomatsakis Jan 24, 2018 •

edited

Loading

estebank Jan 24, 2018

estebank Jan 24, 2018

nikomatsakis left a comment

nikomatsakis Jan 25, 2018

nikomatsakis Jan 25, 2018

nikomatsakis commented Jan 25, 2018

estebank commented Jan 25, 2018

bors commented Jan 25, 2018

Add -Z teach flag to provide extended diagnostic help #47652

Add -Z teach flag to provide extended diagnostic help #47652

Conversation

estebank commented Jan 22, 2018

estebank commented Jan 22, 2018

nikomatsakis commented Jan 22, 2018

nikomatsakis commented Jan 22, 2018

estebank commented Jan 22, 2018

estebank commented Jan 22, 2018

nikomatsakis commented Jan 22, 2018

nikomatsakis commented Jan 22, 2018

zackmdavis Jan 22, 2018 • edited Loading

Choose a reason for hiding this comment

zackmdavis commented Jan 22, 2018

estebank commented Jan 23, 2018 • edited Loading

nikomatsakis commented Jan 23, 2018

nikomatsakis Jan 23, 2018

Choose a reason for hiding this comment

estebank Jan 23, 2018

Choose a reason for hiding this comment

nikomatsakis Jan 24, 2018 • edited Loading

Choose a reason for hiding this comment

estebank Jan 24, 2018

Choose a reason for hiding this comment

estebank Jan 24, 2018

Choose a reason for hiding this comment

nikomatsakis left a comment

Choose a reason for hiding this comment

nikomatsakis Jan 25, 2018

Choose a reason for hiding this comment

nikomatsakis Jan 25, 2018

Choose a reason for hiding this comment

nikomatsakis commented Jan 25, 2018

estebank commented Jan 25, 2018

bors commented Jan 25, 2018

Add `-Z teach` flag to provide extended diagnostic help #47652

Add `-Z teach` flag to provide extended diagnostic help #47652

zackmdavis Jan 22, 2018 •

edited

Loading

estebank commented Jan 23, 2018 •

edited

Loading

nikomatsakis Jan 24, 2018 •

edited

Loading