Add a chapter on divergence #2067
Conversation
rustbot
added
the
S-waiting-on-review
rust-cloud-vms
bot
force-pushed
the
reference-type-inference-divergence
branch
from
3b0ec86 to
920cec4
Compare
rust-cloud-vms
bot
force-pushed
the
reference-type-inference-divergence
branch
from
920cec4 to
d020656
Compare
|
Thanks for the PR @jackh726; I can tell this was written carefully. It will be good to get more of this documented. In particular, it'll be good to have the fallback behavior documented. I'll leave some notes inline. Probably we'll want to move some things around. Adding more examples -- even beyond what I'll note specifically inline -- would be particularly good for this material. It's helpful when each rule has one or more concise and testable examples demonstrating exactly what the rule means to express. |
|
Thanks for the review @traviscross. Good points here, I'll work on sorting through them today/tomorrow. Happy to jump on a call at some point too, if you think any of these could use further discussion. |
rust-cloud-vms
bot
force-pushed
the
reference-type-inference-divergence
branch
2 times, most recently
from
a11338f to
a9d8264
Compare
|
@rustbot author |
|
Error: Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #triagebot on Zulip. |
traviscross
added
S-waiting-on-author
rust-cloud-vms
bot
force-pushed
the
reference-type-inference-divergence
branch
3 times, most recently
from
63775d7 to
f9cf9f5
Compare
nikomatsakis
left a comment
nikomatsakis
left a comment
There was a problem hiding this comment.
Some drive-by comments.
|
Okay, I have addressed the reviews to the best of my ability. regarding moving regarding inlining other rules (#2067 (comment)): regarding adding more examples: @rustbot review |
rustbot
added
S-waiting-on-review
rust-cloud-vms
bot
force-pushed
the
reference-type-inference-divergence
branch
2 times, most recently
from
4040ed0 to
4f34b11
Compare
src/items/functions.md
Outdated
|
|
||
| r[items.fn.implicit-return] | ||
| If the output type is not explicitly stated, it is the [unit type]. | ||
| If the output type is not explicitly stated, it is the [unit type]. However, if the block expression is not [diverging](../divergence.md), then the output type is instead [`!`](../types/never.md). |
There was a problem hiding this comment.
In talking over this PR in the office hours, @ehuss and I puzzled over this rule. Presumably the not is misplaced, but even with that removed, we weren't clear on how to assign meaning to this given, of course:
fn f() {
loop {} // Diverging expression.
}
let _ = || -> ! { f() }; //~ ERROR mismatched types
let _: fn() -> ! = f; //~ ERROR mismatched typesThere was a problem hiding this comment.
Ah, this is a good catch. Indeed, the additional language is not correct for the function signature.
What I had in mind when writing this was that when type checking the function body the implicit return is ! if it is diverging. This is technically covered by my changes to expr.block.type.diverging, so I'm not sure why I thought that these changes were also needed.
I think it was motivated by this example. I guess, maybe there is an equivalent statement missing about the implicit "final type" of a block (which is by default () and otherwise ! when diverging).
There was a problem hiding this comment.
Should this change be reverted, then?
There was a problem hiding this comment.
Well, probably. But I wonder if there should be some additional text added somewhere that the type of the function block may not actually be the return type of the function, but rather must be coercible into the return type of the function.
There was a problem hiding this comment.
I believe that coerce.site.return specifies that the function body is coerced to the function's return type.
I can't find the subsequent part that the function body's type must match? the function's return type (after coercion). Type checking in general isn't documented (#595), and I'm not sure which approach will make sense for that (for example, would there be a dedicated type checking chapter, or would the type checking rules be sprinkled throughout, or a mixture of both?).
I've removed this change in my local branch which I hope to push up soon. I've added this to my notes for future followup.
|
@ehuss and I talked over this PR in the lang-docs office hours this week. We're happy to be getting this documented; thanks to @jackh726 for that. To set expectations here: @ehuss is planning to make a series of editorial revisions. He'll push those onto this branch. Our feeling was that, with these editorial revisions, this is likely to merge soon after. Due to the upcoming holidays, this will happen in 2026. |
This test is meant to show that matching a diverging place against the wildcard pattern does not cause a read and therefore does not cause the block to diverge. This was shown by ascribing the return type of the function to unit. But due to never-to-any coercion, that will always type check, even if the read does take place. Let's instead ascribe the function return type to never and assert that the test must fail. This way, the test would fail if the read did occur.
This tries to add consistency to if, match, break, continue, return, and loop in how the rule for divergence (and their type).
Also consistently uses periods in comments.
This helps make it clear that this is using special syntax that is normally not allowed. Also, unhide `make` so it isn't so mysterious what it is.
This particular example can be written with stable Rust, and I don't think it detracts too much from what it is trying to illustrate.
The addition here isn't quite right. See discussion at https://github.com/rust-lang/reference/pull/2067/changes#r2630177707. As a later followup, we should document that the type of the function block must be coercible into the return type of the function. (Or if there is a more generalized way to state that.)
Also clean up the links.
Rule names shouldn't be nested under other rules.
I'm not sure what happened here. The rule was originally expr.match.type.diverging.conditional, then it was requested to change to expr.match.diverging. I think that suggestion fits well here, so let's go with it.
This starts off the chapter with a direct definition of what a diverging expression is (see discussion in https://github.com/rust-lang/reference/pull/2067/changes#diff-11f5e85172f632195dd6a9a80daa852911fd3d66e788177a3bb39f6c5d7f21cfR10 about this definition). This also adds a very simple example showing divergence. Maybe not the greatest illustration, but I'd prefer to have at least a little something.
This was asked to be changed or removed in rust-lang#2067 (comment) and was removed in 5eef4bc, but I think it is useful to include the links to the appropriate rules that are relevant here. This doesn't exactly duplicate any of the rules, it is just forwarding links, and this is something we do all over the Reference. Also add a mention about panics. I'm not sure if putting this in a note is really the right approach. However, we don't directly document `panic!` yet. We are currently in discussion about adding documentation for built-in macros.
Add a rule identifier to this particular statement. Also some slight rewording for clarity.
For the most part, this seemed to be restating the expr.block.diverging rule. (At least, I couldn't determine if it was expressing something distinct.) This is an interesting property, but one that I think is already covered. So, relegate it to a note linking to the relevant text.
This lifts up this sentence (which was missing a rule), and clarifies that this only applies to loops and labeled block expressions.
I'm not entirely sure how we want to handle this list going forward, because most expressions have some kind of divergence behavior.
ehuss
force-pushed
the
reference-type-inference-divergence
branch
from
8562732 to
dd3cb1a
Compare
|
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. |
7 participants
It was little tricky when trying to describe diverging blocks. The compiler's implementation maintains sort of a "global state" when checking an expression and sub-expressions, which it resets on conditional things. Semantically, I think the way I worded it is much clearer than trying to match the implementation.
Happy to hear any specific feedback, Lcnr made did an initial review pass in rust-lang/project-goal-reference-expansion@4079171, so the second commit tries to address that.
Closes #271
Closes #1033