-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Add E0609 #42585
Add E0609 #42585
Conversation
r? @pnkfelix (rust_highfive has picked a reviewer for you, use r? to override) |
src/librustc_typeck/diagnostics.rs
Outdated
@@ -4095,6 +4095,33 @@ assert_eq!(!Question::No, true); | |||
``` | |||
"##, | |||
|
|||
E0609: r##" | |||
An attempt to get a field which doesn't exist was performed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe "Attempted to access a non-existent field in a struct" reads better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about "An attempt to access a non-existent field in a struct"?
src/librustc_typeck/diagnostics.rs
Outdated
println!("{}", s.foo); // error: no field `foo` on type `StructWithFields` | ||
``` | ||
|
||
To fix this error, check that if you didn't mispell the field's name or that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"misspell"
Remove the "if" as well.
Updated. |
I really don't like the phrasing "An attempt ... was performed". It reads awkwardly. I think using "Attempted to ..." reads better overall. Additionally it appears that this error covers two situations: One where they improperly use field syntax on a type (currently non-structs-or-tuples) and another error when the struct does not contain the given field. And my understanding is that error is separate from when that field is private as well. I think these errors would be more clear if E0609 was split into two errors: one for when it's invalid for the type to have field access performed on it and another for when a struct or tuple doesn't have the field being referenced. |
I splitted the error into two. |
src/librustc_typeck/diagnostics.rs
Outdated
println!("{}", s.foo); // error: no field `foo` on type `StructWithFields` | ||
``` | ||
|
||
To fix this error, check if you didn't misspell the field's name or that the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"check that you didn't misspell the field's name or that the field actually exists"
src/librustc_typeck/diagnostics.rs
Outdated
|
||
```compile_fail,E0610 | ||
let x: u32 = 0; | ||
println!("{}", x.foo); // error: `{interger}` is a primitive type, therefore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"integer"
src/librustc_typeck/diagnostics.rs
Outdated
``` | ||
|
||
Primitive types are the most basic types available in Rust and don't have | ||
fields. If you want to use fields, take a look to the structs. Example: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know how the user would accidentally access a field of a variable when coding. I would think the most likely reason would be they a) didn't know the datatype of the variable they used or b) accidentally used the wrong variable. I don't know if suggesting the user looks at structs will be helpful in them resolving this error. I think this extra description may be unnecessary.
In case you want to keep it, I'd reword the second sentence as it reads as being very informal. Maybe instead "To access data via named fields, struct types are used."?
src/librustc_typeck/diagnostics.rs
Outdated
|
||
// We create an instance of this struct: | ||
let variable = Foo { x: 0, y: -12 }; | ||
// And we can now call its fields: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"call" is usually reserved for use with functions/methods. I'd suggest "access" instead.
src/librustc_typeck/diagnostics.rs
Outdated
println!("x: {}, y: {}", variable.x, variable.y); | ||
``` | ||
|
||
For more information, go read The Rust Book: https://doc.rust-lang.org/book/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"go read" sounds like a derogatory command. I'd suggest "see" instead (and drop the comma).
src/librustc_typeck/check/mod.rs
Outdated
err | ||
} else { | ||
type_error_struct!(self.tcx().sess, field.span, expr_t, E0610, | ||
"`{}` is a primitive type, therefore doesn't have fields", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest replacing the comma with "and"
Updated. |
src/librustc_typeck/diagnostics.rs
Outdated
println!("x: {}, y: {}", variable.x, variable.y); | ||
``` | ||
|
||
For more information, see The Rust Book: https://doc.rust-lang.org/book/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the comma as it's unnecessary.
src/librustc_typeck/diagnostics.rs
Outdated
@@ -4096,7 +4096,7 @@ assert_eq!(!Question::No, true); | |||
"##, | |||
|
|||
E0609: r##" | |||
An attempt to access a non-existent field in a struct was performed. | |||
Attempted to access a non-existent field in a struct was performed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove "was performed", the sentence doesn't make sense given that it now starts with "Attempted"
Updated. |
LGTM 👍 |
@bors: r=Susurrus |
📌 Commit 2f37894 has been approved by |
☀️ Test successful - status-appveyor, status-travis |
Part of #42229.
cc @Susurrus