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

Remove err methods #42887

Merged
merged 4 commits into from
Jun 29, 2017
Merged

Remove err methods #42887

merged 4 commits into from
Jun 29, 2017

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Jun 24, 2017

To be merged after #42519.

cc @Susurrus @QuietMisdreavus

@rust-highfive
Copy link
Collaborator

r? @eddyb

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

@eddyb
Copy link
Member

eddyb commented Jun 24, 2017

r? @nikomatsakis

@rust-highfive rust-highfive assigned nikomatsakis and unassigned eddyb Jun 24, 2017
@Mark-Simulacrum Mark-Simulacrum added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 24, 2017
Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a few suggestions for the wording. What do you think? My sense is that the extended descriptions are a good place to offer a bit more background information.

Erroneous code example:

```compile_fail,E0619
let x;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This example is simple, but it does conflate an uninitialized value with one where we do not know the type. An alternative might be something like this:

let mut x = vec![];
match x.pop() {
    Some(v) => {
        // Here, the type of `v` is not (yet) known, so we
        // cannot resolve this method call:
        v.to_uppercase();
    }
    None => { }
}

This could be fixed by annotating x with a type, like x: Vec<String>. To make it more realistic, we'd need a loop or something else.

Another example that I encounter a lot:

let vec1 = vec![1, 2, 3];
let vec2 = vec1.iter().map(|x| x * 2).collect();
println!("{}", vec2.len()); // error

with the fix:

let vec1 = vec![1, 2, 3];
let vec2: Vec<_>  = vec1.iter().map(|x| x * 2).collect();
println!("{}", vec2.len()); // error

the problem with this one, of course, is that it references the iterator API. But we could probably explain it just by saying that "because collect() can create many kinds of collections, the type is typically inferred from context." or something like that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wo, this example is way better. Great idea!

@@ -4567,6 +4567,52 @@ i_am_a_function();
```
"##,

E0619: r##"
A not (yet) known type was used.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could say a bit more here. This message would perhaps be mildly easier to write if we knew what operation was giving it, but anyhow. (As an aside, this is (yet another...) of those messages that would be greatly improved by a "context sensitive" --explain mechanism, I think).

The type-checker needed to know the type of an expression,
but that type had not yet been inferred. This is most often
fixed by adding a type annotation.

We could go on to give more context, but it may be overkill:

Type inference typically proceeds from the top of the
function to the bottom, figuring out types as it goes.
In some cases -- notably method calls and overloadable
operators like `*` -- the type checker may not have enough
information *yet* to make progress. This can be true even if the
rest of the function provides enough context (because the type-checker
hasn't looked that far ahead yet). In this case, type annotations can be
used to help it along. 

Copy link
Member Author

@GuillaumeGomez GuillaumeGomez Jun 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first sentence isn't supposed to be an explanation, just a short recap of the error. However this explanation could replace the current one since it's (way) more complete.

// as `[usize]`
```

In Rust, some types don't have a size at compile-time (like slices and traits
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe something like this?

In Rust, some types don't have a don't have a known size at compile-time.
For example, in a slice type like [u32], the number of elements is not known
at compile-time and hence the overall size cannot be computed. As a result,
such types can only be manipulated through a reference (e.g., &T or &mut T)
or other pointer-type (e.g., Box or Rc). Try casting to a reference instead:

let x = &[1_usize, 2] as &[usize]; // ok!

@GuillaumeGomez
Copy link
Member Author

Now that the previous one has been merged, I just need to make this one works.

@GuillaumeGomez
Copy link
Member Author

Updated.

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Jun 28, 2017

tidy check (x86_64-unknown-linux-gnu)
[00:02:00] tidy error: duplicate error code: 619
[00:02:00] tidy error: /checkout/src/librustc_typeck/diagnostics.rs:4668: E0619: r##"
[00:02:00] tidy error: /checkout/src/librustc_typeck/diagnostics.rs:4800:     E0619, // intrinsic must be a 

r=me when fixed

@GuillaumeGomez
Copy link
Member Author

@bors: r=nikomatsakis

@bors
Copy link
Contributor

bors commented Jun 28, 2017

📌 Commit 03eb963 has been approved by nikomatsakis

@GuillaumeGomez
Copy link
Member Author

@bors: r-

@GuillaumeGomez
Copy link
Member Author

@bors: r=nikomatsakis

@bors
Copy link
Contributor

bors commented Jun 29, 2017

📌 Commit 5acc1de has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Jun 29, 2017

⌛ Testing commit 5acc1de with merge d0e0f53...

bors added a commit that referenced this pull request Jun 29, 2017
@bors
Copy link
Contributor

bors commented Jun 29, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing d0e0f53 to master...

@bors bors merged commit 5acc1de into rust-lang:master Jun 29, 2017
@GuillaumeGomez GuillaumeGomez deleted the remove-err-methods branch June 29, 2017 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants