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.

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

feat: case and if for branching in rule synthesis. #6

Merged
merged 2 commits into from
Oct 29, 2024

Conversation

winston-h-zhang
Copy link
Contributor

@winston-h-zhang winston-h-zhang commented Oct 29, 2024

This PR:

  • Implements case and if for branching in rule synthesis.

@winston-h-zhang winston-h-zhang changed the title feat: case and if keywords for branching in rule synthesis. feat: case and if keywords for branching in rule synthesis + memory macro Oct 29, 2024
@winston-h-zhang winston-h-zhang changed the title feat: case and if keywords for branching in rule synthesis + memory macro feat: case and if for branching in rule synthesis + memory macro Oct 29, 2024
@winston-h-zhang winston-h-zhang changed the title feat: case and if for branching in rule synthesis + memory macro feat: case and if for branching in rule synthesis. Oct 29, 2024
@winston-h-zhang winston-h-zhang marked this pull request as ready for review October 29, 2024 21:54
Comment on lines 737 to 749
(defun synthesize-if-segment (if-segment curr-rhs end-handle)
(destructuring-bind (head if-form &rest branches)
if-segment
(assert (eql head 'if))
(assert (= (length branches) 2)) ; Must only have then and else branches.
(loop with first = t
for branch-segments in branches
for test-segment = (if first `(when ,if-form) `(when (not ,if-form)))
for curr-rhs-tail = (append curr-rhs (list test-segment))
when first do (setq first nil)
append (synthesize-segments branch-segments (copy-list curr-rhs-tail) end-handle)
into output-rules
finally (return output-rules))))
Copy link
Contributor

@porcuquine porcuquine Oct 29, 2024

Choose a reason for hiding this comment

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

Consider something like this, which is easier for me to follow:

Suggested change
(defun synthesize-if-segment (if-segment curr-rhs end-handle)
(destructuring-bind (head if-form &rest branches)
if-segment
(assert (eql head 'if))
(assert (= (length branches) 2)) ; Must only have then and else branches.
(loop with first = t
for branch-segments in branches
for test-segment = (if first `(when ,if-form) `(when (not ,if-form)))
for curr-rhs-tail = (append curr-rhs (list test-segment))
when first do (setq first nil)
append (synthesize-segments branch-segments (copy-list curr-rhs-tail) end-handle)
into output-rules
finally (return output-rules))))
(defun synthesize-if-segment (if-segment curr-rhs end-handle)
(assert (eql (car if-segment) 'if))
(destructuring-bind (condition if-branch else-branch)
(cdr if-segment)
(flet ((aux (branch condition-form)
(synthesize-segments branch `(,@curr-rhs ,condition-form)) end-handle))
(nconc (aux if-branch `(when ,condition))
(aux else-branch `(when (not ,condition)))))))

when (eql kind :predicate)
collect (make-rule `(,lhs-signal <-- ,@(copy-list curr-rhs-tail))) into output-rules
and collect rhs-handle into curr-rhs-tail
when (or (eql kind :rule-binding) (eql kind :restriction))
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you know you could write the following (for example)?

when (typep kind '(member :rule-binding :restriction))

Copy link
Contributor

@porcuquine porcuquine left a comment

Choose a reason for hiding this comment

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

This looks great, but please strongly consider using my suggested simplification of synthesize-if-segment.

@winston-h-zhang winston-h-zhang merged commit 793d188 into main Oct 29, 2024
1 check passed
@winston-h-zhang winston-h-zhang deleted the lurk-eval branch October 29, 2024 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants