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

Silence resolve errors if there were parse errors #116904

Closed
wants to merge 2 commits into from

Conversation

estebank
Copy link
Contributor

Fix #74863.

In the following case:

error: missing field name before pattern
  --> $DIR/avoid-resolve-errors-caused-by-parse-error.rs:12:27
   |
LL |     if let Website { url, Some(title) } = website {
   |            -------        ^^^^^^^^^^^ this pattern needs to be preceeded by a field name
   |            |
   |            while parsing the fields for this pattern
   |
help: you might have meant to add a field
   |
LL |     if let Website { url, field_name: Some(title) } = website {
   |                           +++++++++++

we no longer emit resolution errors due to url and title not being available, and more gracefully handle when a sub-pattern is missing a field name.

@rustbot
Copy link
Collaborator

rustbot commented Oct 18, 2023

r? @wesleywiser

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 18, 2023
@rust-log-analyzer
Copy link
Collaborator

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
    Checking rustc_ast_passes v0.0.0 (/checkout/compiler/rustc_ast_passes)
   Compiling coverage_test_macros v0.0.0 (/checkout/compiler/rustc_mir_transform/src/coverage/test_macros)
    Checking rustc_expand v0.0.0 (/checkout/compiler/rustc_expand)
    Checking rustc_builtin_macros v0.0.0 (/checkout/compiler/rustc_builtin_macros)
error[E0277]: `Cell<bool>` doesn't implement `DynSync`. Add it to `rustc_data_structures::marker` or use `IntoDynSyncSend` if it's already `Sync`
   --> compiler/rustc_middle/src/ty/context/tls.rs:98:33
    |
98  |         sync::assert_dyn_sync::<ImplicitCtxt<'_, '_>>();
    |                                 ^^^^^^^^^^^^^^^^^^^^ within `ImplicitCtxt<'_, '_>`, the trait `DynSync` is not implemented for `Cell<bool>`
note: required because it appears within the type `ParseSess`
   --> /checkout/compiler/rustc_session/src/parse.rs:194:12
    |
194 | pub struct ParseSess {
---
    |            ^^^^^^^^^^^^
note: required by a bound in `assert_dyn_sync`
   --> /checkout/compiler/rustc_data_structures/src/marker.rs:188:36
    |
188 | pub fn assert_dyn_sync<T: ?Sized + DynSync>() {}
    |                                    ^^^^^^^ required by this bound in `assert_dyn_sync`
For more information about this error, try `rustc --explain E0277`.
error: could not compile `rustc_middle` (lib) due to previous error
warning: build failed, waiting for other jobs to finish...
error: could not compile `rustc_middle` (lib test) due to previous error

) {
Ok(kind) => kind,
Err(err) => {
self.sess.parse_errors_encountered.set(true);
Copy link
Member

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.

Copy link
Member

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.

Copy link
Contributor Author

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.

@petrochenkov petrochenkov self-assigned this Oct 19, 2023
@bors
Copy link
Contributor

bors commented Oct 23, 2023

☔ The latest upstream changes (presumably #116849) made this pull request unmergeable. Please resolve the merge conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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.

Error on struct pattern with enum
7 participants