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

Cleanup hack to expect scrutinee: bool in if-to-match desugaring in match-expr typeck #60707

Closed
Tracked by #53667
Centril opened this issue May 10, 2019 · 2 comments
Closed
Tracked by #53667
Assignees
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. F-let_chains `#![feature(let_chains)]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@Centril
Copy link
Contributor

Centril commented May 10, 2019

In #59288 a hack is introduced in rustc_typeck::check::_match::check_match that checks, via MatchSource::IfDesugar, whether an if-expression was desugared to match and if so, it requires that the scrutinee be typed at bool.

We might want to remove this hack in favor of a more principled solution, with options:

  1. Fix type ascription such that it admits coercions and then use ExprKind::Type(...) to set the expectation that the type be coercible to bool. This will require adjustments to diagnostics logic to reduce clutter.

  2. Use a cast $scrutinee as bool and use ExprKind::Cast(...) to set the expectation that the type be coercible to bool. Same applies wrt. diagnostics.

  3. Add a flag in the type ascription HIR that enables the "coercion" behavior we want. This is still a hack but potentially less of a hack.

  4. Remove the hack entirely and permit if &true { ... } to type check.

    This is a semantic change to the language but perhaps a desirable one. I (@Centril) believe it would facilitate usability since there are cases where you call a function and end up with &bool, e.g. indexing a slice of bools. Right now you would have to deref the returned &bool to make it work. I also think it is a semantic simplification of the language since if becomes more of a pure in-language desugaring than before. cc @rust-lang/lang about this idea of applying default-match-bindings to if conditions.

For more context, see #59288 (comment) and #59288 (review).

@Centril Centril added C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 10, 2019
@Centril Centril self-assigned this May 10, 2019
@Centril Centril added the F-let_chains `#![feature(let_chains)]` label Oct 10, 2019
@c410-f3r
Copy link
Contributor

Some things are outdated. For example, MatchSource::IfDesugar does not exist anymore.

@Mark-Simulacrum
Copy link
Member

I'm going to go ahead and close this per #53667 (comment); it seems like the majority or all of this is no longer the case today. I tried poking around in the compiler to find related code but didn't see anything that stood out as particularly relevant. hir::ExprKind has an If variant today, for example.

@Mark-Simulacrum Mark-Simulacrum closed this as not planned Won't fix, can't repro, duplicate, stale Aug 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. F-let_chains `#![feature(let_chains)]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants