dimitriv / Ziria

A domain-specific-language and compiler for low-level bitstream processing.
92 stars 18 forks source link

Error message when writing (incorrectly) "var x : int = y". #112

Open Dom-Skinner opened 9 years ago

Dom-Skinner commented 9 years ago

The simple code:

fun comp f() {
  var y : int = 0;
  x <-take;
  emit (x+1)
}

let comp main = read >>> f()  >>> write

fails to compile with the error message

EXTRAOPTS='' ../../scripts/preprocesscompile-vs.sh test.zir test.out
test.zir
test.zir:3:15:
    expected 'in' but got `='
Makefile:45: recipe for target 'test.out' failed
make: *** [test.out] Error 1

as var y : int = 0; should be var y : int := 0;

Shouldn't the error message have been something like: expected ':= or ;' but got '=' rather than expected 'in' but got '='

ghost commented 9 years ago

True. That error message is not very helpful. The short answer to why you're getting that error is that the compiler thinks that you wanted to write something like var y: int in emit (1);, which is a valid statement.

And, the long answer......

To see exactly why this error is occurring, lets take a look at similar code which does compile and run fine:

fun comp f() {
  x <- take;
  var y: int := 1 in emit (y);
  emit (x+1);
}

let comp main = read >>> f() >>> write

This could have an infile of 0 and an outfile of 1,1.

The key statement here is var y: int := 1, which is a valid decl defined in https://github.com/dimitriv/Ziria/blob/4756d7f27fab5e1ca46b4f6995e40762b8ae2841/src/Language/Ziria/Parser/Parser.y#L608

Now, just obfuscate the code by changing var y: int := 1 in emit (y) to var y: int in emit (1). Since the initialization is optional, var y: int is still a valid decl.

comp_let_decl succeeds if it sees a decl, as defined in https://github.com/dimitriv/Ziria/blob/4756d7f27fab5e1ca46b4f6995e40762b8ae2841/src/Language/Ziria/Parser/Parser.y#L780

The compiler has successfully parsed var y : int as a comp_let_decl, and expects an in after that - but fails with the error in https://github.com/dimitriv/Ziria/blob/4756d7f27fab5e1ca46b4f6995e40762b8ae2841/src/Language/Ziria/Parser/Parser.y#L918.

The token that's parsed while getting the error is Tdef or '='. Hence, the compiler is complaning that comp_let_decl must be followed by in, and not =.

dimitriv commented 9 years ago

That is correct response. That said, the parser at the moment is quite problematic (lots of s/r conflicts and a few (relatively innocuous) r/r/ conflicts) and I've not had the time to properly refactor the syntax, though I've experimented also with a Caml-like frontend. But we should not postpone cleaning up the frontend any longer! @Dom-Skinner, @siddhanathan interested in helping out?

ghost commented 9 years ago

@dimitriv Yeah, definitely.

cc @mainland

mainland commented 9 years ago

I actually have a conflict-free version of the existing grammar, but it relies on a non-yet-complete rewrite of the typechecker.

@dimitriv, when you say "refactor" the syntax, I assume you mean rewrite?

dimitriv commented 9 years ago

Wow, that's awesome @mainland! With no drastic modifications (or none?) to the GExpr datatype? NB: SrcExpr (and similarly SrcComp) is currently defined as a synonym to GExpr SrcTy or something but we should feel free to do as much surgery as we like there since it is completely independent from the rest of the compiler, and so is type checking SrcExpr -> Expr. I guess this is what you mean when you say you are doing a rewrite of the type checker?

If I remember correctly from my earlier attempts, most of the conflicts came from: (a) the computation or expression arguments to computation functions (b) the list notation { .... } which conflicts too often with sequences of commands/statements.

Also, if you are doing this refactoring, it would be good to check that we parse expressions and computations "the same way", meaning same kind of priorities for conditionals, let bound expressions, etc, i.e. make things parse as uniform as possible.

I can have a look at your .y file and leave comments in there and I am also up for testing and merging when you are done! This is very cool!

dimitriv commented 9 years ago

Btw @mainland, if you are doing this anyway, maybe you can also make sure we parse only lvalues (dereference expressions, I think they are called) in the LHS of assignments, and get rid of the EArrWrite constructor too from GExpr everywhere. The compiler is already prepared for this, thanks to the recent rewrites. I would still keep the smart constructor eArrWrite in AstLabelled and AstUnlabelled as it is convenient to have around. But no need to have a special AST node for this.