Add ‘help’ messages to rustc

[ftxqxd]

This adds ‘help’ diagnostic messages to rustc. This is used for anything that provides help to the user, particularly the --explain messages that were previously integrated into the relevant error message.

They look like this:

match.rs:10:13: 10:14 error: unreachable pattern [E0001]
match.rs:10             1 => {},
                        ^
match.rs:3:1: 3:38 note: in expansion of foo!
match.rs:7:5: 20:2 note: expansion site
match.rs:10:13: 10:14 help: pass `--explain E0001` to see a detailed explanation

(help is coloured cyan.) Adding these errors on a separate line stops the lines from being too long, as discussed in #16619.

[lilyball]

You should probably update src/compiletest/runtest.rs to match "help" lines in is_compiler_error_or_warning(), otherwise any "help" lines that are not matched by a //~ HELP comment will be silently ignored.

[ftxqxd]

@kballard They are intentionally ignored. That function, as the name suggests, only detects warnings and errors. I consider help messages to be a different kind of ‘note’, and notes are ignored by default, so I chose to do the same thing for ‘help’ messages. If ‘help’ messages were not ignored, //~ HELP lines would have to be added everywhere where an error with an explanation occurs. This would mean that in addition to the existing //~ ERROR pattern for detecting, for example, unreachable patterns, there’d have to be a separate //~ HELP pattern for detecting the pass --explain E0001`` message, which is undesirable because more //~ HELP comments would have to be added each time an `--explain` message gets added.

[lilyball]

Oh right, notes were ignored already. I missed that bit. In that case, carry on.

[lilyball]

Given that, LGTM, but I'd prefer someone else weigh in on whether this is an appropriate change before r+'ing.

[brson]

This looks to convert two messages: the error code message, and a single borrow check note. It's not clear to me what the criteria is for picking span_note over span_help. Are there other cases where this should be used?

@nikomatsakis You will probably have an opinion.

[lilyball]

My feeling is "help" is for "helpful suggestions" whereas "note" is for contextual information that may be useful in interpreting the error.

In light of that, the borrowck suggestion is really just a helpful suggestion rather than being contextual information.

[aturon]

@brson @kballard What's the next step on this PR?

[lilyball]

I'm still fine with it.

[brson]

I scanned the source for notes that might be candidates for help and found these:

• "having upstream crates all available in one format will likely make this go away"
• "consider removing this semicolon:"
• "consider changing this closure to take self by mutable reference"
• "closures behind references must be called via &mut"
• "type aliases cannot be used for traits"
• "call the method using the . syntax"
• "Did you mean to write: {} {{ /* fields */ }}?"
• "consider using an explicit lifetime parameter as shown: {}"
• "use "#[unsafe_destructor]" on the implementation to force the compiler to allow this"
• "add #![feature(default_type_params)] to the crate attributes to enable"
• "the trait {} must be implemented because it is required by {}"
• "the trait {} must be implemented for the cast to the object type {}"
• "the Copy trait is required because the repeated element will be copied"
• "all local variables must have a statically known size"
• "the left-hand-side of an assignment must have a statically known size"
• "structs must have a statically known size to be initialized"
• "cannot implement a destructor on a structure or enumeration that does not satisfy Send"
• "use "#[unsafe_destructor]" on the implementation to force the compiler to allow this"
• "the closure that captures {} requires that all captured variables implement the trait {}"
• "only the last field of a struct or enum variant may have a dynamically sized type"
• "did you mean &{}{}?"

... well, there are a lot.

Some notes combine both contextual info and suggestions and need to be separated:

• "{} moved here{} because it has type {}, which is moved by default (use ref to override)"
• "{} moved here{} because it has type {}, which is moved by default (use ref to override)"

[brson]

Thanks @P1start. Here's an issue for the followup #18126