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

Bug: operator precedence around **as** keyword is wrong #279

Closed
ruabmbua opened this issue Dec 17, 2018 · 2 comments · Fixed by #280
Closed

Bug: operator precedence around **as** keyword is wrong #279

ruabmbua opened this issue Dec 17, 2018 · 2 comments · Fixed by #280

Comments

@ruabmbua
Copy link
Contributor

ruabmbua commented Dec 17, 2018

Hi, I just tried the vscode plugin with the language server, and I had a great experience until now! Congrats for the great project!

About the bug:

I opened one of my projects, and immediately in my lib.rs I got a syntax error from the language server. I checked, if the error was valid by compiling the crate, but rustc does not issue any warnings or errors about this specific line of code.

Here is an example I created: playground.

You can try it on the playground, it compiles. If I run the following command with ra_cli, I get some syntax errors:

 > ra_cli parse < bug.rs | grep err
-------------------------------------
parsing: 237.974µs
                err: `expected COMMA`
                err: `expected SEMI`
          err: `expected SEMI`
          err: `expected expression`
  err: `expected an item`
  err: `expected an item`

The error seems to be only about line 7 in the example code. I think something about the + operator might mess up operator precedence, or something else in the parser. My second guess is, that the as u32 syntax is interpreted as something like as {TypeParamBounds}, and the + operator is part of the type / lifetime bounds for some reason. Link to the relevant section in the reference: Type expressions.

Update:
Found same parsing bug in one of the files in this repository: mock_analysis.rs#L76

@ruabmbua
Copy link
Contributor Author

ruabmbua commented Dec 17, 2018

I think I found the bug.

the cast_expr function uses types::_type to get the type, into which the var should be casted.

This is suboptimal, because when type is an ident the function path_type() will be used to parse the type. This however calls path_type_(_, allow_bounds: bool) with true. It should directly call path_type_(_, false), to prevent the parser from eating the plus sign. Bounds are not allowed in the as expression according to language spec.

I will create a PR.

@DJMcNab
Copy link
Contributor

DJMcNab commented Dec 17, 2018

Please see prior discussion in #273. I was waiting for @matklad to respond.

bors bot added a commit that referenced this issue Dec 17, 2018
280: Fixed cast expression parsing in ra_syntax. r=matklad a=ruabmbua

Fixes #279 
Related to #273

The cast expression expected any type via types::type_() function,
but the language spec does only allow TypeNoBounds (types without direct extra bounds
via `+`).

**Example:**

```rust
fn test() {
	6i8 as i32 + 5;
}
```

This fails, because the types::type_() function which should parse the type after the
as keyword is greedy, and takes the plus sign after path types as extra type bounds.

My proposed fix is to replace the not implemented `type_no_plus()` just calls (`type_()`)
function, which is used at several places. The replacement is `type_with_bounds_cond(p: &mut Parser, allow_bounds: bool)`, which passes the condition to relevant sub-parsers.

This function is then called by `type_()` and the new public `type_no_bounds()`.

Co-authored-by: Roland Ruckerbauer <roland.rucky@gmail.com>
@bors bors bot closed this as completed in #280 Dec 17, 2018
bors bot added a commit that referenced this issue Dec 31, 2018
383: Bump failure from 0.1.3 to 0.1.4 r=DJMcNab a=dependabot[bot]

Bumps [failure](https://github.com/rust-lang-nursery/failure) from 0.1.3 to 0.1.4.
<details>
<summary>Changelog</summary>

*Sourced from [failure's changelog](https://github.com/rust-lang-nursery/failure/blob/master/RELEASES.md).*

> # Version 0.1.4
> 
> - Improved error reporting of the derive feature
> - Resolved a potential internal ambiguity when using the backtrace feature
>   that prevented backtrace from improving an upstream API.
> - Changed the bounds on std error compat conversions through the From trait
>   to take Sync and Send into account.
</details>
<details>
<summary>Commits</summary>

- [`70b98e6`](rust-lang-deprecated/failure@70b98e6) 0.1.4
- [`937fb70`](rust-lang-deprecated/failure@937fb70) Add Sync and Send as failure::Error supports them. ([#283](https://github-redirect.dependabot.com/rust-lang-nursery/failure/issues/283))
- [`15b6798`](rust-lang-deprecated/failure@15b6798) Improving procmacro error reporting
- [`22bfd31`](rust-lang-deprecated/failure@22bfd31) support trailing commas in macros ([#273](https://github-redirect.dependabot.com/rust-lang-nursery/failure/issues/273))
- [`26fc6eb`](rust-lang-deprecated/failure@26fc6eb) Future proof debug formatting.  Fixes [#279](https://github-redirect.dependabot.com/rust-lang-nursery/failure/issues/279)
- [`0f89723`](rust-lang-deprecated/failure@0f89723) Reformat for latest rustfmt
- [`8f8f92f`](rust-lang-deprecated/failure@8f8f92f) Update metadata to point to new docs
- See full diff in [compare view](rust-lang-deprecated/failure@0.1.3...0.1.4)
</details>
<br />

[![Dependabot compatibility score](https://api.dependabot.com/badges/compatibility_score?dependency-name=failure&package-manager=cargo&previous-version=0.1.3&new-version=0.1.4)](https://dependabot.com/compatibility-score.html?dependency-name=failure&package-manager=cargo&previous-version=0.1.3&new-version=0.1.4)

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot ignore this [patch|minor|major] version` will close this PR and stop Dependabot creating any more for this minor/major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
- `@dependabot use these labels` will set the current labels as the default for future PRs for this repo and language
- `@dependabot use these reviewers` will set the current reviewers as the default for future PRs for this repo and language
- `@dependabot use these assignees` will set the current assignees as the default for future PRs for this repo and language
- `@dependabot use this milestone` will set the current milestone as the default for future PRs for this repo and language
- `@dependabot badge me` will comment on this PR with code to add a "Dependabot enabled" badge to your readme

Additionally, you can set the following in your Dependabot [dashboard](https://app.dependabot.com):
- Update frequency (including time of day and day of week)
- Automerge options (never/patch/minor, and dev/runtime dependencies)
- Pull request limits (per update run and/or open at any time)
- Out-of-range updates (receive only lockfile updates, if desired)
- Security updates (receive only security updates, if desired)

Finally, you can contact us by mentioning @dependabot.

</details>

Co-authored-by: dependabot[bot] <support@dependabot.com>
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 a pull request may close this issue.

2 participants