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

Test Fwk: Add conversions for ScVal<=>Into<RawVal> #106

Closed
leighmcculloch opened this issue Jun 8, 2022 · 5 comments · Fixed by #81
Closed

Test Fwk: Add conversions for ScVal<=>Into<RawVal> #106

leighmcculloch opened this issue Jun 8, 2022 · 5 comments · Fixed by #81
Assignees

Comments

@leighmcculloch
Copy link
Member

leighmcculloch commented Jun 8, 2022

We should add a impl TryFrom<ScVal> for RawVal that can convert an ScVal to a RawVal. It wouldn't be able to support all types but it could support all but the SCV_OBJECT arm.

For the XDR types that parallel types that are RawValConvertible we should also add a impl From<_> for RawVal. This would be for any arm that has a non-builtin type. i.e. SCV_STATIC, SCV_SYMBOL, and SCV_STATUS.

We can also implement conversions in the opposite direction and most will be symmetrical, except the SCV_POS_I64 arm that would not be able to convert the other way for negative values of int64.

@jonjove asked for how to do these conversions in a meeting today. @graydon suggested we should have this in the common package.

cc @jonjove @graydon

@leighmcculloch
Copy link
Member Author

Actually, I just remember, I took a stab at implementing this previously:

@graydon you and I discussed it in Slack (ref), and you had said to me that you preferred it not to escape the host. Do you still agree that these conversions shouldn't escape the host, or okay for me to revive that PR?

@leighmcculloch
Copy link
Member Author

@jonjove What were you needing this for?

@graydon
Copy link
Contributor

graydon commented Jun 9, 2022

I want the WeakHost type not to escape the host crate. I'm not as concerned about the general concept of conversion, though I suspect it will confuse users that some-but-not-all SCVals can be converted without an Env.

@leighmcculloch
Copy link
Member Author

I want the WeakHost type not to escape the host crate. I'm not as concerned about the general concept of conversion

Ah ok, I think I misunderstood your prior comment.

I suspect it will confuse users that some-but-not-all SCVals can be converted without an Env.

I planned to only implement the to and from EnvVal conversions, not the RawVal.

I'll revive #81 then.

@leighmcculloch leighmcculloch self-assigned this Jun 18, 2022
@leighmcculloch leighmcculloch changed the title Add [Try]From<ScVal/ScStatic/ScStatus/ScSymbol> for RawVal Test Fwk: Add conversions for ScVal<=>RawVal Jul 21, 2022
@leighmcculloch leighmcculloch changed the title Test Fwk: Add conversions for ScVal<=>RawVal Test Fwk: Add conversions for ScVal<=>Into<RawVal> Jul 21, 2022
@leighmcculloch
Copy link
Member Author

leighmcculloch commented Jul 21, 2022

If possible we should make this for conversions from ScVal from/to IntoEnvVal<E, V> so that we can seamless convert SDK types to and from ScVal types. This is requires for making stellar/rs-soroban-sdk#240 usable on all types of UDTs, since UDTs can contain SDK types.

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 a pull request may close this issue.

2 participants