Add a test to ensure that we can parse each file #273

DJMcNab · 2018-12-09T19:19:56Z

Note that this has a non-spurious failure in ra_analysis/src/mock_analysis.

Probably fixes #195.

If my understanding is correct, fixes #214 and fixes #225.

matklad · 2018-12-12T18:41:06Z

Awesome!

And yeah, we need to fix our parser first 😅

DJMcNab · 2018-12-13T17:27:49Z

This also passes silently if the project is in a folder called data, which is not ideal. However, travis should catch that anyway, so all is good.

DJMcNab · 2018-12-15T16:49:48Z

I don't know how to fix the parser without a large refactoring of type parsing (to pass allow_bounds around). Note that libsyntax uses that approach, so I think that should be the goal, but I want to check that that is OK before going further.

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>

matklad · 2018-12-17T19:48:42Z

Let's see if this passes now...

bors r+

273: Add a test to ensure that we can parse each file r=matklad a=DJMcNab Note that this has a non-spurious failure in ra_analysis/src/mock_analysis. Probably fixes #195. Co-authored-by: DJMcNab <36049421+djmcnab@users.noreply.github.com>

bors · 2018-12-17T19:52:57Z

Build failed

continuous-integration/travis-ci/push

CAD97 · 2018-12-17T19:58:11Z

---- self_hosting_parsing stdout ----
thread 'self_hosting_parsing' panicked at 'assertion failed: `(left == right)`
  left: `[SyntaxError { kind: ParseError(ParseError("expected a block")), location: Offset(2109) }, SyntaxError { kind: ParseError(ParseError("expected expression")), location: Offset(2109) }, SyntaxError { kind: ParseError(ParseError("expected SEMI")), location: Offset(2112) }, SyntaxError { kind: ParseError(ParseError("expected SEMI")), location: Offset(2117) }]`,
 right: `[]`: There should be no errors in the file DirEntry("/home/travis/build/rust-analyzer/rust-analyzer/crates/ra_syntax/src/validation/byte_string.rs")', crates/ra_syntax/tests/test.rs:71:9

DJMcNab · 2018-12-17T21:43:04Z

That is #214

DJMcNab · 2018-12-17T22:35:19Z

Now we're blocked on #225.

DJMcNab · 2018-12-18T23:01:21Z

Update - I think I've fixed #225, but the code in 4a7271c is really ugly, so an idea of how to refactor this would be helpful. I'll see why CI is failing tomorrow (assuming it is of course).

matklad · 2018-12-19T19:59:24Z

crates/ra_syntax/src/grammar/expressions.rs

@@ -213,7 +213,7 @@ const LHS_FIRST: TokenSet = token_set_union![
    atom::ATOM_EXPR_FIRST,
 ];

-fn lhs(p: &mut Parser, r: Restrictions) -> Option<CompletedMarker> {
+fn lhs(p: &mut Parser, r: Restrictions) -> (Option<CompletedMarker>, Option<BlockLike>) {


I would expect an Option<(CompletionMarker, BlockLike)> here?

I currently only give a BlockLike in case of macro invocation. This code makes that work without breaking anything else.

matklad · 2018-12-19T20:00:09Z

crates/ra_syntax/src/grammar/expressions.rs

            // test stmt_bin_expr_ambiguity
            // fn foo() {
            //     let _ = {1} & 2;
            //     {1} &2;
            // }
-            if r.prefer_stmt && is_block(lhs.kind()) {
+            if r.prefer_stmt && (is_block(lhs.kind()) || blocklike == Some(BlockLike::Block)) {


is_block(lhs.kind()) check should be redundant, given that we return blockfile from lhs() call

matklad · 2018-12-19T20:04:25Z

crates/ra_syntax/src/grammar/expressions.rs

+    p: &mut Parser,
+    r: Restrictions,
+    mut lhs: CompletedMarker,
+    macro_block_like: Option<BlockLike>,


Similarly, we don't need Option here I guess?

Alternatively, I think we can just pass-in allow_calls directly, instead of both r and block_like.

Note that this has a non-spurious failure in ra_analysis/src/mock_analysis

I'm not certain that this is correct, so extra eyes would be good

TODO: Fix this when the block like macro is in expression position E.g. `test(test!{})` currently parses

This approach is correct, but it needs an addition to Restrictions too This reverts commit ad00d0c.

matklad

Just a couple of nits to fix!

matklad · 2018-12-19T20:56:39Z

crates/ra_syntax/src/grammar/expressions.rs

@@ -158,13 +168,13 @@ fn current_op(p: &Parser) -> (u8, Op) {
 // Parses expression with binding power of at least bp.
 fn expr_bp(p: &mut Parser, r: Restrictions, bp: u8) -> BlockLike {
    let mut lhs = match lhs(p, r) {
-        Some(lhs) => {
+        Some((lhs, macro_blocklike)) => {


Let's rename to blocklike, it's not only about macros

Sure, just missed that one

matklad · 2018-12-19T20:58:19Z

crates/ra_syntax/src/grammar/expressions/atom.rs

@@ -362,7 +366,7 @@ fn match_arm(p: &mut Parser) -> BlockLike {
        patterns::pattern(p);
    }
    if p.eat(IF_KW) {
-        expr_no_struct(p);


Are struct literals really allowed here? Let's add a test if this is the case?

struct Test{field: u32} fn main() { match Ok(1u32) as Result<u32, u32> { Ok(0) if Test{field:0}.field ==0 => Test{field:0}, _ => Test{field:1} }; }

Yep - allowed. The only reason they aren't in other places is due to ambiguity, but the => stops that being an issue anyway.

DJMcNab · 2018-12-19T21:06:26Z

The self_hosting_parsing test seems very slow, and I'm not sure if that can be improved at all. Any ideas?

matklad · 2018-12-19T21:12:10Z

test seems very slow,

I think it traversed /target and node_modules :D We could try to start the traversal at project_dir().join("/crates")? Also, it would be a good idea to check that we've parsed at least 30 files: if we change directory layout later, the test could suddenlty "work" by parsing zero files.

DJMcNab · 2018-12-19T21:13:28Z

Yeah, I'll do that. 🤦‍♂️. It did seem very high compared to gen-tests which is near instant although it does a similar traversal

matklad · 2018-12-19T21:21:45Z

bors r+

Thanks!

273: Add a test to ensure that we can parse each file r=matklad a=DJMcNab Note that this has a non-spurious failure in ra_analysis/src/mock_analysis. Probably fixes #195. If my understanding is correct, fixes #214 and fixes #225. Co-authored-by: DJMcNab <36049421+djmcnab@users.noreply.github.com>

bors · 2018-12-19T21:27:45Z

Build succeeded

continuous-integration/travis-ci/push

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>

DJMcNab mentioned this pull request Dec 17, 2018

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

Closed

ruabmbua mentioned this pull request Dec 17, 2018

Fixed cast expression parsing in ra_syntax. #280

Merged

matklad reviewed Dec 19, 2018

View reviewed changes

DJMcNab added 10 commits December 19, 2018 20:12

Add a test to ensure that we can parse each file

0b77eec

Note that this has a non-spurious failure in ra_analysis/src/mock_analysis

Reload package-lock.json

0bd9d87

Fix parsing of inclusive ranges (rust-lang#214)

20bbe01

I'm not certain that this is correct, so extra eyes would be good

Fix at_ts doc comment

012537b

Actually fix at_ts doc comment (committed wrong file :P)

29bf389

Possibly fix rust-lang#225

97e70bf

Fix expression parsing by bailing out upon a macro block being found

7a8560b

TODO: Fix this when the block like macro is in expression position E.g. `test(test!{})` currently parses

Revert to f6f7c5

cd8e33f

Fix handling of structs in match arms

4dce66a

Revert "Revert to f6f7c5"

def0a95

This approach is correct, but it needs an addition to Restrictions too This reverts commit ad00d0c.

DJMcNab force-pushed the test/self_hosting_parsing branch from 38bf8af to def0a95 Compare December 19, 2018 20:12

Move is_block to lower in the call tree

db67741

matklad reviewed Dec 19, 2018

View reviewed changes

Add tests and only traverse in the crates directory

a3b842f

bors bot merged commit a3b842f into rust-lang:master Dec 19, 2018

DJMcNab deleted the test/self_hosting_parsing branch December 19, 2018 21:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a test to ensure that we can parse each file #273

Add a test to ensure that we can parse each file #273

DJMcNab commented Dec 9, 2018 •

edited

Loading

matklad commented Dec 12, 2018

DJMcNab commented Dec 13, 2018

DJMcNab commented Dec 15, 2018

matklad commented Dec 17, 2018

bors bot commented Dec 17, 2018

CAD97 commented Dec 17, 2018

DJMcNab commented Dec 17, 2018

DJMcNab commented Dec 17, 2018

DJMcNab commented Dec 18, 2018

matklad Dec 19, 2018

DJMcNab Dec 19, 2018

matklad Dec 19, 2018

matklad Dec 19, 2018

matklad left a comment

matklad Dec 19, 2018

DJMcNab Dec 19, 2018

matklad Dec 19, 2018

DJMcNab Dec 19, 2018

DJMcNab commented Dec 19, 2018

matklad commented Dec 19, 2018

DJMcNab commented Dec 19, 2018 •

edited

Loading

matklad commented Dec 19, 2018

bors bot commented Dec 19, 2018

Add a test to ensure that we can parse each file #273

Add a test to ensure that we can parse each file #273

Conversation

DJMcNab commented Dec 9, 2018 • edited Loading

matklad commented Dec 12, 2018

DJMcNab commented Dec 13, 2018

DJMcNab commented Dec 15, 2018

matklad commented Dec 17, 2018

bors bot commented Dec 17, 2018

Build failed

CAD97 commented Dec 17, 2018

DJMcNab commented Dec 17, 2018

DJMcNab commented Dec 17, 2018

DJMcNab commented Dec 18, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matklad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DJMcNab commented Dec 19, 2018

matklad commented Dec 19, 2018

DJMcNab commented Dec 19, 2018 • edited Loading

matklad commented Dec 19, 2018

bors bot commented Dec 19, 2018

Build succeeded

DJMcNab commented Dec 9, 2018 •

edited

Loading

DJMcNab commented Dec 19, 2018 •

edited

Loading