`let`'s not needed in struct field definitions #101789

gimbling-away · 2022-09-14T07:40:13Z

Fixes #101683

rust-highfive · 2022-09-14T07:40:16Z

r? @estebank

(rust-highfive has picked a reviewer for you, use r? to override)

estebank · 2022-09-14T10:20:34Z

You will have to run ./x.py test src/test/ui --bless to update the checked test output.

estebank · 2022-09-14T10:26:36Z

compiler/rustc_parse/src/parser/item.rs

@@ -1725,7 +1725,12 @@ impl<'a> Parser<'a> {
                err.help("see https://doc.rust-lang.org/book/ch05-03-method-syntax.html for more information");
                err
            } else {
-                self.expected_ident_found()
+                let mut err = self.expected_ident_found();
+                if ident.name == kw::Let {


Ideally we would do N=1 lookahead to check if the next element is an identifier, and then do self.bump(); (to consume the let) and then get that ident, emit the error and return Ok(ident) (so that the parser can properly recover and continue creating the struct with the expected field name, and even emit errors if the mistyped field isn't included elsewhere). In all other cases, we keep the current behavior (returning err) and let the other parser recovery kick in.

We might want to extend the existing test to include something like

struct S { let s: (), y: (), } fn main() { let _ = S { // should complain about missing `y` s: (), }; let _ = S { y: (), // should complain about missing `s` }; let _ = S { let s: (), // should complain about missing `y` and the spurious `let` }; let _ = S { let y: (), // should complain about missing `s` and the spurious `let` }; let _ = S { let s = (), // should complain about missing `y`, the `:` to `=` typo and the spurious `let` }; let _ = S { let y = (), // should complain about missing `s`, the `:` to `=` typo and the spurious `let` }; }

Keep in mind that the later four recoveries are not included in this PR (but I would like to have it in the future).

gimbling-away · 2022-09-15T15:42:02Z

You will have to run ./x.py test src/test/ui --bless to update the checked test output.

Yah, will bless it a in a while. Currently struggling with git branches again... (why does github not allow multiple forks )😮‍💨

gimbling-away · 2022-09-24T10:30:21Z

@rustbot author

cc @estebank Finally got some time and implemented what you described. Currently it's borked though and a test breaks. Any idea why? 🤔

Rageking8 · 2022-09-24T11:14:51Z

compiler/rustc_parse/src/parser/item.rs at line 1735:
                 err.help("unlike in C++, Java, and C#, functions are declared in `impl` blocks");
                 err.help("see https://doc.rust-lang.org/book/ch05-03-method-syntax.html for more information");
                 err
-            }
-
-            else {
+            } else {
                 let mut err = self.expected_ident_found();
                 if self.check_ident() {
                     self.bump();

(From CI logs)

CI seems to be complaining about the code formatting, maybe try blessing again with tidy?

Not a hundred percent sure though.

gimbling-away · 2022-09-24T11:17:08Z

compiler/rustc_parse/src/parser/item.rs at line 1735:
                 err.help("unlike in C++, Java, and C#, functions are declared in `impl` blocks");
                 err.help("see https://doc.rust-lang.org/book/ch05-03-method-syntax.html for more information");
                 err
-            }
-
-            else {
+            } else {
                 let mut err = self.expected_ident_found();
                 if self.check_ident() {
                     self.bump();

(From CI logs)

CI seems to be complaining about the code formatting, maybe try blessing again with tidy.

Oh ye, forgot to tidy, aside from that I meant the ICE causing test bork

compiler/rustc_parse/src/parser/item.rs

gimbling-away · 2022-10-02T13:55:54Z

@rustbot ready

src/test/ui/parser/removed-syntax-field-let.stderr

compiler/rustc_parse/src/parser/item.rs

Co-authored-by: jyn514 <jyn514@gmail.com> Co-authored-by: Esteban Kuber <estebank@users.noreply.github.com>

Dylan-DPC · 2022-10-10T12:12:40Z

@bors r=estebank

bors · 2022-10-10T12:12:41Z

📌 Commit 6071b4b has been approved by estebank

It is now in the queue for this repository.

`let`'s not needed in struct field definitions Fixes rust-lang#101683

compiler-errors · 2022-10-10T23:39:01Z

compiler/rustc_parse/src/parser/item.rs

@@ -1788,7 +1788,23 @@ impl<'a> Parser<'a> {
                    }
                }
            } else {
-                self.expected_ident_found()
+                let mut err = self.expected_ident_found();
+                if let Some((ident, _)) = self.token.ident() && ident.as_str() == "let" {


This probably could've been self.token.is_keyword(kw::Let), see Token::is_keyword

Alternatively, this could've used Parser::eat_keyword_noexpect to avoid the bump call below

Awesome, will send an improvement PR later on. 👍🏻

compiler-errors · 2022-10-10T23:41:25Z

compiler/rustc_parse/src/parser/item.rs

+                if let Some((ident, _)) = self.token.ident() && ident.as_str() == "let" {
+                    self.bump(); // `let`
+                    let span = self.prev_token.span.until(self.token.span);
+                    err.span_suggestion(


@Xiretza this is a good example of a #[derive(Subdiagnostic)] candidate that both emits notes and a suggestion!

compiler-errors · 2022-10-10T23:45:58Z

compiler/rustc_parse/src/parser/item.rs

+                    self.bump();
+                    return Ok(ident);


@gimbles for future reference, why is this bumping twice? This should perhaps be:

Suggested change

self.bump();

return Ok(ident);

let ident = self.parse_ident()?;

return Ok(ident);

Otherwise, it seems to me like it's returning let as the struct field name?

Look at the earlier reviews on wonky spans ^^

…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#101360 (Point out incompatible closure bounds) - rust-lang#101789 (`let`'s not needed in struct field definitions) - rust-lang#102846 (update to syn-1.0.102) - rust-lang#102871 (rustdoc: clean up overly complex `.trait-impl` CSS selectors) - rust-lang#102876 (suggest candidates for unresolved import) - rust-lang#102888 (Improve rustdoc-gui search-color test) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rust-highfive assigned estebank Sep 14, 2022

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 14, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 14, 2022

This comment has been minimized.

Sign in to view

estebank reviewed Sep 14, 2022

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 24, 2022

estebank reviewed Sep 24, 2022

View reviewed changes