Auto-Submit | +1 |
Commit-Queue | +1 |
@Thibaud: Could you take a look, since you were the author of the original fix for try_table in https://crrev.com/c/6618664, this just extends a missed case during br_table decoding.
CC'ing Matthias as he's on top of many Wasm-in-JS inlining things and was the original reviewer for https://crrev.com/c/6618664.
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Code-Review | +1 |
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
[wasm] Fix DCHECK failure when inlining br_table into JS
This is analogous to https://crrev.com/c/6618664, where we forgot to fix
it not just for try_table but also br_table.
The issue is that we may not call `length()` after the decode interface
call, since the latter might have left the decoder in a failed state
(e.g., due to bailing out during Wasm-in-JS-inlining), which trips a
DHCECK in the former (see linked CL, same thing there).
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
DCHECK(!has_next());
I am not convinced by this. Can't we make it more robust, e.g.:
```suggestion
DCHECK(decoder_->failed() || !has_next());
```
This function is already bad enough. "Please call it only in this very short time frame, otherwise it isn't going to work" isn't great...
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
DCHECK(!has_next());
I am not convinced by this. Can't we make it more robust, e.g.:
```suggestion
DCHECK(decoder_->failed() || !has_next());
```
This function is already bad enough. "Please call it only in this very short time frame, otherwise it isn't going to work" isn't great...
Recording offline discussion: Right, let's loosen this invariant but not by the suggestion above but by returning false in `has_next()` when `!decoder->ok()` (in other words: remove the `VALIDATE` wrapper). I'll do that in a follow-up.
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
DCHECK(!has_next());
Daniel LehmannI am not convinced by this. Can't we make it more robust, e.g.:
```suggestion
DCHECK(decoder_->failed() || !has_next());
```
This function is already bad enough. "Please call it only in this very short time frame, otherwise it isn't going to work" isn't great...
Recording offline discussion: Right, let's loosen this invariant but not by the suggestion above but by returning false in `has_next()` when `!decoder->ok()` (in other words: remove the `VALIDATE` wrapper). I'll do that in a follow-up.
Continuing this proposed cleanup here: https://crrev.com/c/7073593
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |