Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Let library authors hook into rustc's suggestions #177

Closed
mejrs opened this issue Oct 4, 2022 · 2 comments
Closed

Let library authors hook into rustc's suggestions #177

mejrs opened this issue Oct 4, 2022 · 2 comments
Labels
major-change Major change proposal T-lang to-announce Not yet announced MCP proposals

Comments

@mejrs
Copy link

mejrs commented Oct 4, 2022

Proposal

This is a proposal to add an attribute that can be put on items to customize error messages that item is involved in. It will allow library authors to improve their error messages, and can eliminate (some of) the special casing of types currently present in the compiler's suggestion machinery. I would like to initially introduce this under an unstable feature and later allow use on stable Rust.

There is a proof of concept implementation. (it is just that - a concept - and needs bikeshedding and more work)

Motivation, use-cases, and solution sketches

Recently I've been submitting some PR's to rustc's diagnostics, and as a library author I would like to do similar things but can't. Rust has amazing and helpful error messages and I want to bring these to regular libraries too.

I propose that it takes the form of an attribute named #[diagnostic(..)]. It can be present on structs, fields, traits, methods and so on. This MCP serves mainly to propose this attribute itself.

For an example of what it could look like, the following api surface is implemented in the proof of concept (the poc has positional arguments but named arguments are also possible).

Types mismatches

Explains what to do when a type is mismatched. This is especially useful for typestate apis.

pub enum HttpResponse {
    // ...
}

#[diagnostic(confused_with(
    HttpResponse,
    "call `.body()` to turn this `HttpResponseWithoutBody` into an `HttpResponse`"
))]
pub struct HttpResponseWithoutBody {
    // ...
}

let f: HttpResponse = HttpResponseWithoutBody {};
error[E0308]: mismatched types
  --> $DIR/confused_with.rs:26:27
   |
LL |     let f: HttpResponse = HttpResponseWithoutBody {};
   |            ------------   ^^^^^^^^^^^^^^^^^^^^^^^^^^ expected enum `HttpResponse`, found struct `HttpResponseWithoutBody`
   |            |
   |            expected due to this
   |
   = help: call `.body()` to turn this `HttpResponseWithoutBody` into an `HttpResponse`

Missing trait implementations

Explains why a type doesn't implement a trait and/or suggest what to do instead. It is actually very helpful to communicate that a missing implementation is intentional, and not an oversight.

#[diagnostic(missing_impl(Copy, "Tokens are unique and may not be duplicated"))]
pub struct Token(());

fn copy<T: Copy>() {}


copy::<Token>();
error[E0277]: Tokens are unique and may not be duplicated
  --> $DIR/cannot_copy.rs:7:9
   |
LL |     c::<Token>();
   |         ^^^^^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.

You can see where this goes; it can be extended to do much more.

Isn't this just rustc_on_unimplemented?

Its problem is that we like it but don't love it. We want "something like it" but not it. This is a much more general mechanism - it can be applied everywhere, not just on trait definitions.

(Part of implementing #[diagnostic] would be to extend it to do what rustc_on_unimplemented does)

What about stability? Should it be "unstable on stable" like this post suggests?

The process of improving user diagnostics seems to deadlock easily:

  • Library authors want to improve the errors their libraries emit
  • The Rust teams want this also but don't know what library authors want, so don't want to commit to stabilizing things
  • The vast majority of users are on stable Rust, so experimentation on nightly reaches few users.
  • Few authors experiment on nightly and get no or little user feedback
  • We don't know what works well, so nothing gets stabilized.

Allowing experimentation on stable is one way to break that cycle. I think the best way forward is to do this through lints - require library authors to explicitly opt in. Perhaps something like #![allow(unstable_diagnostics)]. We can make this more granular, like "we're probably going to rework this" to "we're happy with this and are unlikely to change it backwards-incompatibly, but reserve the right" to "we're happy and won't break this".

As we evolve it, existing usage might break. If this happens the attribute will emit a (crate local) lint, and simply do nothing if invoked. This is a best-effort thing; if it fails to parse then the normal diagnostics will be emitted, which is a vast improvement over the status quo.

We can eventually stabilize parts that we are happy with, for which opt-in would no longer be necessary. Or perhaps parts can be split off and stabilized as something else.

Have you seen Iterator's source code? How do you propose to prevent a mess like that?

The current state of rustc_on_unimplemented is pretty bad. I propose to allow this information to live in another place, using include! :

#[diagnostic = include!("diagnostics/iterator_trait.txt")]
pub trait Iterator {
    // ...
}

This can even support localization:

#[diagnostic = include!(concat!("diagnostics/", env!("LOCALE"), "/iterator_trait.txt"))]
pub trait Iterator {
    // ...
}

This include! would need to be fallible - I'm not sure if that can be done.

Summary

To summarize, this MCP proposes the following:

  • Should the #[diagnostic] attribute be implemented?
  • Should the "mismatched_types" api as above be implemented? (perhaps with changes)
  • Should the "missing_impl" api as above be implemented? (perhaps with changes)

There are also open questions:

  • If accepted, what is the process for adding new api to it?
  • Should it be kinda-unstably?

Links and related work

Initial people involved

  • Owner, if known: I would like to work on this.
  • Liaison

What happens now?

This issue is part of the lang-team initiative process. Once this issue is filed, a Zulip topic will be opened for discussion, and the lang-team will review open proposals in its weekly triage meetings. You should receive feedback within a week or two.

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

@mejrs mejrs added major-change Major change proposal T-lang labels Oct 4, 2022
@rustbot
Copy link
Collaborator

rustbot commented Oct 4, 2022

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

@rustbot rustbot added the to-announce Not yet announced MCP proposals label Oct 4, 2022
@nikomatsakis
Copy link
Contributor

I'm going to close this issue, as the lang team recently adopted a new process for managing ideas and we're moving away from MCPs. However, I think this is covering a really important point, and I do want to encourage continued conversation. I see that the issue discussed the idea of doing in-tree experimentation: we have a process for that, but it does require an experienced Rust contributor to serve as the owner of the idea (as well as a lang-team liaison). In this instance though I know there are a number of people interested in this direction so that may be a viable path forward!

Thank you very much for opening the issue and apologies for the process confusion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major-change Major change proposal T-lang to-announce Not yet announced MCP proposals
Projects
None yet
Development

No branches or pull requests

3 participants