-
Notifications
You must be signed in to change notification settings - Fork 21
Clarify Fulminate API #469
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
base: main
Are you sure you want to change the base?
Clarify Fulminate API #469
Conversation
3d98971 to
d1ba26d
Compare
rbanerjee20
left a comment
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.
Mostly looks good to me, bar the comments/questions I've left. Thanks for doing this - the single utils.h file had become quite unruly
| #include "rts_deps.h" | ||
| #include "stack.h" | ||
|
|
||
| #define fallthrough __attribute__((__fallthrough__)) |
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.
Why do we still need this file at all?
| #define alignof _Alignof | ||
|
|
||
| #define bool _Bool | ||
| #define true 1 |
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.
We want to clean up this file too (@pqwy and I discussed it at length a few months ago) but I'm happy to leave that to be in another PR
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.
You should be able to edit this PR and make whatever changes you want
|
|
||
| /* Signed bitvectors */ | ||
|
|
||
| typedef struct cn_bits_i8 { |
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'd probably prefer there to be a different header for the type definitions (e.g. fulminate/ghost_types.h), which gets included in this header, and then the rest can make up eval.h
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.
(if that works)
| @@ -0,0 +1,426 @@ | |||
| #ifndef CN_EVAL_H | |||
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.
and minor question, but why is this under cn-executable and not fulminate?
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've been thinking of Fulminate as the instrumentation, while the stuff in eval.h is the ability to evaluate pure CN terms.
From the testing's perspective, the instrumentation is not necessary, but rather a stronger oracle, whereas evaluating pure CN terms is necessary. In the future, testing will support being run without the instrumentation, but will still require the stuff in cn-executable.
For the counterexample program synthesis prototype, it needs to be able to evaluate CN terms, but instrumentation is unrelated.
| void cn_pop_msg_info(void); | ||
|
|
||
| void cn_assert(cn_bool* cn_b, enum spec_mode spec_mode); | ||
|
|
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.
shouldn't this morally also include the CN_LOAD, CN_STORE etc macros?
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.
Callbacks for use by testing
fulminate/api.h
Why would we want to expose those to testing frameworks? Excluding them is the main motivator for testing.
93a24c5 to
2e649e1
Compare
Closes #461
Addresses the four uses:
fulminate/internals.hcn-executable/eval.hfulminate/user.hfulminate/api.hNote: This doesn't really clean up the
*.cfiles