cucapra / packet-scheduling

MIT License
3 stars 0 forks source link

DSL: Simplify AST #29

Closed anshumanmohan closed 4 months ago

anshumanmohan commented 4 months ago

27 is about to be closed, but I have a little more simplification to offer in the AST. I'd love feedback on this from all. This PR makes the changes I propose below, both in the AST and in the parser. It works correctly on all our sample programs.

However, please treat the text below and the PR itself as a proof of concept. It's not trying to be "set in stone" or prescriptive in the slightest; please do chime in with your thoughts.

At present we have:

type clss = string
type var = string

type declare =
| DeclareClasses of clss list

type policy =
| Class of clss
| Fifo of policy list
| Fair of policy list
| Strict of policy list
| Var of var

type assignment =
| Assn of var * policy

type statement =
| Declare of declare
| Assignment of assignment
| Seq of statement * statement
| Return of policy

type program =
| Prog of declare * (statement list)

Since we have a statement list, we don't really need a Seq constructor in statement. We just need to actually parse a list of statements as a list, not a Seq. If we take on that responsibility at the parser level, our AST gets to be a little simpler:

type statement =
| Declare of declare
| Assignment of assignment
(* | Seq of statement * statement *)
| Return of policy

type program =
| Prog of declare * (statement list)

We should probably also drop the | Declare of declare constructor from statement, so that the user gets exactly one chance to declare their classes up top. That gets us to:

type declare =
| DeclareClasses of clss list

...

type statement =
(* | Declare of declare *)
| Assignment of assignment
(* | Seq of statement * statement *)
| Return of policy

type program =
| Prog of declare * (statement list)

In the same spirit you may want to drag return out into its own type, such that the user can only have one return at the end of the program.

type declare =
| DeclareClasses of clss list

type return = 
| Return of policy

...

type statement =
(* | Declare of declare *)
| Assignment of assignment
(* | Seq of statement * statement *)
(* | Return of policy *)

type program =
| Prog of declare * (statement list) * return

And now you observe that statement is nothing more than a silly wrapper for Assignment, so why not just change statement list into assignment list and be done with it?

type clss = string
type var = string

type declare =
| DeclareClasses of clss list

type policy =
| Class of clss
| Fifo of policy list
| Fair of policy list
| Strict of policy list
| Var of var

type return = 
| Return of policy

type assignment =
| Assn of var * policy

type program =
| Prog of declare * (assignment list) * return
csziklai commented 4 months ago

Sounds good to me! Should make my job with the interpreter easier.

anshumanmohan commented 4 months ago

Super, thank you both! Gonna go ahead and merge then. And yes, @csziklai, I think this will force the user to write "nice" programs more often, so that should be helpful to you! Also, we now actually parse and generate a prog of type Ast.program, and I suspect that will save us from some grief later on 😅

rachitnigam commented 4 months ago

One drive-by thought: The declaration/statement distinction is something Dahlia also has but one thing it suffers from is bad error messages. If declarations and statements are interleaved, the parser just says "parser error". I would recommend adding an error rule in the parser that successfully parses a declaration after a statement and then immediately throws an error.