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

E0608 #42568

Merged
merged 2 commits into from
Jun 16, 2017
Merged

E0608 #42568

merged 2 commits into from
Jun 16, 2017

Conversation

GuillaumeGomez
Copy link
Member

Part of #42229.

cc @Susurrus

@rust-highfive
Copy link
Collaborator

r? @arielb1

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

@@ -4095,6 +4095,27 @@ assert_eq!(!Question::No, true);
```
"##,

E0608: r##"
An attempt to get an index from a type which doesn't implement the `Index` was
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't really getting an index, at least in the eyes of the user, is it? It's more trying to index into the type. Maybe rephrase this as "An attempt to index into a type which doesn't implement the Index trait was performed." Maybe it actually makes sense to use std::ops::Index instead of Index as well.

},
base_t);
let mut err = struct_span_err!(tcx.sess, expr.span, E0608,
"cannot index a value of type `{}`",
Copy link
Contributor

Choose a reason for hiding this comment

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

I would use "cannot index into a value of type {}" instead

Copy link
Member Author

Choose a reason for hiding this comment

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

That's not the into the problem, that's the index. I think switching the message to this would make the error lose its meaning.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is separate than the Into trait stuff. I've always heard indexing described as being into something, like you "index into an array", you don't "index an array". The latter sounds weird to me. With the current PR you use "index into" in the detailed explanation, but the short error title is still "index a value". This should at least be consistent.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, let me rewrite this then.

@GuillaumeGomez
Copy link
Member Author

Updated.

},
base_t);
let mut err = struct_span_err!(tcx.sess, expr.span, E0608,
"cannot index a value of type `{}`",
Copy link
Contributor

Choose a reason for hiding this comment

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

This is separate than the Into trait stuff. I've always heard indexing described as being into something, like you "index into an array", you don't "index an array". The latter sounds weird to me. With the current PR you use "index into" in the detailed explanation, but the short error title is still "index a value". This should at least be consistent.

@Mark-Simulacrum Mark-Simulacrum added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jun 11, 2017
@GuillaumeGomez
Copy link
Member Author

Updated.

@bors
Copy link
Contributor

bors commented Jun 12, 2017

☔ The latest upstream changes (presumably #42585) made this pull request unmergeable. Please resolve the merge conflicts.

@GuillaumeGomez
Copy link
Member Author

Updated as well. cc @Susurrus

0u8[2]; // error: cannot index into a value of type `u8`
```

To be able to index a value from a type, it needs to implement the
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead "To be able to index into a type it needs to implement ..."

@GuillaumeGomez
Copy link
Member Author

And updated.

cc @frewsxcv @QuietMisdreavus

@QuietMisdreavus
Copy link
Member

Looks good!

@bors r+

@bors
Copy link
Contributor

bors commented Jun 15, 2017

📌 Commit b6e9ed1 has been approved by QuietMisdreavus

@bors
Copy link
Contributor

bors commented Jun 16, 2017

⌛ Testing commit b6e9ed1 with merge a311496...

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

bors commented Jun 16, 2017

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

@bors bors mentioned this pull request Jun 16, 2017
@bors bors merged commit b6e9ed1 into rust-lang:master Jun 16, 2017
@GuillaumeGomez GuillaumeGomez deleted the E0608 branch June 18, 2017 16:05
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.

7 participants