Stream: t-compiler/wg-rls-2.0

Topic: Compile times


Joshua Nelson (Jan 12 2020 at 15:05, on Zulip):

Hello all, I want to work on https://github.com/rust-analyzer/rust-analyzer/issues/2801, but every time I make a change it takes 5-10 minutes to compile when I run cargo test. Is there way to run tests for just the parser without having to compile any of the crates depending on it?

matklad (Jan 12 2020 at 15:06, on Zulip):

cargo test -p ra_syntax

Joshua Nelson (Jan 12 2020 at 15:07, on Zulip):

That doesn't seem to be running all the tests I want ...

(bash@lubuntu-thinkpad) ~/.../rust-analyzer/crates ▶️ cargo test -p ra_parser
    Finished test [unoptimized] target(s) in 0.30s
     Running /home/joshua/Documents/Programming/rust/rust-analyzer/target/debug/deps/ra_parser-37542a5f1551f7a0

running 1 test
test token_set::token_set_works_for_tokens ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
Joshua Nelson (Jan 12 2020 at 15:07, on Zulip):

oh you said ra_syntax, whoops. Let me try that.

Joshua Nelson (Jan 12 2020 at 15:09, on Zulip):

ok great! cargo xtask codegen && cargo test -p ra_syntax did exactly what I wanted :)

Edwin Cheng (Jan 12 2020 at 15:22, on Zulip):

@Joshua Nelson Just a little reminder: The Attr node should be placed inside the element node. :

https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Fwg-rls-2.2E0/topic/Attributes.20in.20elements.20of.20array.20and.20args

Joshua Nelson (Jan 12 2020 at 15:25, on Zulip):

Joshua Nelson Just a little reminder: The Attr node should be placed inside the element node. :

https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Fwg-rls-2.2E0/topic/Attributes.20in.20elements.20of.20array.20and.20args

I'm not sure what you mean. This is what my current code looks like:

 fn arg_list(p: &mut Parser) {
     assert!(p.at(T!['(']));
     let m = p.start();
     p.bump(T!['(']);
     while !p.at(T![')']) && !p.at(EOF) {
+        attributes::outer_attributes(p);
         if !p.at_ts(EXPR_FIRST) {
             p.error("expected expression");
             break;

Is there something else I need to change (other than adding tests)?

Edwin Cheng (Jan 12 2020 at 15:33, on Zulip):

The output node should be something like this:

ARG_LIST@[63; 224)
  L_PAREN@[63; 64) "("
  WHITESPACE@[64; 73) "\n        "
  LITERAL@[73; 101)
    ATTR@[157; 176)     <------------------------------
      POUND@[157; 158) "#"
      L_BRACK@[158; 159) "["
      PATH@[159; 164)
        PATH_SEGMENT@[159; 164)
          NAME_REF@[159; 164)
            IDENT@[159; 164) "allow"
      TOKEN_TREE@[164; 175)
        L_PAREN@[164; 165) "("
        IDENT@[165; 174) "dead_code"
        R_PAREN@[174; 175) ")"
      R_BRACK@[175; 176) "]"
    WHITESPACE@[176; 185) "\n        "
    LITERAL@[185; 217)
      FLOAT_NUMBER@[185; 217) "1.797_693_134_862_315 ..."
    COMMA@[217; 218) ","
    WHITESPACE@[218; 223) "\n    "
    R_PAREN@[223; 224) ")"
    STRING@[73; 101) "\"1.797693134862315708 ..."
  COMMA@[101; 102) ","
  WHITESPACE@[102; 111) "\n        "
SEMI@[224; 225) ";"
Joshua Nelson (Jan 12 2020 at 15:41, on Zulip):

So the attr should go _inside_ the literal, not above it? How would I do that?

Currently I have it above: https://github.com/rust-analyzer/rust-analyzer/pull/2813/files#diff-e1a74cafc36f91710ff085ff4aae3fffR69

Joshua Nelson (Jan 12 2020 at 15:59, on Zulip):

I added a new ARG node, since I don't know ahead of time what kind of expression the argument is and there doesn't seem to be a catch-all Expr type. Let me know if I should change that.

Edwin Cheng (Jan 12 2020 at 16:24, on Zulip):

I was finding how to do it and I can't think of a simple approach too. Maybe we could add a helper function in CompletedMarker for that insertion. ? @matklad

Joshua Nelson (Jan 12 2020 at 16:31, on Zulip):

What would the helper function do? Add the node both below and above, like you had?

LITERAL
  ATTR
  LITERAL

That seems a little inelegant to me ...

Edwin Cheng (Jan 12 2020 at 16:32, on Zulip):
LITERA
    ATTR
    FLOAT_NUMBER
Edwin Cheng (Jan 12 2020 at 16:44, on Zulip):

Another option is passing down a flag to indicate we should expect an attribute node inside expr, but it is quite complicated.

Joshua Nelson (Jan 12 2020 at 17:05, on Zulip):

It looks like rustc plans to eventually add attributes on expressions: https://github.com/rust-lang/rust/issues/15701
Would it be easier to parse the attribute in expr() and check later that it's in a valid position?

Edwin Cheng (Jan 12 2020 at 17:15, on Zulip):

I think attr on stmt is already handled.

https://github.com/rust-analyzer/rust-analyzer/blob/d0b52e5d84b8c371b74d4d5d43f45be91f103d12/crates/ra_parser/src/grammar/expressions.rs#L61

Joshua Nelson (Jan 12 2020 at 17:28, on Zulip):

it looks like libsyntax allows it on any expression, not just statements: https://github.com/rust-lang/rust/blob/441519536c8bd138e8c651743249acd6814747a1/src/libsyntax/parse/parser.rs#L2733

Joshua Nelson (Jan 12 2020 at 17:28, on Zulip):

note how there's no special handling for function arguments

Joshua Nelson (Jan 12 2020 at 17:31, on Zulip):

here's the actually attribute handling: https://github.com/rust-lang/rust/blob/441519536c8bd138e8c651743249acd6814747a1/src/libsyntax/parse/parser.rs#L2841

Last update: Jan 21 2020 at 08:25UTC