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

Add parse-error recovery for erroneous struct_id { } form. #8418

Conversation

pnkfelix
Copy link
Member

@pnkfelix pnkfelix commented Aug 9, 2013

Fix #3192. r? anyone

There are 4 different new tests, to check some different scenarios for
what the parse context is at the time of recovery, becasue our
compile-fail infrastructure does not appear to handle verifying
error-recovery situations.

Differentiate between unit-like struct definition item and unit-like
struct construction in the error message.


More generally, outlines a more generic strategy for parse error
recovery: By committing to an expression/statement at set points in
the parser, we can then do some look-ahead to catch common mistakes
and skip over them.

One detail about this strategy is that you want to avoid emitting the
"helpful" message unless the input is reasonably close to the case of
interest. (E.g. do not warn about a potential unit struct for an
input of the form let hmm = do foo { } { };)

To accomplish this, I added (partial) last_token tracking; used for
commit_stmt support.

The check_for_erroneous_unit_struct_expecting fn returns bool to
signal whether it "made progress"; currently unused; this is meant for
use to compose several such recovery checks together in a loop.

There are 4 different new tests, to check some different scenarios for
what the parse context is at the time of recovery, becasue our
compile-fail infrastructure does not appear to handle verifying
error-recovery situations.

Differentiate between unit-like struct definition item and unit-like
struct construction in the error message.

----

More generally, outlines a more generic strategy for parse error
recovery: By committing to an expression/statement at set points in
the parser, we can then do some look-ahead to catch common mistakes
and skip over them.

One detail about this strategy is that you want to avoid emitting the
"helpful" message unless the input is reasonably close to the case of
interest.  (E.g. do not warn about a potential unit struct for an
input of the form `let hmm = do foo { } { };`)

To accomplish this, I added (partial) last_token tracking; used for
`commit_stmt` support.

The check_for_erroneous_unit_struct_expecting fn returns bool to
signal whether it "made progress"; currently unused; this is meant for
use to compose several such recovery checks together in a loop.
let actual = self.this_token_to_str();
self.fatal(
if expected.len() != 1 {
fmt!("expected one of `%s` but found `%s`", expect, actual)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: we prefer "expected foo, found bar" to "expected foo but found bar"

bors added a commit that referenced this pull request Aug 12, 2013
…-for-empty-struct-init, r=pcwalton,me

Fix #3192.  r? anyone

There are 4 different new tests, to check some different scenarios for
what the parse context is at the time of recovery, becasue our
compile-fail infrastructure does not appear to handle verifying
error-recovery situations.

Differentiate between unit-like struct definition item and unit-like
struct construction in the error message.

----

More generally, outlines a more generic strategy for parse error
recovery: By committing to an expression/statement at set points in
the parser, we can then do some look-ahead to catch common mistakes
and skip over them.

One detail about this strategy is that you want to avoid emitting the
"helpful" message unless the input is reasonably close to the case of
interest.  (E.g. do not warn about a potential unit struct for an
input of the form `let hmm = do foo { } { };`)

To accomplish this, I added (partial) last_token tracking; used for
`commit_stmt` support.

The check_for_erroneous_unit_struct_expecting fn returns bool to
signal whether it "made progress"; currently unused; this is meant for
use to compose several such recovery checks together in a loop.
@pnkfelix
Copy link
Member Author

assuming bors merges in the original commit, I'll post a follow up to address pcwalton's comments.

@bors bors closed this Aug 12, 2013
pnkfelix added a commit to pnkfelix/rust that referenced this pull request Aug 12, 2013
flip1995 pushed a commit to flip1995/rust that referenced this pull request Feb 24, 2022
…, r=llogiq

Downgrade transmute_undefined_repr to nursery

Reason: rust-lang#8417. I am skeptical of this lint but maybe there is a narrower subset of types on which it is useful, so keeping it for now but moving to nursery for further development.

---

*Please write a short comment explaining your change (or "none" for internal only changes)*

changelog: Remove [`transmute_undefined_repr`] from default set of enabled lints
flip1995 pushed a commit to flip1995/rust that referenced this pull request Mar 14, 2022
…2, r=Manishearth

Transmute_undefined_repr to nursery again

This PR reinstates rust-lang#8418, which was reverted in rust-lang#8425 (incorrectly I think).

I don't want to start a revert war over this but I feel very strongly that this lint is not in a state that would be a net benefit to users of clippy. In its current form, making this an enabled-by-default `correctness` lint with authoritative-sounding proclamations of undefined behavior does more harm than the benefit of the true positive cases.

I can file a bunch more examples of false positives but I don't want to give the author of this lint the impression that it is ready to graduate from `nursery` as soon as I've exhausted the amount of time I am willing to spend revising this lint.

Instead I would recommend that the author of the lint try running it on some reputable codebases containing transmutes. Everywhere that the lint triggers please consider critically whether it should be triggering. For cases that you think are true positives, please raise a few of them with the crate authors (in a PR or issue) to better understand their perspective if they think the transmute is correct.

---

*Please write a short comment explaining your change (or "none" for internal only changes)*

changelog: Re-remove [`transmute_undefined_repr`] from default set of enabled lints
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong error message trying to initialise an empty struct
3 participants