wasmtime / PR #2400 Specify unsealed / unfilled blocks · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #2400 Specify unsealed / unfilled blocks

Wasmtime GitHub notifications bot (Nov 12 2020 at 04:35):

MattX opened PR #2400 from improve-finalize-msg to main:

Hello! I'm suggesting this quick change that's helped me with debugging. Required information below.

This change has not been discussed in an issue, but it's a cosmetic change which I don't think warrants deep debate.

This change adds an offending block identifier to error messages when a FunctionBuilder is finalized, but some blocks are not sealed or not filled. This should help with debugging. This also removes confusing / outdated references to dropping the FunctionBuilder in the messages.

There are no new test cases, because the only change is in an error message. All existing test cases still pass.

I will assign a random recommended reviewer, but feel free to unassign / reassign the PR.

Wasmtime GitHub notifications bot (Nov 12 2020 at 04:35):

MattX updated PR #2400 from improve-finalize-msg to main:

Hello! I'm suggesting this quick change that's helped me with debugging. Required information below.

This change has not been discussed in an issue, but it's a cosmetic change which I don't think warrants deep debate.

This change adds an offending block identifier to error messages when a FunctionBuilder is finalized, but some blocks are not sealed or not filled. This should help with debugging. This also removes confusing / outdated references to dropping the FunctionBuilder in the messages.

There are no new test cases, because the only change is in an error message. All existing test cases still pass.

I will assign a random recommended reviewer, but feel free to unassign / reassign the PR.

Wasmtime GitHub notifications bot (Nov 12 2020 at 04:38):

MattX edited PR #2400 from improve-finalize-msg to main:

Hello! I'm suggesting this quick change that's helped me with debugging. Required information below.

This change has not been discussed in an issue, but it's a cosmetic change which I don't think warrants deep debate.

This change adds an offending block identifier to error messages when a FunctionBuilder is finalized, but some blocks are not sealed or not filled. This should help with debugging. This also removes confusing / outdated references to dropping the FunctionBuilder in the messages.

There are no new test cases, because the only change is in an error message. All existing test cases still pass.

It doesn't look like I can assign a reviewer, let me know if there's anything I need to do on that front.

Wasmtime GitHub notifications bot (Nov 17 2020 at 00:59):

fitzgen submitted PR Review.

Wasmtime GitHub notifications bot (Nov 20 2020 at 21:22):

fitzgen submitted PR Review.

Wasmtime GitHub notifications bot (Nov 20 2020 at 21:22):

fitzgen submitted PR Review.

Wasmtime GitHub notifications bot (Nov 20 2020 at 21:22):

fitzgen created PR Review Comment:

We can make this a little less word-y by using assert! instead of if and panci!:

                assert!(
                    block_data.pristine || self.func_ctx.ssa.is_sealed(block),
                    "FunctionBuilder finalized, but block {} is not sealed",
                    block,
                );
                assert!(
                    block_data.pristine || block_data.filled,
                    "FunctionBuilder finalized, but block {} is not filled",
                    block,
                );

Wasmtime GitHub notifications bot (Nov 21 2020 at 05:03):

MattX updated PR #2400 from improve-finalize-msg to main:

Hello! I'm suggesting this quick change that's helped me with debugging. Required information below.

This change has not been discussed in an issue, but it's a cosmetic change which I don't think warrants deep debate.

This change adds an offending block identifier to error messages when a FunctionBuilder is finalized, but some blocks are not sealed or not filled. This should help with debugging. This also removes confusing / outdated references to dropping the FunctionBuilder in the messages.

There are no new test cases, because the only change is in an error message. All existing test cases still pass.

It doesn't look like I can assign a reviewer, let me know if there's anything I need to do on that front.

Wasmtime GitHub notifications bot (Nov 23 2020 at 16:45):

fitzgen submitted PR Review.

Wasmtime GitHub notifications bot (Nov 23 2020 at 16:45):

fitzgen merged PR #2400.

Last updated: Nov 22 2024 at 17:03 UTC