Add more rules about when expressions diverge#2186
Conversation
|
@jackh726 Would you be able to review this? |
jackh726
left a comment
There was a problem hiding this comment.
Seems good to me. Definitely the expression list in divergence.md is pretty long now.
| > The future type that rustc generates is roughly equivalent to an enum with one variant per `await` point, where each variant stores the data needed to resume from its corresponding point. | ||
|
|
||
| r[expr.block.async.diverging] | ||
| An async block expression does not itself [diverge], but evaluating the future (such as through `await`) diverges if the output type is the [never type]. |
There was a problem hiding this comment.
The latter half is covered by expr.await.diverging. Is the duplication worth it?
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| ## Divergence | ||
|
|
||
| r[asm.diverging.noreturn] | ||
| The `asm!` macro [diverges] if it uses the [`noreturn`] option. |
There was a problem hiding this comment.
| The `asm!` macro [diverges] if it uses the [`noreturn`] option. | |
| The `asm!` macro [diverges] if it uses the [`noreturn`] option and no `label` block returns unit. |
There was a problem hiding this comment.
Our rule says:
r[asm.options.supported-options.noreturn]
noreturn: The assembly code does not fall through; behavior is undefined if it does. It may still jump tolabelblocks. If anylabelblocks return unit, theasm!block will return unit. Otherwise it will return!(never). As with a call to a function that does not return, local variables in scope are not dropped before execution of the assembly code.
Interestingly, it doesn't actually say whether or not it diverges. But, logically, it should not, and testing bears this out.
The noreturn option is misnamed now that we've stabilized labeled blocks. It should really be nofallthrough or similar — it's asserting only that we don't fall through the assembly code. If we jump from that code to a labeled block, that labeled block can return normally without diverging.
| r[expr.path.diverging] | ||
| A path expression does not [diverge]. | ||
|
|
||
| <!-- | ||
| TODO: When stabilizing never type, this changes to: | ||
|
|
||
| A path expression [diverges] if the path resolves to a value with the [never type] and it is a place expression that is guaranteed to constitute a read. | ||
| --> |
There was a problem hiding this comment.
It's actually possible to observe this on stable Rust. E.g.:
fn phantom_call<T>(_: impl FnOnce(T) -> T) {}
fn main() {
let _ = phantom_call(|x| -> ! {
let _x = x; // OK.
});
let _ = phantom_call(|x| -> ! {
let _ = x; // ERROR: Mismatched types.
});
}Let's also adjust this language to use the "value is guaranteed to be read" phrasing used elsewhere.
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
rust-lang#2067 added a chapter on divergence along with rules for some of the more subtle expressions like `if` or `match`. This adds rules for almost all the rest of the expressions. Most of these are pretty straightforward propagation rules which could be left implicit. It may seem a little excessive to have a separate rule for each one, but I think the consistency is worth it because there are various subtleties. This also may help us be clear about these things when adding new expressions to the language to ensure we think about the divergence rules. This does not cover const expressions (const blocks, static, const, array repeat, etc.) because I still do not yet fully know how those should be documented (see rust-lang#2153). I'm not entirely excited by having the long list of rules in the divergence chapter, mostly because of the length. However, I think it is helpful to cross-index these kinds of things. There are some interesting subtleties that I noticed: - `while` loops cannot diverge. It's a little surprising to me (I would expect the condition to allow it to diverge). I suspect this is due to the desugaring to a loop expression using `break`, which does not diverge. - Updated details for let-chains. References for the implementation: - Place expression with never must be read: https://github.com/rust-lang/rust/blob/59fd4ef94daa991e6797b5aa6127e824f3067def/compiler/rustc_hir_typeck/src/expr.rs#L318-L326 - `break` is never: https://github.com/rust-lang/rust/blob/59fd4ef94daa991e6797b5aa6127e824f3067def/compiler/rustc_hir_typeck/src/expr.rs#L819-L820 - `continue` is never: https://github.com/rust-lang/rust/blob/0376d43d443cba463a0b6a6ec9140ea17d7b7130/compiler/rustc_hir_typeck/src/expr.rs#L861 - `return` is never: https://github.com/rust-lang/rust/blob/0376d43d443cba463a0b6a6ec9140ea17d7b7130/compiler/rustc_hir_typeck/src/expr.rs#L921 - `if` divergence: https://github.com/rust-lang/rust/blob/59fd4ef94daa991e6797b5aa6127e824f3067def/compiler/rustc_hir_typeck/src/expr.rs#L1236-L1242 - `loop`: https://github.com/rust-lang/rust/blob/59fd4ef94daa991e6797b5aa6127e824f3067def/compiler/rustc_hir_typeck/src/expr.rs#L1450-L1456 - `asm!`: https://github.com/rust-lang/rust/blob/59fd4ef94daa991e6797b5aa6127e824f3067def/compiler/rustc_hir_typeck/src/expr.rs#L3684 - `match`: https://github.com/rust-lang/rust/blob/59fd4ef94daa991e6797b5aa6127e824f3067def/compiler/rustc_hir_typeck/src/_match.rs#L19-L178 - block break: https://github.com/rust-lang/rust/blob/59fd4ef94daa991e6797b5aa6127e824f3067def/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs#L1167-L1171 - lazy bool: https://github.com/rust-lang/rust/blob/0376d43d443cba463a0b6a6ec9140ea17d7b7130/compiler/rustc_hir_typeck/src/op.rs#L111 Closes rust-lang#2152
|
(For the record, I am finding more mistakes here and am working on them.) |
#2067 added a chapter on divergence along with rules for some of the more subtle expressions like
iformatch. This adds rules for almost all the rest of the expressions.Most of these are pretty straightforward propagation rules which could be left implicit. It may seem a little excessive to have a separate rule for each one, but I think the consistency is worth it because there are various subtleties. This also may help us be clear about these things when adding new expressions to the language to ensure we think about the divergence rules.
This does not cover const expressions (const blocks, static, const, array repeat, etc.) because I still do not yet fully know how those should be documented (see #2153).
I'm not entirely excited by having the long list of rules in the divergence chapter, mostly because of the length. However, I think it is helpful to cross-index these kinds of things.
There are some interesting subtleties that I noticed:
whileloops cannot diverge. It's a little surprising to me (I would expect the condition to allow it to diverge). I suspect this is due to the desugaring to a loop expression usingbreak, which does not diverge.References for the implementation:
breakis never: https://github.com/rust-lang/rust/blob/59fd4ef94daa991e6797b5aa6127e824f3067def/compiler/rustc_hir_typeck/src/expr.rs#L819-L820continueis never: https://github.com/rust-lang/rust/blob/0376d43d443cba463a0b6a6ec9140ea17d7b7130/compiler/rustc_hir_typeck/src/expr.rs#L861returnis never: https://github.com/rust-lang/rust/blob/0376d43d443cba463a0b6a6ec9140ea17d7b7130/compiler/rustc_hir_typeck/src/expr.rs#L921ifdivergence: https://github.com/rust-lang/rust/blob/59fd4ef94daa991e6797b5aa6127e824f3067def/compiler/rustc_hir_typeck/src/expr.rs#L1236-L1242loop: https://github.com/rust-lang/rust/blob/59fd4ef94daa991e6797b5aa6127e824f3067def/compiler/rustc_hir_typeck/src/expr.rs#L1450-L1456asm!: https://github.com/rust-lang/rust/blob/59fd4ef94daa991e6797b5aa6127e824f3067def/compiler/rustc_hir_typeck/src/expr.rs#L3684match: https://github.com/rust-lang/rust/blob/59fd4ef94daa991e6797b5aa6127e824f3067def/compiler/rustc_hir_typeck/src/_match.rs#L19-L178Closes #2152