add help when constructing ADTs with private fields#156943
Conversation
rustbot
added
S-waiting-on-review
|
r? @TaKO8Ki rustbot has assigned @TaKO8Ki. Use Why was this reviewer chosen?The reviewer was selected based on:
|
| --> $DIR/suggest-private-fields.rs:17:9 | ||
| | | ||
| LL | bb: 20, | ||
| | ^^ `B` does not have this field |
There was a problem hiding this comment.
Perhaps keeping this message is better? "Struct does not have this field" and "Struct have inaccessible private fields" convey two different pieces of information.
There was a problem hiding this comment.
I think it's better to drop it. As mentioned in the original issue, it's unnecessary to state this since the struct can't be constructed this way anyways.
There was a problem hiding this comment.
E0560 still happens on this line. So at the very least, this is an item that shouldn't be affected (be removed) in this PR, right?
There was a problem hiding this comment.
Fair point. Maybe we should make a new error for trying to construct a struct with private fields? E0560 is misleading because even if you fix the name the struct is impossible to construct this way.
| error[E0451]: field `b` of struct `Foo` is private | ||
| --> $DIR/E0451.rs:18:29 | ||
| | | ||
| LL | let f = bar::Foo{ a: 0, b: 0 }; | ||
| | ^ private field | ||
| | | ||
| help: initializer expressions cannot construct structs with private fields | ||
| --> $DIR/E0451.rs:18:13 | ||
| | | ||
| LL | let f = bar::Foo{ a: 0, b: 0 }; | ||
| | ^^^^^^^^^^^^^^^^^^^^^^ |
There was a problem hiding this comment.
Personally, I feel the new help message creates extra noise in some cases. For example here. I’d rather expect this to become a note, because help is used to provide suggestions.
error[E0451]: field `b` of struct `Foo` is private
--> $DIR/E0451.rs:18:29
|
LL | let f = bar::Foo{ a: 0, b: 0 };
| ^ private field
|
+ note: initializer expressions cannot construct structs with private fields
- help: initializer expressions cannot construct structs with private fields
- --> $DIR/E0451.rs:18:13
- |
- LL | let f = bar::Foo{ a: 0, b: 0 };
- | ^^^^^^^^^^^^^^^^^^^^^^There was a problem hiding this comment.
I understand the concern, but I also think that the span is helpful for learners who might not know what "initializer expression" or "initializer syntax" refers to.
I have no problem converting it to a note, though.
There was a problem hiding this comment.
IMHO, I still think the span makes the output redundant. I write Rust code in an editor like Zed, and my terminal is only 280 pixels tall (about 13 lines of rendering) at the bottom. In this situation, when scrolling through the compiler output, I prefer it to be straightforward and clear.
Learners may not know the meaning of this term only at the beginning, but once they do, they won't need to see this output every time.
rustbot
added
S-waiting-on-author
|
Reminder, once the PR becomes ready for a review, use |
|
☔ The latest upstream changes (presumably #157600) made this pull request unmergeable. Please resolve the merge conflicts. |
4 participants
Fixes #153259.
I'm a little unsure about the wording. I don't want to imply that private fields can never be used in initializer expressions, since private fields are fine if the fields are in the same module. The existing error message uses the term "private", though, so I went for consistency.
I also wonder if it would be useful to include a note about
fn newbeing a common idiom to construct private structs, or even suggesting it if it exists.