-
Notifications
You must be signed in to change notification settings - Fork 45
llvm: Remove LLVM declares from LLVM overrides
#1705
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: master
Are you sure you want to change the base?
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
89653bd to
19c0c38
Compare
19c0c38 to
db9b71f
Compare
fcdb4be to
6c1093d
Compare
|
Status: I'm adapting GREASE to these changes to make sure they work in practice. [EDIT]: See GaloisInc/grease#525 |
RyanGlScott
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.
Do you envision these changes being breaking to most downstream consumers? (I mainly ask this in a SAW context, since we are in the middle of planning new releases soon.)
After this change, it would be feasible to completely remove the quasiquoters. I haven't done that in this PR, but would be happy to do so if requested.
That is an interesting idea, although I do like the fact that the quasiquoter syntax is much more terse than what the equivalent Haskell code would be. As such, I'm a bit on the fence about this idea. Perhaps we could discuss this in a separate issue?
crux-llvm/test-data/golden/golden/double_free.pre-clang19.z3.good
Outdated
Show resolved
Hide resolved
Only the ones (like crucible-llvm-syntax or GREASE) that use the S-expression syntax. I would expect that SAW, like Crux-LLVM, will require no changes. That being said, I'd prefer to delay merging until the SAW release is complete just in case. |
Matching conventions elsewhere in the codebase.
Fixes #1138.
While this PR adds lines overall, a large chunk of that is identity cases in pattern matches that are included for the sake of future-proofing those
cases. Fundamentally, this is a significant simplification.Some improvements to the
Castmodule that were enabled by this change:One breaking change: Crucible-LLVM S-expression files now have to use the exact signature of the underlying override. See the changes to tests for examples of this breakage. Not a huge deal, as the S-expression syntax is mostly developer-facing.
After this change, it would be feasible to completely remove the quasiquoters. I haven't done that in this PR, but would be happy to do so if requested. It might be nice to instead move them to the type level as quasiquoters for
LLVMOverride p sym ext args retwhereargsandretare determined by their LLVM types.