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

Upgrade to cosmwasm 0.16 #378

Merged
merged 2 commits into from
Aug 5, 2021
Merged

Upgrade to cosmwasm 0.16 #378

merged 2 commits into from
Aug 5, 2021

Conversation

ethanfrey
Copy link
Member

Closes #377

Copy link
Member Author

@ethanfrey ethanfrey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works, one open question

app.wrap()
.query_wasm_smart::<Reply, _, _>(&reflect_addr, &query)
.unwrap_err();
let res: StdResult<Reply> = app.wrap().query_wasm_smart(&reflect_addr, &query);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@uint I was forced to change this with the impl changes. I first changed <Reply, _, _> to simply <Reply>, but I got:

error[E0632]: cannot provide explicit generic arguments when `impl Trait` is used in argument position
   --> packages/multi-test/src/app.rs:676:33
    |
676 |             .query_wasm_smart::<Reply>(&reflect_addr, &query)
    |                                 ^^^^^ explicit generic argument not allowed

This is how I got it to compile. Maybe you have another idea?

(This is only a bit of a pain with unwrap_err(), so not a big deal, mainly curious)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ethanfrey Oh, huh. I had no idea that's a limitation. I think your change is perfectly fine, but I also think we should just switch back all the fns in the std that have type parameters alongside impl blocks. We (arguably) lose a little cosmetically, but if users get to use turbofish syntax again, that's worth it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then again, something like ::<Reply, _, _> is not pretty. No one sane will ever want to type out the last two types. Hm.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to change here.
Since this is only needed for unwrap_err() (normally we just do let res: Reply = ... ?, I would not change the API here. Just something to be brought up if there are cases where we have impl and generics mixed and the generic cannot be easily inferred by the input type or the return value. I believe this is the case some places where we take closures as arguments.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But let's not change the std lib now. I just wanted to flag it for reflection. One place in ~20 contract with a simple work-around doesn't make it a bad API decision.

@ethanfrey ethanfrey merged commit 01a8bc9 into main Aug 5, 2021
@ethanfrey ethanfrey deleted the 377-cosmwasm-0.16 branch August 5, 2021 17:37
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 this pull request may close these issues.

Upgrade CosmWasm to 0.16.0
2 participants