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 ErrorGuaranteed to hir::{Expr,Ty}Kind::Err variants #108379

Merged
merged 3 commits into from
Feb 26, 2023

Conversation

compiler-errors
Copy link
Member

First step in making the Err variants of ExprKind and TyKind require an ErrorGuaranteed during parsing. Making the corresponding AST versions require ErrorGuaranteed is a bit harder, whereas it was pretty easy to do this for HIR, so let's do that first.

The only weird thing about this PR is that ErrorGuaranteed is moved to rustc_span. This is certainly not the right place to put it, but rustc_hir cannot depend on rustc_error because the latter already depends on the former. Should I just pull out some of the error machinery from rustc_error into an even more minimal crate that rustc_hir can depend on? Advice would be appreciated.

@rustbot
Copy link
Collaborator

rustbot commented Feb 23, 2023

r? @lcnr

(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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Feb 23, 2023
@rust-log-analyzer

This comment has been minimized.

@rustbot
Copy link
Collaborator

rustbot commented Feb 23, 2023

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@cjgillot
Copy link
Contributor

The only weird thing about this PR is that ErrorGuaranteed is moved to rustc_span. This is certainly not the right place to put it, but rustc_hir cannot depend on rustc_error because the latter already depends on the former. Should I just pull out some of the error machinery from rustc_error into an even more minimal crate that rustc_hir can depend on? Advice would be appreciated.

Why does rustc_errors depend on rustc_hir? Can the dependency chain become rust_span -> rustc_errors -> rustc_hir instead by moving stuff out of rustc_hir?

@bors
Copy link
Contributor

bors commented Feb 23, 2023

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

@compiler-errors
Copy link
Member Author

compiler-errors commented Feb 24, 2023

@cjgillot:

Why does rustc_errors depend on rustc_hir?

Two reasons. First is that rustc_errors depends on rustc_hir directly, but that can be easily removed. Second is because rustc_errors depends on rustc_lint_defs (because we use LintExpectationId in diagnostics), and rustc_lint_defs depends on rustc_hir because LintExpectationId has a HirId inside of it.

Even if I fix this somehow, I'll be back into the same situation if/when I want to add ErrorGuaranteed to ast::ExprKind::Err, etc., because LintExpectationId also depends on ast::AttrId.

I'm not sure if there's a good way to untangle this cycle now I think of it. I could pull ErrorGuaranteed out into a totally separate crate?

@cjgillot
Copy link
Contributor

What a knot, indeed. ErrorGuaranteed could move to rustc_span, and be accessible to all from there.
rustc_errors already uses unchecked_claim_error_was_emitted, so that wouldn't break much of it.

@cjgillot
Copy link
Contributor

r=me after rebase

@compiler-errors
Copy link
Member Author

@bors r=cjgillot

@bors
Copy link
Contributor

bors commented Feb 25, 2023

📌 Commit 0f4a7d1 has been approved by cjgillot

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 25, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 26, 2023
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#107941 (Treat `str` as containing `[u8]` for auto trait purposes)
 - rust-lang#108299 (Require `literal`s for some `(u)int_impl!` parameters)
 - rust-lang#108337 (hir-analysis: make a helpful note)
 - rust-lang#108379 (Add `ErrorGuaranteed` to `hir::{Expr,Ty}Kind::Err` variants)
 - rust-lang#108418 (Replace parse_[sth]_expr with parse_expr_[sth] function names)
 - rust-lang#108424 (rustc_infer: Consolidate obligation elaboration de-duplication)
 - rust-lang#108475 (Fix `VecDeque::shrink_to` and add tests.)
 - rust-lang#108482 (statically guarantee that current error codes are documented)
 - rust-lang#108484 (Remove `from` lang item)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 19b8685 into rust-lang:master Feb 26, 2023
@rustbot rustbot added this to the 1.69.0 milestone Feb 26, 2023
@compiler-errors compiler-errors deleted the hir-error-guaranteed branch August 11, 2023 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc 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