-
Notifications
You must be signed in to change notification settings - Fork 272
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
[Ed25519] Wrap the return value of Ed25519.sign and .verify with Right
#4992
base: trunk
Are you sure you want to change the base?
Conversation
I'm a bit confused. These aren't in the list of wrappers, right? In which case I think you should just forget about defining the But also, it works with |
this is what I get from |
I can only think of two ways that'd happen.
The second shouldn't happen if the builtin is in one of the standard places, though ( |
BTW, do you have some local work that isn't pushed? It doesn't seem to me like what's in the PR should actually work. If the |
oh so I had manually edited some generated code to make a wrapper that didn't do anything. I'll be removing the FOp like you suggested. |
ok @dolio this is working now! I just needed to regenerate the racket library. |
These are just raising racket exceptions, right? Is that how they're supposed to work in the error cases? Seems weird that they return an |
The original unison impl has type |
Summary:
I haven't been able to figure out how to hook my new primops up to the
nativeEvalInContext
pipeline, but it looks like they're all set forcompile.native
. I just needed to wrap the return value inRight
, and the following tests now pass:Issue: XXX-XXXX
Test plan:
Add the above tests to the workspace, and then do
compile.native ed25519Tests ed-test
.The file
~/.cache/unisonlanguage/racket-tmp/ed-test.rkt
will then be produced, and running:racket ~/.cache/unisonlanguage/racket-tmp/ed-test.rkt
produces a successful test run!