rpc-alt: loosen pure input layouts calculation #21560
Conversation
## Description If it appears based on context that a pure input is being used as multiple types, produce no layout for that input, rather than erroring out pure input layout calculation. ## Test plan Updated tests.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
amnn
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
| @@ -2895,10 +2906,19 @@ mod tests { | |||
| ], | |||
| }; | |||
|
|
|||
| insta::assert_snapshot!( | |||
| resolver.pure_input_layouts(&ptb).await.unwrap_err(), | |||
| @"Conflicting types for input 3: u64 and u32" | |||
There was a problem hiding this comment.
How come the snap is newly added? Did we not have a snap with the conflicting type error before?
There was a problem hiding this comment.
Previously the snapshot was inline (that's what the @ means) and was an error message, now it's a longer output, moved to a snap.
| if prev != tag { | ||
| return Err(Error::InputTypeConflict(ix, prev.clone(), tag.clone())); | ||
| *type_ = Some(Err(())); |
There was a problem hiding this comment.
I wanted to understand this change better: at the call site, what happens when we see a type is Some(Err)?
There was a problem hiding this comment.
At the output of this function, Some(Err()) and None are represented the same (the absence of a MoveTypeLayout) -- that's what the new test shows -- inside the function, we need to distinguish these cases, because when we encounter a new use of the input, we need to know whether it's empty because we haven't seen a usage before (in which case, fill it), or because we have seen conflicting usages before (in which case, leave it empty).
| ??? | ||
| u64 | ||
| ??? | ||
| ??? |
There was a problem hiding this comment.
Adding to your earlier question: This is the input that has conflicting usages (which caused the error previously). Now it shows up as having no discernable layout.
| if prev != tag { | ||
| return Err(Error::InputTypeConflict(ix, prev.clone(), tag.clone())); | ||
| *type_ = Some(Err(())); |
There was a problem hiding this comment.
is InputTypeConflict enum still necessary after this change, let's remove it from error.rs otherwise?
Description
If it appears based on context that a pure input is being used as multiple types, produce no layout for that input, rather than erroring out pure input layout calculation.
Test plan
Updated tests.
Release notes
Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.
For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.