Update env to include delete-im, remove-EnvVal changes #794
Conversation
There was a problem hiding this comment.
Small suggestions inline, otherwise lgtm.
I would consider getting rid of the EnvVal and EnvObj types. The goal was in part to get rid of unnecessary concepts, but this PR seems to attempt to maintain those two concepts, which I think are unnecessary and I regret adding them. Because they just make one more thing to learn when navigating this code.
I would love so as well. I already removed EnvVal. |
I think yes. The attributes guarantees that the ABI of the inner value and struct are the same, so that you could transmute between them. More details here. We only need it on types that cross extern/exported functions, and the only types we have that do that are |
| @@ -111,16 +113,21 @@ impl PartialOrd for AccountId { | |||
|
|
|||
| impl Ord for AccountId { | |||
| fn cmp(&self, other: &Self) -> core::cmp::Ordering { | |||
| self.0.cmp(&other.0) | |||
| self.env.check_same_env(&other.env); | |||
There was a problem hiding this comment.
Good call to add this check.
What
Update env to include delete-im, remove-EnvVal changes
Why
[TODO: Why this change is being made. Include any context required to understand the why.]
Known limitations
[TODO or N/A]