Stream: git-wasmtime

Topic: wasmtime / issue #5267 test: Remove destructor in cancel_...


view this post on Zulip Wasmtime GitHub notifications bot (Nov 15 2022 at 15:06):

alexcrichton commented on issue #5267:

Thanks for the PR, but I believe the test is working as intended. The test intentionally is moving the destructor into the returned future, not as part of simply the initial closure to create the future.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 15 2022 at 15:31):

MediosZ commented on issue #5267:

Thanks for the clarification!
But why calling the destructor before yielding?
Now the destructor is called before cancelling the future.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 15 2022 at 17:14):

alexcrichton commented on issue #5267:

The destructor isn't called due to the & in drop(&dtor), that just serves for moving it into the future where it's forced to live in the automatically-closed-over-environment until the end.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 15 2022 at 17:25):

jameysharp commented on issue #5267:

Wow, that's subtle. I would never have guessed that's what drop(&dtor); was intended to do there.

Would something like let dtor = dtor; work instead? Or moving the let dtor = SetOnDrop(caller); statement to inside the closure?

At the least, @MediosZ, if you wanted to turn this PR into one that adds a comment explaining what's going on, that would be super helpful to anyone looking at this test later.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 15 2022 at 18:07):

alexcrichton commented on issue #5267:

Sure yeah I don't mind how this is written, I just want to make sure the test tests what it's supposed to test.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 16 2022 at 04:01):

MediosZ commented on issue #5267:

Sure, I believe a comment is needed to explain what happens here.


Last updated: Oct 23 2024 at 20:03 UTC