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

Hide type errors likely caused by incorrect struct literal #60118

Closed
wants to merge 3 commits into from

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Apr 19, 2019

When encountering a parse error that could be a struct literal, but that we can't be certain, keep the found span around until we find a type error that confirms it, provide the appropriate suggestion to surround with braces and hide every type error and non-found ident error coming from within the likely struct literal's span.

r? @petrochenkov

Follow up to #59981

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 19, 2019
When encountering a possible struct literal that is actually
interpreted as a block because it could be a value `foo { bar }` or
`foo { bar: baz }`, we now collect information on both `bar`'s span
and the block's span. If we encounter a type error on `bar`, we
skip it.
@estebank estebank force-pushed the recover-struct-lit-2 branch from edf78f7 to 0691e58 Compare April 20, 2019 01:32
@estebank

This comment has been minimized.

@bors

This comment has been minimized.

@bors

This comment has been minimized.

@estebank

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Success: Queued 392a7be89b77c187cb74e1b6f29d6b81d9540093 with parent 8aaae42, comparison URL.

@petrochenkov
Copy link
Contributor

I wouldn't do this personally, too many hacks for covering one specific case.
(Any extra code is a burden that hinders learning and refactorings, in general.)

cc @rust-lang/compiler

@rust-timer

This comment has been minimized.

@estebank estebank added I-nominated S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 22, 2019
@petrochenkov petrochenkov removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 28, 2019
@nikomatsakis
Copy link
Contributor

I think I agree with @petrochenkov that this PR is going to impose a difficult maintenance burden -- the reason is that it seems quite "non-local" in the way that it communicates information (also, adding shared mutable state will generally interfere with incremental and parallelization efforts, even if it is using a lock).

However, I am generally in favor of us trying to get more sophisticated in the way that we suppress downstream errors -- but I think we should be careful in how we engineer it.

@petrochenkov
Copy link
Contributor

However, I am generally in favor of us trying to get more sophisticated in the way that we suppress downstream errors

We have a number of generic tools for doing that - Expr::Err/Ty::Err at parsing stage, Def::Err at resolution stage, ty::Err in type checking.
This case is tricky because we need to silence further errors not in the node that produced the initial error, but in some other code in proximity of it.

@nikomatsakis
Copy link
Contributor

Right.

@petrochenkov
Copy link
Contributor

Perhaps we need a generic NodeId poisoning.
All nodes poisoned before lowering to HIR will be lowered into Errs.
Or something like that.

@estebank estebank closed this May 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants