rust-lang / rust-lang/rust

Allow `#[diagnostic::on_unimplemented]` on `impl`s, specially inherent `impl`s

Open
#151,796 3 comments 4 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

A-diagnostics D-diagnostic-infra F-on_unimplemented T-compiler
Dominant language
Rust
Stars
119k
Forks
16.2k
PR merge metrics
PR metrics pending

Description

While working on a derived builder using const params to track fields being set/unset, I noticed that it would be useful to be able to customize the output to explain in users' terms why the method isn't being found:

error[E0599]: no method named `build` found for struct `PartialTest<false, true>` in the current scope
  --> tests/no_compile/missing_fields_in_builder.rs:13:33
   |
 3 | #[bitfield(u32, default = 0, forbid_overlaps)]
   | ---------------------------------------------- method `build` not found for this struct
...
13 |     Test::builder().with_bar(1).build();
   |                                 ^^^^^ method not found in `PartialTest<false, true>`
   |
   = note: the method was found for
           - `PartialTest<true, true>`

error[E0599]: no method named `with_bar` found for struct `PartialTest<true, true>` in the current scope
  --> tests/no_compile/missing_fields_in_builder.rs:15:45
   |
 3 | #[bitfield(u32, default = 0, forbid_overlaps)]
   | ---------------------------------------------- method `with_bar` not found for this struct
...
15 |     Test::builder().with_bar(1).with_foo(1).with_bar(2).build();
   |                                             ^^^^^^^^ method not found in `PartialTest<true, true>`
   |
   = note: the method was found for
           - `PartialTest<foo, false>`
help: one of the expressions' fields has a method of the same name
   |
15 |     Test::builder().with_bar(1).with_foo(1).value.with_bar(2).build();
   |                                             ++++++

A slightly better output with customization could be:

error[E0599]: all fields must be initialized before `Test` can be built
  --> tests/no_compile/missing_fields_in_builder.rs:13:33
   |
 3 | #[bitfield(u32, default = 0, forbid_overlaps)]
   | ---------------------------------------------- method `build` not found for this struct
...
13 |     Test::builder().with_bar(1).build();
   |                                 ^^^^^ method not found in `PartialTest<false, true>`
   |
   = note: no method named `build` found for struct `PartialTest<false, true>` in the current scope
   = note: the method was found for
           - `PartialTest<true, true>`

error[E0599]: the same field can't be set twice
  --> tests/no_compile/missing_fields_in_builder.rs:15:45
   |
 3 | #[bitfield(u32, default = 0, forbid_overlaps)]
   | ---------------------------------------------- method `with_bar` not found for this struct
...
15 |     Test::builder().with_bar(1).with_foo(1).with_bar(2).build();
   |                                             ^^^^^^^^ method can't be called twice
   |
   = note: no method named `with_bar` found for struct `PartialTest<true, true>` in the current scope
   = note: method not found in `PartialTest<true, true>`
   = note: the method was found for
           - `PartialTest<foo, false>`
help: one of the expressions' fields has a method of the same name
   |
15 |     Test::builder().with_bar(1).with_foo(1).value.with_bar(2).build();
   |                                             ++++++

This could be accomplished with:

#[diagnostic::on_unimplemented(message = "all fields must be initialized before `Test` can be built")]
impl PartialTest<true, true> {
    fn build(self) -> Test { .. }
}
#[diagnostic::on_unimplemented(message = "the same field can't be set twice", label = "method can't be called twice")]
impl<const foo: bool> PartialTest<foo, false> {
    fn with_bar(self) -> PartialTest<foo, true> { .. }
}

Ideally, it would be the following, but giving enough hooks to allow for it might be too much:

error[E0599]: all fields must be initialized before `Test` can be built
  --> tests/no_compile/missing_fields_in_builder.rs:13:33
   |
13 |     Test::builder().with_bar(1).build();
   |                                 ^^^^^ `foo` wasn't initalized
   |
   = note: method `with_foo` must be called on the builder first

error[E0599]: the same field can't be set twice
  --> tests/no_compile/missing_fields_in_builder.rs:15:45
   |
15 |     Test::builder().with_bar(1).with_foo(1).with_bar(2).build();
   |                                             ^^^^^^^^ method can't be called twice

but the API to accomplish that could get unwieldy

#[diagnostic::on_unimplemented(
    message = "all fields must be initialized before `Test` can be built",
    on(Self = "PartialTest<true, false>", label = "`foo` wasn't initialized"),
    on(Self = "PartialTest<false, true>", label = "`bar` wasn't initialized"),
    on(Self = "PartialTest<false, false>", label = "`foo` and `bar` weren't initialized"),
    drop_original_message,
    drop_original_label,
    drop_found_list,
    dont_point_at_type
)]
impl PartialTest<true, true> {
    fn build(self) -> Test { .. }
}
#[diagnostic::on_unimplemented(
    message = "the same field can't be set twice",
    label = "method can't be called twice",
    drop_original_message,
    drop_original_label,
    drop_found_list,
    dont_suggest,
    dont_point_at_type
)]
impl<const foo: bool> PartialTest<foo, false> {
    fn with_bar(self) -> PartialTest<foo, true> { .. }
}

Some of this customization could be accomplished by creating a trait for each builder and annotating those, but that would be an arbitrary restriction of the diagnostic machinery, which would also cause projects to pay additional compilation cost purely to customize the diagnostics.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the tests/no_compile/missing_fields_in_builder.rs example and compare the current diagnostics with the proposed #[diagnostic::on_unimplemented] forms. Trace the compiler's existing diagnostic customization machinery, then define and validate a coherent approach for applying it to inherent and other impls without requiring artificial traits.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
compilers
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.