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

Operator expressions grammar #141

Merged
merged 3 commits into from
Mar 10, 2018
Merged

Conversation

brauliobz
Copy link
Contributor

Part of #84.

Copy link
Contributor

@Havvy Havvy left a comment

Choose a reason for hiding this comment

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

Other than the missing expression at the top, looks great.

@@ -1,5 +1,18 @@
# Operator expressions

> **<sup>Syntax</sup>**
> _OperatorExpression_ :
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing GroupedExpression.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Grouped expression will be located directly on the Expression production. I think it would be out of place if put together with the OperatorExpressions -- even though it is in the same section. I put it in the operator expression section mainly because I think it is too small to be on a separate section by itself.

What do you think?

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 grammar should be associated with its prose. GroupedExpression is on this page, so it should show up here. If it feels out of place in the grammar, it should feel out of place on the prose and moved to its own page, albeit the shortness is concerning. Although some examples could help with that.

@@ -36,6 +53,11 @@ assert_eq!(y, 20);

## Borrow operators

> **<sup>Syntax</sup>**
> _BorrowExpression_ :
> &nbsp;&nbsp; &nbsp;&nbsp; (`&`|`&&`) [_Expression_]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the && variant here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because the lexer will give us && when it finds two contiguous ampersands, since && is a valid symbol. The same problem occurs with << and >> in generics/paths, and || in closures. Example:

let & & mut a = & & mut 10;
let &&  mut a = &&  mut 10;

I'm currently very busy, but I'll put some explanation in the next days, since it isn't really obvious why && is accepted.

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

@@ -60,8 +82,25 @@ let mut array = [-2, 3, 9];
}
```

For ease of typing, consecutive borrow expressions can be expressed by the
Copy link
Contributor

Choose a reason for hiding this comment

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

Ease of typing compared to what? To most people reading the reference, it would seem obvious that && is allowed and can be chained just like e.g. --1 works (though is useless). Though I'm guessing that the x && y operator giving an && token meaning is to blame here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, indeed the compound && being a single token is to blame here. I've changed the text to only refer to this fact.

@mrhota
Copy link
Contributor

mrhota commented Jan 21, 2018

@brauliobz ping! what's the status?

@brauliobz
Copy link
Contributor Author

@mrhota Hey, I'm back! I'm rebasing my branches and after that I'll deal with this PR.

Copy link
Contributor

@Havvy Havvy left a comment

Choose a reason for hiding this comment

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

I still think GroupedExpression should still be on this page, but not going to block the rest of this on that.

@Havvy Havvy merged commit e490727 into rust-lang:master Mar 10, 2018
@Havvy
Copy link
Contributor

Havvy commented Mar 10, 2018

💟 Thanks!

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