Option::get_or_insert*: Ensure dead branch isn't taken

[shahn]

This doesn't really mean any kind of speedup, but it is aligned with the intention of this API.

[rust-highfive]

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

[KalitaAlexey]

@shahn,
What is the difference?

[shahn]

unreachable!() is a macro that causes a panic when it is executed, so it allows you to codify your belief that a certain code path will not be taken (and if it will, you'll learn via the panic). The unreachable intrinsic is a way to tell LLVM that it should assume that the code path can never be taken and generate code accordingly. If you're wrong about your assessment, you will not get a runtime panic but rather undefined behaviour, so it's a dangerous operation (and thus unsafe to use). Let me know if you need more explanation :)

[KalitaAlexey]

@shahn,
It is actually very good explained.
Thank you.

[alexcrichton]

Do you have an example of this leading to and improvement? If not I'd be tempted to leave the panic because LLVM in theory should be able to easily optimize it out and it's otherwise better to reduce unsafe code

[aturon]

I agree with @alexcrichton, I'd prefer not to introduce unsafety unless there's a clear benefit.

[KalitaAlexey]

@alexcrichton, @aturon,
How is it unsafe?
This arm is really unreachable, isn't it?
The only way is a data race, isn't it?

[aturon]

@KalitaAlexey I mean unsafe in the sense of unsafe blocks. While I agree that this use seems fine in principle, we tend to avoid any use of unsafe code in std unless there's a compelling reason to do so.

[KalitaAlexey]

@aturon,
I got reason, but I didn't get motivation.
I understand that it is about metrics.
The lesser unsafe the better.

[shahn]

Ah. I guess I was not aware of the policy to reduce unsafe code in the stdlib, I thought it was where it should go. I have some sample code that doesn't eliminate the branch, but only in debug mode, not release. I haven't tried very hard to find a test case for release mode though. Please feel free to close this if you think the branch in debug mode is not a good enough justification :)

[alexcrichton]

Ok, in that case yeah I'm going to close this for now. If there's a case where release mode doesn't optimize away the branch feel free to resubmit!