Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Construct sexp #1328

Draft
wants to merge 5 commits into
base: llama-lsp-lookahead
Choose a base branch
from
Draft

Construct sexp #1328

wants to merge 5 commits into from

Conversation

gcrois
Copy link

@gcrois gcrois commented Jul 17, 2024 •
edited
Loading

This branch seeks to add functionality to generate an sexp from valid Hazel code in the file src/haz3lweb/SexpConversion.re.

  • Triv
  • Bool
  • Int
  • Float
  • String
  • ListLit
  • Invalid
  • EmptyHole
  • MultiHole
  • Constructor
  • Fun
  • Tuple
  • Var
  • Let
  • TyAlias
  • Ap
  • If
  • Seq
  • Test
  • Parens
  • Cons
  • ListConcat
  • UnOp
  • BinOp
  • Match

@gcrois gcrois added assistant Part of the Hazel Assistant project in-development for PRs that remain in development labels Jul 17, 2024
@disconcision disconcision changed the base branch from dev to llama-lsp-lookahead July 17, 2024 01:38
@gcrois
Copy link
Author

gcrois commented Jul 19, 2024

Remaining: finish sexp_of_uexp, build test cases

@cyrus- cyrus- requested a review from disconcision July 24, 2024 16:15
cyrus-
cyrus- requested changes Jul 25, 2024
View reviewed changes
Copy link
Member

@cyrus- cyrus- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reviewed the go functions

src/haz3lweb/SexpConversion.re
switch (term.term) {
| Invalid(string) => Atom(string)
| EmptyHole => Atom("?")
| MultiHole(_) => Atom("Not implemented")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be "(multihole ...)" for completion.

src/haz3lweb/SexpConversion.re
| Tuple(list) => List([Sexp.Atom("tuple")] @ List.map(go, list))
| Var(t) => Atom(t)
| Let(pat, exp1, exp2) =>
List([Atom("let"), List([go(exp1), go(exp2)]), goUPat(pat)])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the pattern should go first, then exp1 and then next to it (not nested) exp2

src/haz3lweb/SexpConversion.re
| Seq(exp1, exp2) => List([Atom("seq"), go(exp1), go(exp2)])
| Test(t) => List([Atom("test"), go(t)])
| Cons(head, tail) => List([Atom("cons"), go(head), go(tail)])
| ListConcat(list1, list2) => List([Atom("@"), go(list1), go(list2)])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe concat to be consistent with us using cons rather than ::

src/haz3lweb/SexpConversion.re
and goUPat: Term.UPat.t => Sexp.t =
pat => {
switch (pat.term) {
| EmptyHole => Atom("?")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@disconcision do we want to use ? or ?? for empty holes?

src/haz3lweb/SexpConversion.re
switch (pat.term) {
| EmptyHole => Atom("?")
| Triv => List([])
| MultiHole(_) => Atom("Not implemented")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see above

src/haz3lweb/SexpConversion.re
typ => {
switch (typ.term) {
| EmptyHole => Atom("EmptyHole")
| MultiHole(_) => Atom("MultiHole")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see above

src/haz3lweb/SexpConversion.re
| Var(string) => Atom(string)
| Constructor(string) => Atom(string)
| Invalid(string) => Atom(string)
| Sum(_list) => Atom("list") //List([Sexp.Atom("sum")] @ List.map(goTyp, list))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to add support for sum types (algebraic data types)

src/haz3lweb/SexpConversion.re
| Constructor(string) => Atom(string)
| Invalid(string) => Atom(string)
| Sum(_list) => Atom("list") //List([Sexp.Atom("sum")] @ List.map(goTyp, list))
| Arrow(_, _) => Atom("Arrow")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to add support for arrow types

src/haz3lweb/SexpConversion.re
| Invalid(string) => Atom(string)
| Sum(_list) => Atom("list") //List([Sexp.Atom("sum")] @ List.map(goTyp, list))
| Arrow(_, _) => Atom("Arrow")
| Parens(_) => Atom("Parens")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just recurse down as in the other parens cases

src/haz3lweb/SexpConversion.re
| Sum(_list) => Atom("list") //List([Sexp.Atom("sum")] @ List.map(goTyp, list))
| Arrow(_, _) => Atom("Arrow")
| Parens(_) => Atom("Parens")
| Ap(_, _) => Atom("Ap")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

treat the same as other ap cases

@cyrus- cyrus- marked this pull request as draft August 4, 2024 03:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cyrus- cyrus- cyrus- requested changes

@disconcision disconcision Awaiting requested review from disconcision

Assignees
No one assigned
Labels
assistant Part of the Hazel Assistant project in-development for PRs that remain in development
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gcrois @cyrus-