search

microsoft / knossos-ksc

Compiler with automatic differentiation
Other
45 stars 10 forks source link

Assert that Let vars are not decl #840

Closed acl33 closed 3 years ago

acl33 commented 3 years ago

This continues in the theme of #827. Let vars never have decl set - the current codebase is 100% consistent on that front. So, this PR adds an assertion to enforce this (i.e. make sure we stay consistent in the future!). The assert also provides some documentation.

There is a counterargument that we do not generally check vars elsewhere aren't decl, so why should we do so here? But I think we should so that we make clear that decl does not mean "binding occurrence".

A further extension might be to assert that decl is set for Def args, not done in this PR.

awf commented 3 years ago

Just as it's been on my mind: I've been wondering about explicitly removing the "decl" flag, and just making Defs and Lambdas explicitly mention a list of (Var, Type) pairs. This makes the AST much more accurately match the syntax, and would probably result in a lot of cleanup. Thoughts?

acl33 commented 3 years ago

Just as it's been on my mind: I've been wondering about explicitly removing the "decl" flag, and just making Defs and Lambdas explicitly mention a list of (Var, Type) pairs. This makes the AST much more accurately match the syntax, and would probably result in a lot of cleanup. Thoughts?

I like that following your suggestion, a Var no longer needs to know that it's part of a Def/Lam.

But, it seems odd that Var can have a type itself, and have a type in the Def/Lam. Would type_propagation update the type in the Var, from the type in the containing Lam/Def ? (I suppose this is a bit like how typepropagation would have to set the `typefield of the Lam by reading thearg_type` ?)

However, that may be a poor line of reasoning, as it seems to lead to Lam containing (arg: str, arg_type: Type, body: Expr), and I'm not sure whether that's just odd, or indeed accurate.

Or, since the decl only really seems to affect equality (decl != non-decl) and printing, can we remove the decl from Var, have Lam use the type_ in its arg, and then just use a different printing routine?

dcrc2 commented 3 years ago

Or, since the decl only really seems to affect equality (decl != non-decl) and printing, can we remove the decl from Var, have Lam use the type_ in its arg, and then just use a different printing routine?

That sounds good to me; I think it's the closest match for how our other AST implementations (Haskell and C++) handle this.

acl33 commented 3 years ago

So do we want to drop this in favour of removing decl altogether? @awf ?

awf commented 3 years ago

Yes, let's drop decl. Thanks!