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

Improve errors after re rebalance coherence #66253

Merged

Conversation

@ohadravid
Copy link
Contributor

ohadravid commented Nov 9, 2019

Following #65247, I noticed that some error messages should be updated to reflect the changes of re_rebalance_coherence (also there was a note in the RFC about it).

First, error message E0210 was updated to match the RFC, and I also tried to improve a little the error when the "order" of types is problematic.

For code like this:

#![feature(re_rebalance_coherence)] // Now stable

struct Wrap<T>(T);

impl<T> From<Wrap<T>> for T {
    fn from(x: Wrap<T>) -> T {
        x.0
    }
}

The old error was:

error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct<T>`)
 --> src/lib.rs:5:6
  |
5 | impl<T> From<Wrap<T>> for T {
  |      ^ type parameter `T` must be used as the type parameter for some local type
  |
  = note: only traits defined in the current crate can be implemented for a type parameter

and the new error is:

error[E0210]: type parameter `T` must be covered by another type when it appears before the first local type (`Wrap<T>`)
  --> main.rs:66:6
   |
66 | impl<T> From<Wrap<T>> for T {
   |      ^ type parameter `T` must be covered by another type when it appears before the first local type (`Wrap<T>`)
   |
   = note: implementing a foreign trait is only possible if at least one of the types for which is it implemented is local, and no uncovered type parameters appear before that first local type
   = note: in this case, 'before' refers to the following order: `impl<..> ForeignTrait<T1, ..., Tn> for T0`, where `T0` is the first and `Tn` is the last

I tried to point at the uncovered T, but couldn't get something which was reliable (but I'll be happy to try if someone points me in the right direction).

r? @estebank
cc @nikomatsakis

Fixes #65247

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Nov 9, 2019

Some changes occurred in diagnostic error codes

cc @GuillaumeGomez

src/librustc_typeck/coherence/orphan.rs Outdated Show resolved Hide resolved
src/librustc_typeck/coherence/orphan.rs Outdated Show resolved Hide resolved
@ohadravid ohadravid force-pushed the ohadravid:improve-errors-after-re-rebalance-coherence branch 3 times, most recently from 6cea880 to a5a8410 Nov 10, 2019
@ohadravid ohadravid force-pushed the ohadravid:improve-errors-after-re-rebalance-coherence branch from a5a8410 to 64cf59d Nov 10, 2019
@rust-highfive

This comment was marked as resolved.

Copy link
Collaborator

rust-highfive commented Nov 10, 2019

The job x86_64-gnu-llvm-6.0 of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
2019-11-10T14:00:24.1126700Z ##[command]git remote add origin https://github.com/rust-lang/rust
2019-11-10T14:00:24.1369736Z ##[command]git config gc.auto 0
2019-11-10T14:00:24.1460322Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2019-11-10T14:00:24.1525609Z ##[command]git config --get-all http.proxy
2019-11-10T14:00:24.1674062Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/66253/merge:refs/remotes/pull/66253/merge
---
2019-11-10T14:06:44.8867025Z    Compiling serde_json v1.0.40
2019-11-10T14:06:45.9749407Z    Compiling tidy v0.1.0 (/checkout/src/tools/tidy)
2019-11-10T14:06:56.9164790Z     Finished release [optimized] target(s) in 1m 24s
2019-11-10T14:06:56.9201009Z tidy check
2019-11-10T14:06:58.1630535Z tidy error: /checkout/src/librustc_typeck/error_codes.rs:2130: line longer than 80 chars
2019-11-10T14:06:59.7165803Z Found 485 error codes
2019-11-10T14:06:59.7165949Z Found 0 error codes with no tests
2019-11-10T14:06:59.7166031Z Done!
2019-11-10T14:06:59.7166080Z some tidy checks failed
2019-11-10T14:06:59.7166080Z some tidy checks failed
2019-11-10T14:06:59.7166149Z 
2019-11-10T14:06:59.7166179Z 
2019-11-10T14:06:59.7167117Z command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor"
2019-11-10T14:06:59.7167255Z 
2019-11-10T14:06:59.7167284Z 
2019-11-10T14:06:59.7170021Z failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
2019-11-10T14:06:59.7170403Z Build completed unsuccessfully in 0:01:28
2019-11-10T14:06:59.7170403Z Build completed unsuccessfully in 0:01:28
2019-11-10T14:06:59.7212346Z == clock drift check ==
2019-11-10T14:06:59.7232881Z   local time: Sun Nov 10 14:06:59 UTC 2019
2019-11-10T14:06:59.7938197Z   network time: Sun, 10 Nov 2019 14:06:59 GMT
2019-11-10T14:06:59.7938287Z == end clock drift check ==
2019-11-10T14:07:01.2616504Z 
2019-11-10T14:07:01.2733235Z ##[error]Bash exited with code '1'.
2019-11-10T14:07:01.2769031Z ##[section]Starting: Checkout
2019-11-10T14:07:01.2771159Z ==============================================================================
2019-11-10T14:07:01.2771246Z Task         : Get sources
2019-11-10T14:07:01.2771317Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@ohadravid ohadravid force-pushed the ohadravid:improve-errors-after-re-rebalance-coherence branch from 64cf59d to 2db744c Nov 10, 2019
@estebank

This comment has been minimized.

Copy link
Contributor

estebank commented Nov 14, 2019

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Nov 14, 2019

📌 Commit 2db744c has been approved by estebank

JohnTitor added a commit to JohnTitor/rust that referenced this pull request Nov 14, 2019
…ebalance-coherence, r=estebank

Improve errors after re rebalance coherence

Following rust-lang#65247, I noticed that some error messages should be updated to reflect the changes of `re_rebalance_coherence` (also there was a [note](https://rust-lang.github.io/rfcs/2451-re-rebalancing-coherence.html#teaching-users) in the RFC about it).

First, error message `E0210` was updated to match the RFC, and I also tried to improve a little the error when the "order" of types is problematic.

For code like this:
```
#![feature(re_rebalance_coherence)] // Now stable

struct Wrap<T>(T);

impl<T> From<Wrap<T>> for T {
    fn from(x: Wrap<T>) -> T {
        x.0
    }
}
```

The old error was:
```
error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct<T>`)
 --> src/lib.rs:5:6
  |
5 | impl<T> From<Wrap<T>> for T {
  |      ^ type parameter `T` must be used as the type parameter for some local type
  |
  = note: only traits defined in the current crate can be implemented for a type parameter

```

and the new error is:
```
error[E0210]: type parameter `T` must be covered by another type when it appears before the first local type (`Wrap<T>`)
  --> main.rs:66:6
   |
66 | impl<T> From<Wrap<T>> for T {
   |      ^ type parameter `T` must be covered by another type when it appears before the first local type (`Wrap<T>`)
   |
   = note: implementing a foreign trait is only possible if at least one of the types for which is it implemented is local, and no uncovered type parameters appear before that first local type
   = note: in this case, 'before' refers to the following order: `impl<..> ForeignTrait<T1, ..., Tn> for T0`, where `T0` is the first and `Tn` is the last
```

I tried to point at the uncovered `T`, but couldn't get something which was reliable (but I'll be happy to try if someone points me in the right direction).

r? @estebank
cc @nikomatsakis

Fixes rust-lang#65247
bors added a commit that referenced this pull request Nov 14, 2019
Rollup of 9 pull requests

Successful merges:

 - #66253 (Improve errors after re rebalance coherence)
 - #66264 (fix an ICE in macro's diagnostic message)
 - #66349 (expand source_util macros with def-site context)
 - #66351 (Tweak non-char/numeric in range pattern diagnostic)
 - #66360 (Fix link to Exten in Vec::set_len)
 - #66361 (parser: don't use `unreachable!()` in `fn unexpected`.)
 - #66363 (Improve error message in make_tests)
 - #66369 (compiletest: Obtain timestamps for common inputs only once)
 - #66372 (Fix broken links in Ipv4Addr::is_benchmarking docs)

Failed merges:

r? @ghost
@bors bors merged commit 2db744c into rust-lang:master Nov 14, 2019
4 checks passed
4 checks passed
pr Build #20191110.30 succeeded
Details
pr (Linux mingw-check) Linux mingw-check succeeded
Details
pr (Linux x86_64-gnu-llvm-6.0) Linux x86_64-gnu-llvm-6.0 succeeded
Details
pr (Linux x86_64-gnu-tools) Linux x86_64-gnu-tools succeeded
Details
@ohadravid ohadravid deleted the ohadravid:improve-errors-after-re-rebalance-coherence branch Nov 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.