Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

cleanup our region error API #110220

Merged
merged 3 commits into from
Apr 13, 2023
Merged

cleanup our region error API #110220

merged 3 commits into from
Apr 13, 2023

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Apr 12, 2023

r? types

- require `TypeErrCtxt` to always result in an error
- move `resolve_regions_and_report_errors` to the `ObligationCtxt`
- merge `process_registered_region_obligations` into `resolve_regions`
@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 Apr 12, 2023
@rustbot
Copy link
Collaborator

rustbot commented Apr 12, 2023

Some changes occurred in engine.rs, potentially modifying the public API of ObligationCtxt.

cc @lcnr, @compiler-errors

Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

r=me

@@ -1763,7 +1763,9 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> {

// constrain inference variables a bit more to nested obligations from normalize so
// we can have more helpful errors.
ocx.select_where_possible();
//
// we intentionally errors from normalization here.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// we intentionally errors from normalization here.
// we intentionally drop errors from normalization here,
// since the normalization is just done to make the types better.

if let Some(_) = self.infcx.tcx.sess.has_errors_or_delayed_span_bugs() {
// ok, emitted an error.
} else {
self.infcx
Copy link
Member

Choose a reason for hiding this comment

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

This might create a problem if we create an error then cancel it, but I guess we can just wait until the ICE is filed :)

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, imo that should not happen. Once you start using the err_ctxt() you should only cancel if we already emit an error somewhere else.

Copy link
Member

Choose a reason for hiding this comment

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

I think this caused rust-lang/rust-clippy#10645 🙈

@compiler-errors compiler-errors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 12, 2023
@lcnr
Copy link
Contributor Author

lcnr commented Apr 12, 2023

@bors r=compiler-errors rollup

@bors
Copy link
Collaborator

bors commented Apr 12, 2023

📌 Commit c0d3d32 has been approved by compiler-errors

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 12, 2023
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Apr 13, 2023
cleanup our region error API

- require `TypeErrCtxt` to always result in an error, closing rust-lang#108810
- move `resolve_regions_and_report_errors` to the `ObligationCtxt`
- call `process_registered_region_obligations` in `resolve_regions`
- move `resolve_regions` into the `outlives` submodule
- add `#[must_use]` to functions returning lists of errors

r? types
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 13, 2023
cleanup our region error API

- require `TypeErrCtxt` to always result in an error, closing rust-lang#108810
- move `resolve_regions_and_report_errors` to the `ObligationCtxt`
- call `process_registered_region_obligations` in `resolve_regions`
- move `resolve_regions` into the `outlives` submodule
- add `#[must_use]` to functions returning lists of errors

r? types
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 13, 2023
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#110072 (Stabilize IsTerminal)
 - rust-lang#110195 (Erase lifetimes above `ty::INNERMOST` when probing ambiguous types)
 - rust-lang#110218 (Remove `ToRegionVid`)
 - rust-lang#110220 (cleanup our region error API)
 - rust-lang#110234 (Fix btree `CursorMut::insert_after` check)
 - rust-lang#110262 (Update unwind_safe.rs)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 6f1500a into rust-lang:master Apr 13, 2023
@rustbot rustbot added this to the 1.70.0 milestone Apr 13, 2023
@lcnr lcnr deleted the regionzz branch April 13, 2023 11:43
# for free to join this conversation on GitHub. Already have an account? # 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants