Skip to content

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Jul 1, 2017

Add support to rustc_on_unimplemented to reference the full path of
the annotated trait. For the following code:

pub mod Bar {
    #[rustc_on_unimplemented = "test error `{Self}` with `{Bar}` `{Baz}` `{Quux}` in `{Foo}`"]
    pub trait Foo<Bar, Baz, Quux> {}
}

the error message will be:

test error `std::string::String` with `u8` `_` `u32` in `Bar::Foo`

@rust-highfive
Copy link
Contributor

r? @pnkfelix

(rust_highfive has picked a reviewer for you, use r? to override)

@estebank
Copy link
Contributor Author

estebank commented Jul 1, 2017

Originally found that this would be a nice addition when working on #43001, to make sure that the path of the trait is always correct on the error message.

@Mark-Simulacrum
Copy link
Member

[00:03:27] Expected a gate test for the feature 'on_unimplemented'.
[00:03:27] Hint: create a file named 'feature-gate-on_unimplemented.rs' in the compile-fail
[00:03:27]       test suite, with its failures due to missing usage of
[00:03:27]       #![feature(on_unimplemented)].
[00:03:27] Hint: If you already have such a test and don't want to rename it,
[00:03:27]       you can also add a // gate-test-on_unimplemented line to the test file.
[00:03:27] tidy error: Found 1 features without a gate test.
[00:03:27] some tidy checks failed
[00:03:27] 
[00:03:27] 
[00:03:27] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools/x86_64-unknown-linux-gnu/release/tidy" "/checkout/src" "--no-vendor" "--quiet"
[00:03:27] expected success, got: exit code: 1
[00:03:27] 
[00:03:27] 
[00:03:27] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:03:27] Build completed unsuccessfully in 0:01:00
[00:03:27] make: *** [tidy] Error 1
[00:03:27] Makefile:74: recipe for target 'tidy' failed

@Mark-Simulacrum Mark-Simulacrum added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 2, 2017
@aidanhs aidanhs added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 5, 2017
@estebank estebank force-pushed the on-unimplemented-path branch from 1eb6669 to a43654c Compare July 5, 2017 18:25
@aidanhs
Copy link
Contributor

aidanhs commented Jul 5, 2017

@estebank looks like the tidy failure is still waiting on you? Might be worth getting it fixed before review in case it reveals any other test failures.

@aidanhs
Copy link
Contributor

aidanhs commented Jul 5, 2017

Oh wait, you just pushed a change :)

@aidanhs
Copy link
Contributor

aidanhs commented Jul 5, 2017

Some other test failures have been revealed

[01:09:26] error[E0232]: this attribute must have a value
[01:09:26]   --> $DIR/bad-annotation.rs:26:1
[01:09:26]    |
[01:09:26] 26 | #[rustc_on_unimplemented] //~ ERROR this attribute must have a value
[01:09:26]    | ^^^^^^^^^^^^^^^^^^^^^^^^^ attribute requires a value
[01:09:26]    |
[01:09:26]    = note: eg `#[rustc_on_unimplemented = "foo"]`
[01:09:26] 
[01:09:26] error[E0230]: there is no type parameter C on trait BadAnnotation2
[01:09:26]   --> $DIR/bad-annotation.rs:30:1
[01:09:26]    |
[01:09:26] 30 | #[rustc_on_unimplemented = "Unimplemented trait error on `{Self}` with params `<{A},{B},{C}>`"]
[01:09:26]    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[01:09:26] 
[01:09:26] error[E0231]: only named substitution parameters are allowed
[01:09:26]   --> $DIR/bad-annotation.rs:35:1
[01:09:26]    |
[01:09:26] 35 | #[rustc_on_unimplemented = "Unimplemented trait error on `{Self}` with params `<{A},{B},{}>`"]
[01:09:26]    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[01:09:26] 
[01:09:26] error: aborting due to 3 previous errors

Add support to `rustc_on_unimplemented` to reference the full path of
the annotated trait. For the following code:

```rust
pub mod Bar {
    #[rustc_on_unimplemented = "test error `{Self}` with `{Bar}` `{Baz}` `{Quux}` in `{Foo}`"]
    pub trait Foo<Bar, Baz, Quux> {}
}
```

the error message will be:

```
test error `std::string::String` with `u8` `_` `u32` in `Bar::Foo`
```
@estebank estebank force-pushed the on-unimplemented-path branch from a43654c to 05d3526 Compare July 5, 2017 23:47
@estebank
Copy link
Contributor Author

estebank commented Jul 6, 2017

r? @aturon

@rust-highfive rust-highfive assigned aturon and unassigned pnkfelix Jul 6, 2017
@arielb1
Copy link
Contributor

arielb1 commented Jul 11, 2017

r? @arielb1

I'll try to review this later today.

@arielb1
Copy link
Contributor

arielb1 commented Jul 11, 2017

I don't think rustc::traits should be having an error reporting logic distinct from typeck, but let's leave that refactoring to a separate PR.

@arielb1
Copy link
Contributor

arielb1 commented Jul 11, 2017

LGTM

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 11, 2017

📌 Commit 05d3526 has been approved by arielb1

@arielb1
Copy link
Contributor

arielb1 commented Jul 11, 2017

@bors rollup

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Jul 12, 2017
…rielb1

`rustc_on_unimplemented` supports referring to trait

Add support to `rustc_on_unimplemented` to reference the full path of
the annotated trait. For the following code:

```rust
pub mod Bar {
    #[rustc_on_unimplemented = "test error `{Self}` with `{Bar}` `{Baz}` `{Quux}` in `{Foo}`"]
    pub trait Foo<Bar, Baz, Quux> {}
}
```

the error message will be:

```
test error `std::string::String` with `u8` `_` `u32` in `Bar::Foo`
```
bors added a commit that referenced this pull request Jul 12, 2017
Rollup of 8 pull requests

- Successful merges: #42670, #42826, #43000, #43011, #43098, #43100, #43136, #43137
- Failed merges:
@bors bors merged commit 05d3526 into rust-lang:master Jul 12, 2017
bors added a commit that referenced this pull request Jul 28, 2017
Use `rustc_on_unimplemented`'s trait name argument in `try`

Follow up to #43000 and #43001. Fix #42694.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants