Handle incorrect placement of parentheses in trait bounds more gracefully#84896
Handle incorrect placement of parentheses in trait bounds more gracefully#84896bors merged 1 commit intorust-lang:masterfrom
Conversation
|
r? @jackh726 (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
jackh726
left a comment
jackh726
left a comment
There was a problem hiding this comment.
One small comment, but I'm not sure it's worth following up on in the PR
| error: expected parameter name, found `{` | ||
| --> $DIR/trait-object-delimiters.rs:8:17 | ||
| | | ||
| LL | fn foo3(_: &dyn {Drop + AsRef<str>}) {} | ||
| | ^ expected parameter name | ||
|
|
||
| error: expected one of `!`, `(`, `)`, `,`, `?`, `for`, lifetime, or path, found `{` | ||
| --> $DIR/trait-object-delimiters.rs:8:17 | ||
| | | ||
| LL | fn foo3(_: &dyn {Drop + AsRef<str>}) {} | ||
| | -^ expected one of 8 possible tokens | ||
| | | | ||
| | help: missing `,` | ||
|
|
||
| error: expected identifier, found `<` | ||
| --> $DIR/trait-object-delimiters.rs:12:17 | ||
| | | ||
| LL | fn foo4(_: &dyn <Drop + AsRef<str>>) {} | ||
| | ^ expected identifier |
There was a problem hiding this comment.
The fact that these two give different errors is a bit confusing. One expects a parameter and the other an identifier.
There was a problem hiding this comment.
Yeah, I agree. I had a fix that improved this case, but it regressed other cases on malformed super-traits badly, so I pulled it of. I still left the test in in case we ever revisit this.
|
@bors r+ |
|
📌 Commit 6b64202 has been approved by |
bors
added
S-waiting-on-bors
|
@bors rollup |
| | | ||
| help: remove the parentheses | ||
| | | ||
| LL | fn foo2(_: &dyn Drop + AsRef<str>) {} |
There was a problem hiding this comment.
This suggestion seems odd, since if you made this change, you'd then get the error above of "use parentheses to disambiguate". Shouldn't this suggest to use &(dyn Drop + AsRef<str>) instead?
There was a problem hiding this comment.
Yeah, this is less than ideal. But I don't think we have the right information to give that suggestion. This will eventually lead the user to the right solution though.
There was a problem hiding this comment.
An unwritten rule is that we are ok with giving incomplete suggestions if the resulting code will give you a second correct suggestion. We try to get ahead and give a good direct fix when possible, but the amount of metadata we'd need to carry seems low benefit for the maintenance burden. Particularly in the parser, I have to be very careful not to introduce invalid changes to the grammar by accident.
| --> $DIR/trait-object-delimiters.rs:8:17 | ||
| | | ||
| LL | fn foo3(_: &dyn {Drop + AsRef<str>}) {} | ||
| | ^ expected parameter name |
There was a problem hiding this comment.
This is really weird to me — it's not expecting a parameter name, but rather a type/trait, no?
There was a problem hiding this comment.
It's a consequence of the recovery machinery. Making the errors for the {} case better regressed other errors involving trait T: A + B {, so I decided not to address it in this PR.
| LL | fn foo3(_: &dyn {Drop + AsRef<str>}) {} | ||
| | -^ expected one of 8 possible tokens | ||
| | | | ||
| | help: missing `,` |
Handle incorrect placement of parentheses in trait bounds more gracefully Fix rust-lang#84772. CC `@jonhoo`
Handle incorrect placement of parentheses in trait bounds more gracefully Fix rust-lang#84772. CC ``@jonhoo``
Handle incorrect placement of parentheses in trait bounds more gracefully Fix rust-lang#84772. CC ```@jonhoo```
Handle incorrect placement of parentheses in trait bounds more gracefully Fix rust-lang#84772. CC ````@jonhoo````
Handle incorrect placement of parentheses in trait bounds more gracefully Fix rust-lang#84772. CC `````@jonhoo`````
Rollup of 11 pull requests Successful merges: - rust-lang#84409 (Ensure TLS destructors run before thread joins in SGX) - rust-lang#84500 (Add --run flag to compiletest) - rust-lang#84728 (Add test for suggestion to borrow unsized function parameters) - rust-lang#84734 (Add `needs-unwind` and beginning of support for testing `panic=abort` std to compiletest) - rust-lang#84755 (Allow using `core::` in intra-doc links within core itself) - rust-lang#84871 (Disallows `#![feature(no_coverage)]` on stable and beta (using standard crate-level gating)) - rust-lang#84872 (Wire up tidy dependency checks for cg_clif) - rust-lang#84896 (Handle incorrect placement of parentheses in trait bounds more gracefully) - rust-lang#84905 (CTFE engine: rename copy → copy_intrinsic, move to intrinsics.rs) - rust-lang#84953 (Remove unneeded call to with_default_session_globals in rustdoc highlight) - rust-lang#84987 (small nits) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
6 participants
Fix #84772.
CC @jonhoo