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

Add let_chains references #1179

Merged
merged 2 commits into from
Aug 2, 2022
Merged

Add let_chains references #1179

merged 2 commits into from
Aug 2, 2022

Conversation

c410-f3r
Copy link
Contributor

src/expressions/if-expr.md Outdated Show resolved Hide resolved
@nikomatsakis
Copy link
Contributor

Thanks @c410-f3r

Comment on lines -93 to -113
An `if let` expression is equivalent to a [`match` expression] as follows:

<!-- ignore: expansion example -->
```rust,ignore
if let PATS = EXPR {
/* body */
} else {
/*else */
}
```

is equivalent to

<!-- ignore: expansion example -->
```rust,ignore
match EXPR {
PATS => { /* body */ },
_ => { /* else */ }, // () if there is no else
}
```

Copy link
Contributor Author

@c410-f3r c410-f3r Apr 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It hasn't been the case for quite some time-> rust-lang/rust#80357

Besides, it is strange to document an inner implementation detail

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is necessarily there as an implementation detail, but as a way to specify how it logically desugars. Then, one can refer to the match documentation to know how things work wrt scoping and temporaries and such. Otherwise, there might need to be some duplication of the documentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My mistake, code desugaring only changed for while expressions. for expressions stay the same

Comment on lines 13 to 14
> &nbsp;&nbsp; ( [_Expression_]<sub>_except struct expression_</sub>
> | `let` [_Pattern_] `=` [_Expression_]<sub>_except struct expression_</sub> )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason this is surrounded in parentheses?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

Comment on lines -93 to -113
An `if let` expression is equivalent to a [`match` expression] as follows:

<!-- ignore: expansion example -->
```rust,ignore
if let PATS = EXPR {
/* body */
} else {
/*else */
}
```

is equivalent to

<!-- ignore: expansion example -->
```rust,ignore
match EXPR {
PATS => { /* body */ },
_ => { /* else */ }, // () if there is no else
}
```

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is necessarily there as an implementation detail, but as a way to specify how it logically desugars. Then, one can refer to the match documentation to know how things work wrt scoping and temporaries and such. Otherwise, there might need to be some duplication of the documentation.

if let PAT = EXPR && EXPR { .. }
## Chains of expressions

It is possible to chain multiple expressions into a single statement to avoid nested declarations.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think "statement" here is a little confusing. This also doesn't really present any context, or explain how to chain or how chains work. We generally don't use code blocks as a replacement for explaining how something works, but rather as a helpful illustration and example.

Can you try to add some more detail here? I would expect at least a couple paragraphs explaining what chaining is, what the syntax is, what the semantics are, that you can combine boolean values and let pattern bindings, how combining multiple pattern bindings works, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrote what was suggested in #1179 (comment)

@@ -126,30 +104,43 @@ if let E::X(n) | E::Y(n) = v {
```

The expression cannot be a [lazy boolean operator expression][_LazyBooleanOperatorExpression_].
Use of a lazy boolean operator is ambiguous with a planned feature change of the language (the implementation of if-let chains - see [eRFC 2947][_eRFCIfLetChain_]).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this also retain the old note about how to deal with use of lazy boolean expressions when combined with if let? Maybe something like:

The scrutinee expression cannot be a [lazy boolean operator expression] due to ambiguity and precedence with [chains of expressions].
If a lazy boolean operator expression is needed, then parentheses can be used. For example:

if let PAT = ( EXPR && EXPR ) { .. }
if let PAT = ( EXPR || EXPR ) { .. }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done without invalid snippets

Comment on lines +112 to +111
```rust
fn single() {
let outer_opt = Some(Some(1i32));

if let Some(inner_opt) = outer_opt
&& let Some(number) = inner_opt
&& number == 1
{
println!("Peek a boo");
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This example isn't super clear to me. Can it perhaps be broken up in two and explain what it is illustrating? Maybe something like this:

Suggested change
```rust
fn single() {
let outer_opt = Some(Some(1i32));
if let Some(inner_opt) = outer_opt
&& let Some(number) = inner_opt
&& number == 1
{
println!("Peek a boo");
}
}
The following is an example of chaining multiple expressions, mixing `let` bindings and boolean expressions, and with expressions able to reference pattern bindings from previous expressions:
```rust
let outer_opt = Some(Some(1i32));
if let Some(inner_opt) = outer_opt
&& let Some(number) = inner_opt
&& number == 1
{
println!("Peek a boo");
}
```
The above is equivalent to the following without using expression chains:
```rust

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

```

The only remark is the fact that parenthesis (`if (let Some(a) = opt && (let Some(b) = a)) && b == 1`) and `||` operators (`if let A(x) = e1 || let B(x) = e2`) are not currently supported.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The note about || not being allowed is already stated above, so I don't think it needs to be repeated here.

And the grammar doesn't actually define if (let …) as being valid, so this is a little confusing. I left a comment on the stabilization PR, but it seems like there is some disconnect between what this documentation is stating and how it actually works.

Generally the reference doesn't specify what may or may not be supported in the future, so I'm not sure it is worth stating anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

## `if` expressions
## Syntax

The same syntax is used by `if`, `if let` and chains of expressions.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was perhaps intending maybe a little bit of a greater rewrite here, as I'm not sure I entirely see if and if let being fundamentally different. Is there a reason to keep them as separate sections?

I think what I was thinking is it would say something like:

The condition operand can be a list of one or more IfLet expressions separated by &&.
If all of the boolean values evaluate to true and all patterns match their scrutinee value, the consequent block is executed and…
…and include a description of pattern bindings, scopes, && priority, etc.

I guess I'm not seeing why they would be kept separate. if EXPR {} just seems to be a special case where there is an if expression with a single boolean value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have enough free time to perform a greater rewrite

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mind if I push some changes to your branch?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not at all

Comment on lines 9 to 10
> &nbsp;&nbsp; `if` _IfLetList_ [_BlockExpression_]\
> &nbsp;&nbsp; ( `else` _IfLetList_ [_BlockExpression_] )<sup>\?</sup>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This grammar doesn't look correct to me. The else part seems to have multiple issues:

  • Doesn't allow if foo {} else {} where it is an unconditional else block.
  • Doesn't allow if foo {} else if bar {} where there is an else if.
  • Seems to allow if foo {} else bar {} which isn't valid.

The expression cannot be a [lazy boolean operator expression][_LazyBooleanOperatorExpression_].
Use of a lazy boolean operator is ambiguous with a planned feature change of the language (the implementation of if-let chains - see [eRFC 2947][_eRFCIfLetChain_]).
When lazy boolean operator expression is desired, this can be achieved by using parenthesis as below:
The expression cannot be a [lazy boolean operator expression][_LazyBooleanOperatorExpression_]. Scrutinee expressions also cannot be a [lazy boolean operator expression] due to ambiguity and precedence with [chains of expressions].
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These links seem to be broken.

Also, can you keep it to one sentence per line?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@c410-f3r c410-f3r force-pushed the let-chains branch 2 times, most recently from bd2f4b2 to 68f3e4a Compare July 25, 2022 11:14
c410-f3r and others added 2 commits July 25, 2022 15:42
This updates the `if` and `while` expressions so that they each are
presented as a single expression kind with multiple condition operators
instead of being logically separated from their `let` counterparts.

This also includes various fixes and additions.
Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

I pushed the changes that I was thinking of in terms of unifying if/if let and while/while let.

@ehuss ehuss merged commit f3d3953 into rust-lang:master Aug 2, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 2, 2022
Update books

## reference

2 commits in a92be0fef439b3d8e0468d82cb24812d303520a0..f3d3953bf3b158d596c96d55ce5366f9f3f972e9
2022-07-21 19:01:23 -0700 to 2022-08-01 17:17:37 -0700
- Add `let_chains` references (rust-lang/reference#1179)
- Remove outdated warning (rust-lang/reference#1243)

## rust-by-example

18 commits in 3155db49b0d57cd82c65456ac210b69ecec5ccb1..ee342dc91e1ba1bb1e1f1318f84bbe3bfac04798
2022-07-05 20:35:53 -0300 to 2022-07-27 11:06:36 -0300
- Closure inferred twice (rust-lang/rust-by-example#1588)
- fix a syntax bug in example assembly (rust-lang/rust-by-example#1511)
- Minor grammar change in src/std/rc.md paragraph 2 (rust-lang/rust-by-example#1586)
- Fix typo in asm.md (rust-lang/rust-by-example#1585)
- Fix incorrect padding in fixed-width print (rust-lang/rust-by-example#1584)
- Update print.md (rust-lang/rust-by-example#1582)
- add-chapter-on-defaults (rust-lang/rust-by-example#1580)
- Fix typo (rust-lang/rust-by-example#1579)
- fix a compile error (rust-lang/rust-by-example#1578)
- Suggest using mod.rs pattern to share test code (rust-lang/rust-by-example#1577)
- fix a compile error in iter_any.md (rust-lang/rust-by-example#1576)
- Mention attribute like macros in attributes.md (rust-lang/rust-by-example#1574)
- Update exercise to be clearer (rust-lang/rust-by-example#1573)
- fixes link for turbofish in testcase_mapreduce.md (rust-lang/rust-by-example#1572)
- Fix inconsistency between comment and code in hello/print.md (rust-lang/rust-by-example#1571)
- Fixes a typo in print.md (rust-lang/rust-by-example#1570)
- into_iter-moves-elements (rust-lang/rust-by-example#1569)
- Fix a typo in print.md (rust-lang/rust-by-example#1568)

## rustc-dev-guide

16 commits in d5201cddace979b299ec1bf9fd8997338151aa9d..04f3cf0bb2f5a6ee2bfc4b1a6a6cd8c11d1c5531
2022-07-21 04:48:49 +0200 to 2022-07-31 07:46:57 +0200
- address review comment
- accept review suggestion
- try address review comments
- summary of chapter
- Update src/building/compiler-documenting.md
- revamp doc-build chapter
- minor fixes
- Prefer relative links
- Fix the link to clippy docs
- Fix the link to `ResolverAstLowering`
- Fix the link to `ProcMacro` trait
- Fix the link to `Lazy&lt;T&gt;`
- Add instructions to fix build errors in std after adding a new target
- Document how to build a cross-compiler
- Add documentation about Microsoft provided debuggers and CodeView/PDB… (rust-lang/rustc-dev-guide#1406)
- rust-analyzer is now a subtree

## embedded-book

2 commits in 766979590da8100998f0d662499d4a901d8d1640..befe6840874311635c417cf731377f07234ee373
2022-07-04 09:13:58 +0000 to 2022-07-25 07:51:14 +0000
- Updated instructions for running first Hardware example  (rust-embedded/book#323)
- Improved ligability for hardware.md  (rust-embedded/book#324)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants