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

Semicolon after diverging try operator shouldn't matter #106357

Open
clarfonthey opened this issue Jan 1, 2023 · 8 comments
Open

Semicolon after diverging try operator shouldn't matter #106357

clarfonthey opened this issue Jan 1, 2023 · 8 comments
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues. F-never_type `#![feature(never_type)]` T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@clarfonthey
Copy link
Contributor

clarfonthey commented Jan 1, 2023

Originally mentioned in this comment: #96373 (comment)

Essentially, semicolons after other diverging expressions are allowed. For example:

fn works() -> Result<bool, i32> {
    return Err(1);
}

Here, the compiler is clever enough to note that because return Err(1) always diverges, adding a semicolon after doesn't matter. However, the below code which is equivalent to this does not work properly:

fn breaks() -> Result<bool, i32> {
    Err(1)?;
}

Here's a playground link containing the above two examples: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=af894bab48348897a421a9402856b71f

The exact compiler error:

error[[E0308]](https://doc.rust-lang.org/nightly/error-index.html#E0308): mismatched types
 --> src/lib.rs:5:16
  |
5 | fn breaks() -> Result<bool, i32> {
  |    ------      ^^^^^^^^^^^^^^^^^ expected enum `Result`, found `()`
  |    |
  |    implicitly returns `()` as its body has no tail or `return` expression
6 |     Err(1)?;
  |            - help: remove this semicolon to return this value
  |
  = note:   expected enum `Result<bool, i32>`
          found unit type `()`

For more information about this error, try `rustc --explain E0308`.
error: could not compile `playground` due to previous error
@andersk
Copy link
Contributor

andersk commented Jan 1, 2023

My understanding is that this is related to the fallback issues that have been preventing stabilization of the ! type. See

@clarfonthey
Copy link
Contributor Author

I was kind of afraid that's the case.

If this seems too separate from the existing issues for those, feel free to close it! Although I guess that this is another example that might be useful to test for.

@aDotInTheVoid
Copy link
Member

I'm not sure this is a bug,

fn breaks() -> Result<bool, i32> {
    Err(1)?;
}

gets desugared to

fn breaks() -> Result<bool, i32> {
    match Err(1) {
         Ok(x) => x,
         Err(e) => return Err(e.into())
    };
}

Unless you are relying on the compiler to eliminate the Ok branch as unreachable, which isn't guaranteed to happen, the whole expression isn't diverging (in the mind of the type system).

I don't think the compiler should be required to do this elimination, as involving optimizations in exausiveness checks seems like it could get unintuitive fast, and expose many people to accidental breakage.

@scottmcm scottmcm added the T-types Relevant to the types team, which will review and decide on the PR/issue. label Jan 2, 2023
@clarfonthey
Copy link
Contributor Author

So, I would agree with this, except for the fact that the compiler could absolutely infer the type of Err(1) to be Result<!, i32> and thus treat the Ok branch as diverging. But that leads into the ! fallback problems that were mentioned.

@est31
Copy link
Member

est31 commented Jan 4, 2023

Related: rust-lang/style-team#165 (comment)

@kadiwa4
Copy link
Contributor

kadiwa4 commented Jan 5, 2023

Also related: feature exhaustive_patterns #51085

@Enselic Enselic added the F-never_type `#![feature(never_type)]` label Jun 10, 2024
@WaffleLapkin
Copy link
Member

So. Mrgh. This issue is indeed with the fallback.

fn breaks() -> Result<bool, i32> {
    match Err(1) {
         Ok(x) => x,
         Err(e) => return Err(e.into())
    };
}

This desugaring causes the type of the match to be inferred to (), due to the fallback issue. I would expect that this will start "working" in 2024 edition.

Note however that I would highly advice against writing Err(x)?. The type of Ok variant should no be inferred from the ?. IMO it's a bug in ?'s desugaring. Similarly, allowing ; after diverging expressions is somewhat confusing. I was discussing with T-lang if we can break both of those things, but it did not cut the 2024 edition (and not yet decided that we actually want to break these things).

@WaffleLapkin WaffleLapkin added C-discussion Category: Discussion or questions that doesn't represent real issues. and removed C-bug Category: This is a bug. labels Jun 13, 2024
@traviscross
Copy link
Contributor

traviscross commented Jun 14, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues. F-never_type `#![feature(never_type)]` T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

9 participants