-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DSL for module semantics #32
Changes from 1 commit
152b2d0
a1c51f9
5380d76
2b82dcc
6b6bd32
835bf0d
7d757da
7aaff60
cecacb5
2a158db
066f8c3
6c7d379
97d3fe6
68c6067
b8681db
9b3e6e3
030a3a8
d967107
8f6c590
0f48772
35be3dd
edda75e
ff216fb
21e0a94
236de04
1560490
e7a5252
6bc75a6
506306e
eb57512
5aeb393
a453c69
88fe94b
42b276f
65e6777
edb1fbf
8bb5fe0
6872abe
616eded
d764e97
5c9aed2
ba5b056
31213dd
474c65f
384016d
fd8ec1f
97e95c3
0e45bef
199efd1
cb45bce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -37,9 +37,7 @@ let free_defid id = {empty with defid = Set.singleton id.it} | |||||
let rec free_iter iter = | ||||||
match iter with | ||||||
| Opt | List | List1 -> empty | ||||||
| ListN (e, None) -> free_exp e | ||||||
| ListN (e, Some (id)) -> | ||||||
union (free_exp e) (free_varid id) | ||||||
| ListN (e, _) -> free_exp e | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesn't this need to be:
Suggested change
|
||||||
|
||||||
|
||||||
(* Types *) | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -68,8 +68,7 @@ let iter_nl_list f xs = List.iter (function Nl -> () | Elem x -> f x) xs | |||||
let rec check_iter env ctx iter = | ||||||
match iter with | ||||||
| Opt | List | List1 -> () | ||||||
| ListN (e, opt) -> | ||||||
Option.iter (fun id -> check_id env ctx id) opt; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't we still need this check? |
||||||
| ListN (e, _) -> | ||||||
check_exp env ctx e | ||||||
|
||||||
and check_exp env ctx e = | ||||||
|
@@ -163,13 +162,9 @@ let union = Env.union (fun _ ctx1 ctx2 -> assert (ctx1 = ctx2); Some ctx1) | |||||
let rec annot_iter env iter : Il.Ast.iter * occur = | ||||||
match iter with | ||||||
| Opt | List | List1 -> iter, Env.empty | ||||||
| ListN (e, None) -> | ||||||
| ListN (e, opt) -> | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit:
Suggested change
Also, I believe this needs to check that the variable has the right type. |
||||||
let e', occur = annot_exp env e in | ||||||
ListN (e', None), occur | ||||||
| ListN (e, Some id) -> | ||||||
let e', occur1 = annot_exp env e in | ||||||
let occur2 = Env.singleton id.it (Env.find id.it env) in | ||||||
ListN (e', Some id), union occur1 occur2 | ||||||
ListN (e', opt), occur | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
and annot_exp env e : Il.Ast.exp * occur = | ||||||
let it, occur = | ||||||
|
@@ -290,6 +285,12 @@ and annot_iterexp env occur1 (iter, ids) at : Il.Ast.iterexp * occur = | |||||
) occur1 | ||||||
in | ||||||
let ids' = List.map (fun (x, _) -> x $ at) (Env.bindings occur1') in | ||||||
|
||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
(match iter' with | ||||||
| ListN (_, Some id) -> | ||||||
assert (List.length ids' = 1 && id.it = (List.hd ids').it) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure I understand this assertion. Why can't there be any other ids in that case? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is because I would like to block the use of "iter with index" like this:
If you think we should allow semantics like this, I will remove the assertion. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think I follow, what problem do you see with that example? In Shouldn't using them for indexing be one of the primary use cases for iter vars? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I apologize for not providing sufficient explanation earlier. |
||||||
| _ -> ()); | ||||||
|
||||||
(iter', ids'), union occur1' occur2 | ||||||
|
||||||
|
||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -55,9 +55,7 @@ let free_defid id = {empty with defid = Set.singleton id.it} | |||||
let rec free_iter iter = | ||||||
match iter with | ||||||
| Opt | List | List1 -> empty | ||||||
| ListN (e, None) -> free_exp e | ||||||
| ListN (e, Some id) -> | ||||||
union (free_exp e) (free_varid id) | ||||||
| ListN (e, _) -> free_exp e | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't his be
Suggested change
|
||||||
|
||||||
|
||||||
(* Types *) | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I refined the module semantics. To reflect the aforementioned comments, I extended the DSL (you can see the detailed explanation below). This change requires updating the EL parser, IL validation and other components, and we're planning to update them. Before that, I'd like to know whether the updated DSL looks good enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm implementing this in dsl branch. It might be helpful to see the implementation to understand the extended syntax.