Make diagnostics emitting independent of the happy code path

[oli-obk]

In our first @rust-lang/wg-diagnostics meeting on zulip we figured that a first start would be to create a new trait

trait AsError {
    /// ParseSess, Session or `TyCtxt`
    type Session;
    fn to_error(self, session: Self::Session) -> DiagnosticBuilder;
    fn emit(self) {
        self.to_error().emit()
    }
}

that is implemented for structs containing all relevant info for a diagnostic, e.g.

struct TypeError {
    expected: Ty,
    found: Ty,
    span: Span,
}

and then the code that currently constructs the diagnostic and emits it would just become

TypeError {
    expected,
    found,
    span: Span,
}.emit();

This issue has been assigned to @jumbatm via this comment.

[Manishearth]

The vast majority of these could probably be handled by a custom derive:

#[derive(AsError)]
#[error = "Cannot move {ty} out of borrow", span]
#[note = "First borrwed here", other_span]
#[code = E12345]
struct MoveOutOfBorrowError {
    ty: Ty,
    span: Span,
    other_span: Span,
}

(exact syntax is bikesheddable, of course)

[estebank]

At first I thought about how the derive macro would be constraining for some of the more complex errors, but looking at it I can see how it'd be more than appropriate for the great majority of errors we emit:

#[derive(AsError)]
#[code = E12345]
struct MoveOutOfBorrowError {
    ty: Ty,
    #[error = "cannot move {ty} out of borrow"]
    #[label = "cannot move out of borrow"]
    span: Span,
    #[label = "`{ty}` first borrowed here"]
    other_span: Span,
    #[suggestion(msg = "consider cloning here", code = "{opt_sugg}.clone()")]
    opt_sugg: Option<Span>,
}

[Manishearth]

Oh, tagging spans with this stuff is genius.

[Manishearth]

Hmm, it needs a little tweak:

#[derive(AsError)]
#[code = E12345]
struct MoveOutOfBorrowError {
    ty: Ty,
    #[error = "cannot move {ty} out of borrow"]
    #[label = "cannot move out of borrow"]
    span: Span,
    #[label = "`{ty}` first borrowed here"]
    other_span: Span,
    #[suggestion(msg = "consider cloning here", code = "{1}.clone()")]
    opt_sugg: Option<(Span, Ident)>,
}

[Manishearth]

So here we'd have to special case Ty, Span, and Option. Not too bad.

[Electron-libre]

This sounds like a great improvment in typeck maintainability.
(And probably a lot of work too)
It could be a good opportunity for me to pratice custom derives.

I think I can do this, It will just take time and trials.

What do you think ?

[Manishearth]

Go for it, I think. cc @estebank

The derive will basically have to specially notice Tys and Spans and some other stuff, though. Getting the suggestions working is tricky too.

[estebank]

I think we can iterate and disregard the more complex parts of the feature (IE, implement them manually in the code for now) and later pull them into the proc macro.

[Mark-Simulacrum]

@Electron-libre Are you still working on this? If so please @rustbot claim this issue, thanks!

[Electron-libre]

No sorry, my son is born ( 😍 ) few days after my last comment and then I forgot about this issue.
I will not invest time in OSS contributions the next months.
Someone else should pick this issue.

[Manishearth]

I and @yaahc may end up working on this