-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Silence resolve errors if there were parse errors #116904
Conversation
r? @wesleywiser (rustbot has picked a reviewer for you, use r? to override) |
The job Click to see the possible cause of the failure (guessed by this bot)
|
) { | ||
Ok(kind) => kind, | ||
Err(err) => { | ||
self.sess.parse_errors_encountered.set(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems way more flow-dependent than just checking if we've emitted any session errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For example, this doesn't protect against erroneous recoveries that don't result in a bubbled up Err
value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I experimented with silencing all resolve errors if there were any parse errors, but in practice it wasn't an improvement. The only place where the silencing really comes in handy is when parsing patterns fails, and the silencing should be only within that scope, not all resolve errors.
☔ The latest upstream changes (presumably #116849) made this pull request unmergeable. Please resolve the merge conflicts. |
Fix #74863.
In the following case:
we no longer emit resolution errors due to
url
andtitle
not being available, and more gracefully handle when a sub-pattern is missing a field name.