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

Don't (ab)use TyKind::Error when not planning to emit error #70866

Closed
8 tasks done
mark-i-m opened this issue Apr 6, 2020 · 0 comments · Fixed by #70551
Closed
8 tasks done

Don't (ab)use TyKind::Error when not planning to emit error #70866

mark-i-m opened this issue Apr 6, 2020 · 0 comments · Fixed by #70551
Labels
A-type-system Area: Type system C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@mark-i-m
Copy link
Member

mark-i-m commented Apr 6, 2020

cc #70551 @eddyb

Currently, it appears we are misusing TyKind::Error (and Const::Error) in these places, which we should refactor:

@jonas-schievink jonas-schievink added C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-type-system Area: Type system labels Apr 6, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Apr 8, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Apr 9, 2020
De-abuse TyKind::Error in pattern type checking

r? @eddyb

cc rust-lang#70866

In particular, I would appreciate extra scrutiny over the soundness of these changes.

Also, this will go a bit slowly because I'm going to use my other PR (rust-lang#70551) to check if I missed anything.
Centril added a commit to Centril/rust that referenced this issue Apr 10, 2020
De-abuse TyKind::Error in pattern type checking

r? @eddyb

cc rust-lang#70866

In particular, I would appreciate extra scrutiny over the soundness of these changes.

Also, this will go a bit slowly because I'm going to use my other PR (rust-lang#70551) to check if I missed anything.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 7, 2020
bors added a commit to rust-lang-ci/rust that referenced this issue May 21, 2020
…, r=varkor

De-abuse TyKind::Error in exhaustiveness checking

Replaces rust-lang#71074. Context: rust-lang#70866.

In order to remove the use of `TyKind::Error`, I had to make sure we skip over those fields whose inhabitedness should not be observed. This is potentially error-prone however, since we must be careful not to mix filtered and unfiltered lists of patterns. I managed to hide away most of the filtering behind a new `Fields` struct, that I used everywhere relevant. I quite like the result; I think the twin concepts of `Constructor` and `Fields` make a good mental model.

As usual, I tried to separate commits that shuffle code around from commits that require more thought to review.

cc @varkor @Centril
RalfJung added a commit to RalfJung/rust that referenced this issue Jun 18, 2020
Make all uses of ty::Error delay a span bug

r? @eddyb

A second attempt at rust-lang#70245

resolves rust-lang#70866
Manishearth added a commit to Manishearth/rust that referenced this issue Jun 18, 2020
Make all uses of ty::Error delay a span bug

r? @eddyb

A second attempt at rust-lang#70245

resolves rust-lang#70866
@bors bors closed this as completed in 9d388d4 Jun 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-type-system Area: Type system C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants