-
Notifications
You must be signed in to change notification settings - Fork 42
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
crucible{,-llvm}-syntax: Evaluating programs with syntax extensions #1121
Conversation
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.
Adds a
exec-crucible-llvm
executable based oncrucibler
- has lots of shared code withcrucibler
, but is also substantially different, so not sure if more sharing is feasible
I might be missing something, but the code in exec-crucible-llvm
doesn't seem that different to me. At a glance, the big differences that stand out are:
exec-crucible-llvm
uses a differentExtensionImpl
exec-crucible-llvm
uses a differentsetupHook
exec-curcible-llvm
uses a differentParserHooks
Am I missing something? If that's it, I could definitely imagine factoring out a large chunk of the functionality that is shared in common between crucibler
and exec-crucible-llvm
, including the three things above as arguments to the function.
(As a side note, I've never understand why it's named crucibler
in the first place. I was going to suggest naming the other tool something like crucibler-llvm
, but I'm not sure if it's a convention worth keeping.)
No, you're right - I guess the real question is: Where should that shared code go, given that it mostly consists of |
I was thinking of moving most of this functionality to Command -> ExtensionImpl -> ParserHooks -> SimulatorHooks -> IO () Where |
Done. I don't love this solution, as library clients still need to build code they don't need ( In the future, it might be nice to create a |
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 broadly aligned with the goal of factoring out the CLI-related bits, although we certainly don't have to hold up this PR on that.
Everything else looks great!
(cherry picked from commit 8c302b7)
Fixes #1115.
SimulateProgramHooks
to bind anext
variable, so hooks can be extension-specificexec-crucible-llvm
executable based oncrucibler
- has lots of shared code withcrucibler
, but is also substantially different, so not sure if more sharing is feasible