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 information about || and && to grammar describing while let. #772

Merged
merged 2 commits into from
Apr 8, 2020
Merged

Add information about || and && to grammar describing while let. #772

merged 2 commits into from
Apr 8, 2020

Conversation

steffahn
Copy link
Member

@steffahn steffahn commented Mar 4, 2020

Also fix italics to be consistent with other pages.
See #771. I also copied and adapted the corresponding paragraph from the if let page.
Edit: Instead of copying the paragraph from if let mentioning the unstable if-let chains feature, now the copy as well as the original paragraph are removed.

Also fix italics to be consistent with other pages.
Comment on lines 126 to 145
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:

<!-- ignore: psuedo code -->
```rust,ignore
// Before...
while let PAT = EXPR && EXPR { .. }

// After...
while let PAT = ( EXPR && EXPR ) { .. }

// Before...
while let PAT = EXPR || EXPR { .. }

// After...
while let PAT = ( EXPR || EXPR ) { .. }
```
Copy link
Contributor

Choose a reason for hiding this comment

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

This is user advice and discussion of unstable features, which is off-topic for the reference.

Copy link
Member Author

Choose a reason for hiding this comment

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

Then perhaps it should also be removed from the page about if expressions?

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably yes, cc @ehuss

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 it's fine to keep this information, though place it in a "note" block. I don't see a reason to never talk about such things, since it is relevant here (explaining a strange peculiarity of the language). The C++ reference does the same with side notes mentioning future changes or how to disambiguate things that aren't obvious.

The information only needs to live in one place. if let would be fine, with while let linking to if let for more information. I might even trim while let even more to remove redundant information, but doesn't necessarily need to be done in this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Putting it in a note and deduplicating seems like a reasonable resolution to me.

@steffahn
Copy link
Member Author

steffahn commented Mar 4, 2020

@ehuss I'm questioning if "Expression_except struct or lazy boolean operator expression" is even accurate. It seems like the compiler is currently, internally, for parsing and some typechecking, treating

let PAT = EXPR

just like an expression of type bool with precedence between && and ==. Hence not only lazy boolean operators but also binary and postfix .. and ..= don't work in if let expression without parantheses.

@ehuss
Copy link
Contributor

ehuss commented Mar 4, 2020

Hm, I don't have the answer for that, @Centril should. It looks like let has special precedence rules (parse_let_expr) from if let (parse_assoc_expr). I'm not too familiar with how that works.

I general, the reference is missing lots of information on precedence rules and disambiguations, so don't be too surprised about details like this not being covered (though they should!).

@Centril
Copy link
Contributor

Centril commented Mar 7, 2020

@ehuss I'm questioning if "Expression__except struct or lazy boolean operator expression_" is even accurate. It seems like the compiler is currently, internally, for parsing and some typechecking, treating

This only applies to parsing and AST level validation. Type checking (done on HIR) does not care about parenthesis and precedences. The old note is probably outdated by now.

Hm, I don't have the answer for that, @Centril should. It looks like let has special precedence rules (parse_let_expr) from if let (parse_assoc_expr). I'm not too familiar with how that works.

Yes, the parser treats let pat = scrutinee as an expression on its own, and specifically requires that scrutinee has a minimum precedence of one above the precedence of &&. The relevant code is:

/// In `let p = e`, operators with precedence `<=` this one requires parenthesis in `e`.
pub fn prec_let_scrutinee_needs_par() -> usize {
    AssocOp::LAnd.precedence()
}

    /// Parses a `let $pat = $expr` pseudo-expression.
    fn parse_let_expr(&mut self, attrs: AttrVec) -> PResult<'a, P<Expr>> {
        let pat = self.parse_top_pat(GateOr::No)?;
        self.expect(&token::Eq)?;
        let expr = self.with_res(Restrictions::NO_STRUCT_LITERAL, |this| {
            this.parse_assoc_expr_with(1 + prec_let_scrutinee_needs_par(), None.into())
        })?;

...so what we should do is document the minimum precedence requirement; everything else just falls out automatically. (We shouldn't document that let is an expression on its own, as that is still an unstable implementation detail.)

Copy link
Contributor

@Centril Centril left a comment

Choose a reason for hiding this comment

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

Let's merge this but also follow up re. the precedence issue.

@Centril Centril merged commit e8f420d into rust-lang:master Apr 8, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 14, 2020
Update books

## reference

13 commits in 89dd146..3ce94ca
2020-03-31 09:42:10 -0700 to 2020-04-11 17:00:27 +0200
- UB definition: mention size_of_val for determining a reference's span (rust-lang/reference#793)
- Emphasize that `#[inline]` in all shapes is a hint. (rust-lang/reference#791)
- Added missing comma to Unions reference (rust-lang/reference#790)
- Attributes are now allowed on `if` and `if let` (rust-lang/reference#783)
- `enum`s can now be `#[repr(transparent)]` (rust-lang/reference#741)
- Undocument `use` paths. (rust-lang/reference#665)
- Clarify pub(restricted) a bit (rust-lang/reference#481)
- Add information about || and && to grammar describing `while let`. (rust-lang/reference#772)
- Document the `automatically_derived` attribute. (rust-lang/reference#555)
- Fix unstable check. (rust-lang/reference#743)
- Use common script for link checking. (rust-lang/reference#765)
- Add a basic style guide. (rust-lang/reference#787)
- Size fo empty structs in C is zero byte. (rust-lang/reference#782)

## book

14 commits in c8841f2841a2d26124319ddadd1b6a245f9a1856..f5db319e0b19c22964398d56bc63103d669e1bba
2020-03-22 09:07:01 -0500 to 2020-04-13 08:06:03 -0500
- Update ch01-01-installation.md to require TLS 1.2 (rust-lang/book#2301)
- traits as parameters: Use references instead of moving the values (rust-lang/book#2239)
- Fix a broken link
- Update go docs link (rust-lang/book#2285)
- Add the farsi translation repo link (rust-lang/book#2283)
- Add the missing word to the sentence (ch06-2) (rust-lang/book#2278)
- Fixes hardcoded output (rust-lang/book#2276)
- Use rust-lang/rust linkchecker on CI. (rust-lang/book#2272)
- Add union to the list of keywords (rust-lang/book#2271)
- Clarify the wording (rust-lang/book#2256)
- Improve sentence flow (rust-lang/book#2255)
- Add missing apostrophe (rust-lang/book#2247)
- Update cargo profiles link. (rust-lang/book#2245)
- Add note about chapter 18 in chapter 6 (rust-lang/book#2238)

## rust-by-example

1 commits in a6638463efc7631bc0e8dc67ccd256d4e1b61f1a..c106d1683c3a2b0960f0f0fb01728cbb19807332
2020-04-06 09:39:03 -0500 to 2020-04-09 09:14:39 -0300
- Improve the conversion example (rust-lang/rust-by-example#1329)

## edition-guide

1 commits in 37f9e6848411188a1062ead1bd8ebe4b8aa16899..8204c1d123472cd17f0c1c5c77300ae802eb0271
2020-02-10 14:36:14 +0100 to 2020-04-09 18:55:50 -0700
- Fix forge link. (rust-lang/edition-guide#200)

## embedded-book

1 commits in d22a9c487c78095afc4584f1d9b4ec43529d713c..668fb07b6160b9c468f598e839c1e044db65de30
2020-03-04 09:46:30 +0000 to 2020-04-13 12:38:16 +0000
- Add triagebot configuration  (rust-embedded/book#232)
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 14, 2020
Update books

## reference

13 commits in 89dd146..3ce94ca
2020-03-31 09:42:10 -0700 to 2020-04-11 17:00:27 +0200
- UB definition: mention size_of_val for determining a reference's span (rust-lang/reference#793)
- Emphasize that `#[inline]` in all shapes is a hint. (rust-lang/reference#791)
- Added missing comma to Unions reference (rust-lang/reference#790)
- Attributes are now allowed on `if` and `if let` (rust-lang/reference#783)
- `enum`s can now be `#[repr(transparent)]` (rust-lang/reference#741)
- Undocument `use` paths. (rust-lang/reference#665)
- Clarify pub(restricted) a bit (rust-lang/reference#481)
- Add information about || and && to grammar describing `while let`. (rust-lang/reference#772)
- Document the `automatically_derived` attribute. (rust-lang/reference#555)
- Fix unstable check. (rust-lang/reference#743)
- Use common script for link checking. (rust-lang/reference#765)
- Add a basic style guide. (rust-lang/reference#787)
- Size fo empty structs in C is zero byte. (rust-lang/reference#782)

## book

14 commits in c8841f2841a2d26124319ddadd1b6a245f9a1856..f5db319e0b19c22964398d56bc63103d669e1bba
2020-03-22 09:07:01 -0500 to 2020-04-13 08:06:03 -0500
- Update ch01-01-installation.md to require TLS 1.2 (rust-lang/book#2301)
- traits as parameters: Use references instead of moving the values (rust-lang/book#2239)
- Fix a broken link
- Update go docs link (rust-lang/book#2285)
- Add the farsi translation repo link (rust-lang/book#2283)
- Add the missing word to the sentence (ch06-2) (rust-lang/book#2278)
- Fixes hardcoded output (rust-lang/book#2276)
- Use rust-lang/rust linkchecker on CI. (rust-lang/book#2272)
- Add union to the list of keywords (rust-lang/book#2271)
- Clarify the wording (rust-lang/book#2256)
- Improve sentence flow (rust-lang/book#2255)
- Add missing apostrophe (rust-lang/book#2247)
- Update cargo profiles link. (rust-lang/book#2245)
- Add note about chapter 18 in chapter 6 (rust-lang/book#2238)

## rust-by-example

1 commits in a6638463efc7631bc0e8dc67ccd256d4e1b61f1a..c106d1683c3a2b0960f0f0fb01728cbb19807332
2020-04-06 09:39:03 -0500 to 2020-04-09 09:14:39 -0300
- Improve the conversion example (rust-lang/rust-by-example#1329)

## edition-guide

1 commits in 37f9e6848411188a1062ead1bd8ebe4b8aa16899..8204c1d123472cd17f0c1c5c77300ae802eb0271
2020-02-10 14:36:14 +0100 to 2020-04-09 18:55:50 -0700
- Fix forge link. (rust-lang/edition-guide#200)

## embedded-book

1 commits in d22a9c487c78095afc4584f1d9b4ec43529d713c..668fb07b6160b9c468f598e839c1e044db65de30
2020-03-04 09:46:30 +0000 to 2020-04-13 12:38:16 +0000
- Add triagebot configuration  (rust-embedded/book#232)
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.

None yet

3 participants