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

New error codes #42614

Merged
merged 6 commits into from
Jun 23, 2017
Merged

New error codes #42614

merged 6 commits into from
Jun 23, 2017

Conversation

GuillaumeGomez
Copy link
Member

@rust-highfive
Copy link
Collaborator

r? @pnkfelix

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

@@ -4152,6 +4152,252 @@ println!("x: {}, y: {}", variable.x, variable.y);
For more information see The Rust Book: https://doc.rust-lang.org/book/
"##,

E0611: r##"
Attempted to access to a private field on a tuple-struct.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be "Attempted to access a private field on a tuple-struct."


Since the field is private, you have two solutions:

First one: set the field public:
Copy link
Contributor

Choose a reason for hiding this comment

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

Use "make" instead of "set"

println!("{}", y.0); // So we can access it directly.
```

Second one: add a getter function to keep the field private but allow to access
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead "add a getter function to keep the field private but allow for accessing its value:".

// on type `Foo`
```

If a tuple/tuple-struct type has x fields, you can only try to access these x
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest using "n" here instead of "x" as "n" is commonly used for referring to integer values.

println!("({}, {}, {}, {})", x.0, x.1, x.2, x.3);
```

If you want to index an array, use `[]` instead:
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead "If you want to index into an array, use [] instead:"

f.method();
```

However, if you wanted to access a field, of the struct, check if you didn't
Copy link
Contributor

Choose a reason for hiding this comment

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

Too many commas and incorrect conjugation. Instead "However, if you wanted to access a field of a struct check that the field name is spelled correctly."

"##,

E0616: r##"
Attempted to access to a private field on a struct.
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead "Attempted to access a private field on a struct."


If you want to access this field, you have two options:

First one: set the field public:
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead: "1) Set the field public:"

I don't like the double-colons on this line.

println!("{}", f.x); // ok!
```

Second one: add a getter function:
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead "2) Add a getter function:"

idx.node, expr_t).emit();
} else {
type_error_struct!(self.tcx().sess, expr.span, expr_t, E0613,
"attempted tuple index `{}` on type `{}`, but the type was not a \
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest instead: "attempted to access tuple index {} on type {}, but the type is not a tuple or tuple struct"

@GuillaumeGomez
Copy link
Member Author

@Susurrus: Thanks a lot for this review! Updated. :)

Copy link
Member

@frewsxcv frewsxcv left a comment

Choose a reason for hiding this comment

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

Travis found some errors

println!("({}, {}, {}, {})", x.0, x.1, x.2, x.3);
```

If you want to index into an array, use `[]` instead:
Copy link
Member

Choose a reason for hiding this comment

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

index into a slice

would this be more accurate?

Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted to include both Vec and slices.

Copy link
Member

Choose a reason for hiding this comment

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

In the context of Rust, when I see the term "array" I often think of fixed-length arrays [T; n]. But I don't have a better term on hand that covers the cases here, so this is fine with me.

println!("[{}, {}, {}, {}]", x[0], x[1], x[2], x[3]);
```

If you want to access a field of a struct, check the field's name wasn't
Copy link
Member

Choose a reason for hiding this comment

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

If you want to access a field of a struct, ensure the field's name is spelled correctly

this sounds better to me, but i don't feel strongly

@arielb1 arielb1 added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jun 13, 2017
@bors
Copy link
Contributor

bors commented Jun 14, 2017

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

@GuillaumeGomez
Copy link
Member Author

Rebased.

@QuietMisdreavus
Copy link
Member

QuietMisdreavus commented Jun 15, 2017

Would it be worth combining E0611 (private field access, tuple struct) and E0616 (private field access, traditional struct)? From the user's perspective, it's the same error, just in different situations.

(EDIT: For any infra team members who see this in a week, I believe imperio just left for a 10-day vacation, so updates on this will likely wait until he's come back from that.)

@alexcrichton
Copy link
Member

ping @pnkfelix, I think this is ready for a re-review

@alexcrichton alexcrichton added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 22, 2017
@pnkfelix
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jun 22, 2017

📌 Commit ee60064 has been approved by pnkfelix

@pnkfelix pnkfelix added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 22, 2017
@bors
Copy link
Contributor

bors commented Jun 22, 2017

⌛ Testing commit ee60064 with merge bd62230...

@bors
Copy link
Contributor

bors commented Jun 23, 2017

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

@bors bors merged commit ee60064 into rust-lang:master Jun 23, 2017
@GuillaumeGomez GuillaumeGomez deleted the new-error-codes branch June 24, 2017 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants