Suggest turbofish for multi-param generics used as call arguments - #160104
Suggest turbofish for multi-param generics used as call arguments#160104martonmoro wants to merge 2 commits into
Conversation
|
The parser was modified, potentially altering the grammar of (stable) Rust cc @fmease |
rustbot
added
S-waiting-on-review
|
Thanks for the pull request, and welcome! The Rust Project is excited to review your changes, and you should hear from @hanna-kruppe (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
Why was this reviewer chosen?The reviewer was selected based on:
|
|
I’m not familiar enough with the parser code to review this, and won’t have time to get familiar with it, so: @rustbot reroll |
| @@ -0,0 +1,24 @@ | |||
| struct Many<A, B, C, D> { | |||
There was a problem hiding this comment.
can we use run-rustfix for this test?
| let failed = self.create_snapshot_for_diagnostic(); | ||
| self.restore_snapshot(start); | ||
| match self.parse_paren_comma_seq(|p| { | ||
| let expr = p.parse_expr_catch_underscore(Restrictions::empty())?; |
There was a problem hiding this comment.
The fallback here reparses the entire argument list and applies recovery to every Binary(Lt). If a later argument causes the initial parse failure, earlier valid comparisons are reconsidered.
For example this code:
struct S;
struct Many<A, B, C, D>(A, B, C, D);
impl<A, B, C, D> Many<A, B, C, D> {
fn new() -> Self {
todo!()
}
}
fn take_three(_: bool, _: bool, _: Many<i32, Many<(), i32, S, S>, i32, i32>) {}
fn main() {
let (a, b, c, d) = (1, 2, 3, 4);
take_three(a < b, c > (d), Many<i32, Many<(), i32, S, S>, i32, i32>::new());
}the current output is:
--> /tmp/now.rs:14:16
|
14 | take_three(a < b, c > (d), Many<i32, Many<(), i32, S, S>, i32, i32>::new());
| ^^^^^
|
help: use `::<...>` instead of `<...>` to specify lifetime, type, or const arguments
|
14 | take_three(a ::< b, c > (d), Many<i32, Many<(), i32, S, S>, i32, i32>::new());
| ++
error: generic args in this position require the turbofish syntax
--> /tmp/now.rs:14:32
|
14 | take_three(a < b, c > (d), Many<i32, Many<(), i32, S, S>, i32, i32>::new());
| ^^^^^^^^
|
help: use `::<...>` instead of `<...>` to specify lifetime, type, or const arguments
|
14 | take_three(a < b, c > (d), Many::<i32, Many<(), i32, S, S>, i32, i32>::new());
| ++
error[E0061]: this function takes 3 arguments but 2 arguments were supplied
--> /tmp/now.rs:14:5
|
14 | take_three(a < b, c > (d), Many<i32, Many<(), i32, S, S>, i32, i32>::new());
| ^^^^^^^^^^----------------------------------------------------------------- argument #3 of type `Many<i32, Many<(), i32, S, S>, i32, i32>` is missing
|
note: function defined here
--> /tmp/now.rs:10:4
|
10 | fn take_three(_: bool, _: bool, _: Many<i32, Many<(), i32, S, S>, i32, i32>) {}
| ^^^^^^^^^^ -------------------------------------------
help: provide the argument
|
14 | take_three(a < b, c > (d), Many<i32, Many<(), i32, S, S>, i32, i32>::new(), /* Many<i32, Many<(), i32, S, S>, i32, i32> */);
| ++++++++++++++++++++++++++++++++++++++++++++++++which is wrong.
The nightly output for this code is :
error: expected expression, found `,`
--> src/main.rs:14:61
|
14 | take_three(a < b, c > (d), Many<i32, Many<(), i32, S, S>, i32, i32>::new());
| ^ expected expressionwhich points out the rootcause.
| // The attempt above can emit diagnostics before it fails (like in | ||
| // `tests/ui/parser/attribute/attr-stmt-expr-attr-bad.rs`), and parsing again the same | ||
| // tokens would emit them a second time. | ||
| if self.dcx().err_count() != err_count { |
There was a problem hiding this comment.
seems like a hack here.
| let failed = self.create_snapshot_for_diagnostic(); | ||
| self.restore_snapshot(start); | ||
| match self.parse_paren_comma_seq(|p| { | ||
| let expr = p.parse_expr_catch_underscore(Restrictions::empty())?; |
There was a problem hiding this comment.
generally, i don't like the way of repeat the same parsing here, can we do in the way of stop where error happened and try to speculatively detect a missing turbofish.
4 participants
Fixes #159745
The issue occurred because when parsing
bar(Many<i32, Many<(), i32, S, S>, i32, i32>::new())it parsed<as aBinary(Lt)and treated the comma as the end of argument 1 then, when it gets to>it tries parsing it again as an operator, attempting to parse a RHS which in this case seems like a bare comma that cannot start an expression. After that, it parses the rest of the "comma-separated arguments" incorrectly, which is why the error wasexpected expression, found ','. Some other methods seemed relevant here, likecheck_no_chained_comparison(only when the very next token is also a comparison) andcheck_mistyped_turbofish_with_multiple_type_params(only called fromparse_full_stmtafter the parser already hit an error expecting;, and comma is an error at stmt level but not at expr level), but they were not called during parsing. (There is probably some refactoring potential incheck_mistyped_turbofish_with_multiple_type_paramsand my function tho).My first attempt was to recover whenever a call argument parsed as
Binary(Lt)followed by a comma, re-parsing the rest withparse_generic_arg. That turned out to be wrong, because it can't tell the broken case apart from a valid one:f(a < b, c > (d))is two arguments and looks identical to the parser at that point, so it started rejecting code that compiles today.So instead of deciding per-argument, I let
parse_expr_paren_seqparse the whole argument list as expressions first, and only if that fails do I reparse the list with the turbofish recovery enabled. If the retry also fails, I cancel it and return the original error, restoring the parser to where the first attempt died so the reported span doesn't move.I scoped this to call-argument parsing, not to the shared comma-separated list parser.