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

Introduce HuddleM #40

Merged
merged 3 commits into from
Nov 20, 2024
Merged

Introduce HuddleM #40

merged 3 commits into from
Nov 20, 2024

Conversation

nc6
Copy link
Collaborator

@nc6 nc6 commented Nov 13, 2024

This is an attempt at addressing #35.

We provide a monad for specifying Huddle definitions, which will collect them in definition order. This allows a fairly convenient way to put together a specification, at the cost of it being trickier to re-use definitions elsewhere, since they need to be returned from the monad.

This is an attempt at addressing #35.

We provide a monad for specifying Huddle definitions, which will collect
them in definition order. This allows a fairly convenient way to put
together a specification, at the cost of it being trickier to re-use
definitions elsewhere, since they need to be returned from the monad.
@nc6 nc6 requested a review from Soupstraw November 13, 2024 13:45
example/Monad.hs Outdated
Comment on lines 23 to 24
setRootRules [transaction]
pure ()
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if it would make sense to return all of the roots instead of setting them via setRootRules

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The reason I didn't do this is that it might be needed to return specific things to allow them to be re-used.

Copy link

@aniketd aniketd left a comment

Choose a reason for hiding this comment

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

LGTM! 🙌

By using an ordered map we allow a lot of information for sorting the
entries arising from Huddle, and in particular for _merging_ such
entries, where we want to override some definitions.
We add some tools to allow extending Huddle specifications using the
semigroup instance established:

- We allow one to reference a HuddleItem (which is a rule, a group, or
a generic def) as a Type0.
- We then allow one to reference (by name) a HuddleItem from an existing
spec.

These two combined allow us to (somewhat) "extend" a specification in a
nice manner - we can reference items from the previous spec by their
name, and then selectively override things using the semigroup instance
(but respecting the original ordering).

There are two "disappointments" involved in this, however:
- Since the items from a previous spec are referenced by name, we lose
the type-safety provided by the Haskell compiler. It's quite possible to
refer to an item that doesn't exist, and you won't find out until
runtime.
- The whole thing falls apart for generic rules. When calling a generic
rule in the usual way, we do two things:
  - Apply the actual argument and turn it into a 'GRuleCall' which we
return to the call site.
  - Discard the argument, create an appropriate number of fresh names
and insert this into Huddle as a 'GRuleDef'. This crucially ignores any
actual arguments, which is why we can pass an error as a fake argument
in the Includable instance for HuddleM and have it all work.
  Unfortunately, what we cannot do is go from the 'GRuleDef' and extract
from it the fact that, on the Haskell side, this is a function (with an
unknown number of parameters). Which is very annoying.

I have some ideas about resolving this second issue in a
slightly-less-horrible way; they will follow in a subsequent commit.
@nc6 nc6 merged commit b09cd50 into master Nov 20, 2024
4 checks passed
@nc6 nc6 deleted the nc/huddleM branch November 20, 2024 14:13
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.

3 participants