Skip to content

If-Let-Else and Let-Else doesn't produce same result which creates UB. #134766

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

Closed
Azvanzed opened this issue Dec 25, 2024 · 2 comments
Closed

If-Let-Else and Let-Else doesn't produce same result which creates UB. #134766

Azvanzed opened this issue Dec 25, 2024 · 2 comments
Labels
T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@Azvanzed
Copy link

I tried this code:

static MUTEX: spin::Mutex<Option<fn()>> = spin::Mutex::new(None);

// SAMPLE A
fn main() {
    let Some(value) = *MUTEX.lock() else {
         unreachable!("Mutex not set");
     };

     value();
 }

// SAMPLE B
fn main() {
    if let Some(value) = *MUTEX.lock() {
        value();
    } else {
        unreachable!("Mutex not set");
    }
 }

I expected to see this happen:
A. calls the value function then the mutex unlocks. (IDA decompilation)

Image

Instead, this happened:
B. mutex unlocks then calls the value function. (IDA decompilation)

Image

Meta

The bug does occur in nightly.

rustc --version --verbose:

rustc 1.83.0 (90b35a623 2024-11-26)
binary: rustc
commit-hash: 90b35a6239c3d8bdabc530a6a0816f7ff89a0aaf
commit-date: 2024-11-26
host: x86_64-pc-windows-msvc
release: 1.83.0
LLVM version: 19.1.1
Backtrace

Not needed since this is visible statically.

@Azvanzed Azvanzed added the C-bug Category: This is a bug. label Dec 25, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Dec 25, 2024
@Azvanzed Azvanzed changed the title If and If-Else doesn't produce same result which creates UB. If-Let-Else and Let-Else doesn't produce same result which creates UB. Dec 25, 2024
@traviscross traviscross added T-lang Relevant to the language team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. C-bug Category: This is a bug. labels Dec 25, 2024
@traviscross
Copy link
Contributor

traviscross commented Dec 25, 2024

This isn't a bug in rustc. In your code, since you didn't separately assign the mutex guard to some binding, it's just a temporary, and those don't generally outlive the statement in which they are created (with some caveats about tail expressions). See, e.g.:

That said, we agree this is an unfortunate discrepancy between if-let and let-else, and it's something we're thinking about. If we were to change it (over an edition), it would likely be in the direction of making if-let more like let-else, i.e. continuing the direction we took in Rust 2024 to shorten the scope of temporaries.

In any event, it's probably advisable in cases like this to assign the mutex guard to a binding so as to be more explicit about the intended scope.

@traviscross traviscross closed this as not planned Won't fix, can't repro, duplicate, stale Dec 25, 2024
@Azvanzed
Copy link
Author

Interesting; I've never looked at it this way but now that you've explained it it makes more sense.

Either way making it more similar to each other would be neat to avoid someone else falling in the same mistake i did.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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