softdevteam / grmtools

Rust grammar tool libraries and binaries
Other
517 stars 31 forks source link

lrpar:%prec not followed by token name #450

Closed liluyue closed 6 months ago

liluyue commented 6 months ago

I use %nonassoc,always error

截屏2024-05-13 14 48 55
ltratt commented 6 months ago

If you can include the raw text of the input and the error that's produced, that would be helpful.

liluyue commented 6 months ago

If you can include the raw text of the input and the error that's produced, that would be helpful. Priority added to the example:


%start Expr
%left '+' '-'
%left '*' '/'
%nonassoc 'u'
%%
Expr -> Result<u64, ()>:
Expr '+' Expr { Ok($1? + $3?) }
| Expr '-' Expr { Ok($1? - $3?) }
|  Expr '*' Expr { Ok($1? * $3?) }
| Expr '/' Expr { Ok($1? / $3?) }
| Factor { $1 }
| '-' Factor %prec 'u' { Ok(-$2?) }
;

Factor -> Result<u64, ()>: '(' Expr ')' { $2 } | 'INT' { let v = $1.maperr(|| ())?; parse_int($lexer.span_str(v.span())) } ; %% // Any functions here are in scope for all the grammar actions above.

fn parseint(s: &str) -> Result<u64, ()> { match s.parse::() { Ok(val) => Ok(val), Err() => { eprintln!("{} cannot be represented as a u64", s); Err(()) } } }

ltratt commented 6 months ago

Can you also include the error you're seeing please?

liluyue commented 6 months ago
[Error] in src/calc.y
    12|     | '-' Factor %prec 'u' { Ok(-$2?) }
                                %prec not followed by token name

    11|     | Factor { $1 }
              ^^^^^^ Unknown reference to rule 'Factor'
ltratt commented 6 months ago

That probably means that your lexer isn't defining u is a token type? [It could be a bug in cfgrammar of course, but it has a check if self.ast.tokens.contains(&sym) that looks sensible to me, at least at first glance.]

liluyue commented 6 months ago

It can be troublesome to resolve the shift/reduce between rules without %nonassoc“. According to yacc's rules,% nonassoc** should not be bound to a token

ltratt commented 6 months ago

I don't think %nonassoc is relevant to the error you're seeing but @ratmice might have a better idea than me.

ratmice commented 6 months ago

To get the example working through nimbleparse (besides adding a lex file for it), I had to add the following, Edit: It seems like that might not be producing the AST I would expect though.

%token 'u'
%expect-unused 'u'

as well as using the Unmatched trick from https://softdevteam.github.io/grmtools/master/book/errorrecovery.html in the lexer. Which seemed to work.

Edit: I do find it odd that using the Unmatched trick, that %token still seems to be required.

Also unrelated but it is strange that the following error lacks ^^^^ underlining presumably under %prec Presumably this error might have a zero length span on the line?

 [Error] in src/calc.y
    12|     | '-' Factor %prec 'u' { Ok(-$2?) }
                                %prec not followed by token name
ltratt commented 6 months ago

I think there's at laest 1 bug in grmtools here. Investigating.

ltratt commented 6 months ago

OK so with this lexer:

%%
[0-9]+ "INT"
\+ "+"
\- "-"
/ "/"
\* "*"
\( "("
\) ")"
u "u"
[\t \n\r]+ ;

and this grammar:

%start Expr
%left '+' '-'
%left '*' '/'
%nonassoc 'u'
%expect-unused 'u'
%%
Expr -> Result<u64, ()>:
      Expr '+' Expr { Ok($1? + $3?) }
    | Expr '-' Expr { Ok($1? - $3?) }
    |  Expr '*' Expr { Ok($1? * $3?) }
    | Expr '/' Expr { Ok($1? / $3?) }
    | Factor { $1 }
    | '-' Factor %prec 'u' { Ok(-$2?) }
    ;

Factor -> Result<u64, ()>:
      '(' Expr ')' { $2 }
    | 'INT'
      {
          let v = $1.map_err(|_| ())?;
          parse_int($lexer.span_str(v.span()))
      }
    ;
%%
// Any functions here are in scope for all the grammar actions above.

fn parse_int(s: &str) -> Result<u64, ()> {
    match s.parse::<u64>() {
        Ok(val) => Ok(val),
        Err(_) => {
            eprintln!("{} cannot be represented as a u64", s);
            Err(())
        }
    }
}

I was able to replicate the problem. https://github.com/softdevteam/grmtools/pull/453 fixes the problem for me (and with inputs such as "2+3*4" gives a parse tree I'd expect), even if it's not quite the fix I might have hoped for. Will see what @ratmice says in the review though, as my memory is fuzzy on some details.

ltratt commented 6 months ago

I'm going to keep this one open until #453 is reviewed, because I'm not 100% confident of my fix!