[wasm][cleanup] Simplify decoder br/try/handler table iterators [v8/v8 : main]

0 views
Skip to first unread message

Daniel Lehmann (Gerrit)

unread,
Oct 22, 2025, 11:10:19 AM (yesterday) Oct 22
to Matthias Liedtke, Thibaud Michaud, V8 LUCI CQ, v8-re...@googlegroups.com, was...@google.com
Attention needed from Matthias Liedtke

Daniel Lehmann added 6 comments

Patchset-level comments
File-level comment, Patchset 1 (Latest):
Daniel Lehmann . resolved

Let's see if it sticks.

File src/wasm/function-body-decoder-impl.h
Line 4117, Patchset 1 (Latest): BranchTableIterator<ValidationTag> br_table_iterator(this, imm);
Daniel Lehmann . unresolved

Since that's used a good 50 lines below, I added a bit more context to the name.

Line 1022, Patchset 1 (Latest): DCHECK(has_next());
Daniel Lehmann . unresolved

Same here: enforce API for good measure.

Line 963, Patchset 1 (Latest): DCHECK(has_next());
Daniel Lehmann . unresolved

Probably a good idea to enforce the API contract here; analogously to br_table.

Line 4159, Patchset 1 (Parent): uint32_t br_table_length = iterator.length();
Daniel Lehmann . unresolved
Line 3781, Patchset 1 (Parent): uint32_t try_table_length = try_table_iterator.length();
Daniel Lehmann . unresolved

Reverted https://crrev.com/c/6618664 since no longer necessary.

Open in Gerrit

Related details

Attention is currently required from:
  • Matthias Liedtke
Submit Requirements:
  • requirement satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: v8/v8
Gerrit-Branch: main
Gerrit-Change-Id: I5b6cab948095b97e3768760824199ac3a238c417
Gerrit-Change-Number: 7073593
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Lehmann <dleh...@chromium.org>
Gerrit-Reviewer: Daniel Lehmann <dleh...@chromium.org>
Gerrit-Reviewer: Matthias Liedtke <mlie...@chromium.org>
Gerrit-CC: Thibaud Michaud <thib...@chromium.org>
Gerrit-Attention: Matthias Liedtke <mlie...@chromium.org>
Gerrit-Comment-Date: Wed, 22 Oct 2025 15:10:12 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

Matthias Liedtke (Gerrit)

unread,
Oct 22, 2025, 11:13:24 AM (yesterday) Oct 22
to Daniel Lehmann, Thibaud Michaud, V8 LUCI CQ, v8-re...@googlegroups.com, was...@google.com
Attention needed from Daniel Lehmann

Matthias Liedtke voted and added 6 comments

Votes added by Matthias Liedtke

Code-Review+1

6 comments

Patchset-level comments
Matthias Liedtke . resolved

LGTM, thanks a lot!

File src/wasm/function-body-decoder-impl.h
Line 4117, Patchset 1 (Latest): BranchTableIterator<ValidationTag> br_table_iterator(this, imm);
Daniel Lehmann . resolved

Since that's used a good 50 lines below, I added a bit more context to the name.

Matthias Liedtke

Acknowledged

Daniel Lehmann . resolved

Same here: enforce API for good measure.

Matthias Liedtke

Acknowledged

Line 963, Patchset 1 (Latest): DCHECK(has_next());
Daniel Lehmann . resolved

Probably a good idea to enforce the API contract here; analogously to br_table.

Matthias Liedtke

Yes, thanks!

Line 4159, Patchset 1 (Parent): uint32_t br_table_length = iterator.length();
Daniel Lehmann . resolved

Reverted from https://crrev.com/c/7072273.

Matthias Liedtke

Acknowledged

Line 3781, Patchset 1 (Parent): uint32_t try_table_length = try_table_iterator.length();
Daniel Lehmann . resolved

Reverted https://crrev.com/c/6618664 since no longer necessary.

Matthias Liedtke

Acknowledged

Open in Gerrit

Related details

Attention is currently required from:
  • Daniel Lehmann
Submit Requirements:
  • requirement satisfiedCode-Owners
  • requirement satisfiedCode-Review
  • requirement satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: v8/v8
Gerrit-Branch: main
Gerrit-Change-Id: I5b6cab948095b97e3768760824199ac3a238c417
Gerrit-Change-Number: 7073593
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Lehmann <dleh...@chromium.org>
Gerrit-Reviewer: Daniel Lehmann <dleh...@chromium.org>
Gerrit-Reviewer: Matthias Liedtke <mlie...@chromium.org>
Gerrit-CC: Thibaud Michaud <thib...@chromium.org>
Gerrit-Attention: Daniel Lehmann <dleh...@chromium.org>
Gerrit-Comment-Date: Wed, 22 Oct 2025 15:13:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Daniel Lehmann <dleh...@chromium.org>
satisfied_requirement
open
diffy

Daniel Lehmann (Gerrit)

unread,
Oct 22, 2025, 11:29:52 AM (24 hours ago) Oct 22
to Matthias Liedtke, Thibaud Michaud, V8 LUCI CQ, v8-re...@googlegroups.com, was...@google.com

Daniel Lehmann voted Commit-Queue+2

Commit-Queue+2
Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
  • requirement satisfiedCode-Owners
  • requirement satisfiedCode-Review
  • requirement satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: v8/v8
Gerrit-Branch: main
Gerrit-Change-Id: I5b6cab948095b97e3768760824199ac3a238c417
Gerrit-Change-Number: 7073593
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Lehmann <dleh...@chromium.org>
Gerrit-Reviewer: Daniel Lehmann <dleh...@chromium.org>
Gerrit-Reviewer: Matthias Liedtke <mlie...@chromium.org>
Gerrit-CC: Thibaud Michaud <thib...@chromium.org>
Gerrit-Comment-Date: Wed, 22 Oct 2025 15:29:47 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
satisfied_requirement
open
diffy

V8 LUCI CQ (Gerrit)

unread,
Oct 22, 2025, 11:52:07 AM (24 hours ago) Oct 22
to Daniel Lehmann, Matthias Liedtke, Thibaud Michaud, v8-re...@googlegroups.com, was...@google.com

V8 LUCI CQ submitted the change

Change information

Commit message:
[wasm][cleanup] Simplify decoder br/try/handler table iterators

Follow-up from https://crrev.com/c/7072273.
Change-Id: I5b6cab948095b97e3768760824199ac3a238c417
Reviewed-by: Matthias Liedtke <mlie...@chromium.org>
Commit-Queue: Daniel Lehmann <dleh...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#103297}
Files:
  • M src/wasm/function-body-decoder-impl.h
Change size: S
Delta: 1 file changed, 20 insertions(+), 21 deletions(-)
Branch: refs/heads/main
Submit Requirements:
  • requirement satisfiedCode-Review: +1 by Matthias Liedtke
Open in Gerrit
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: merged
Gerrit-Project: v8/v8
Gerrit-Branch: main
Gerrit-Change-Id: I5b6cab948095b97e3768760824199ac3a238c417
Gerrit-Change-Number: 7073593
Gerrit-PatchSet: 2
Gerrit-Owner: Daniel Lehmann <dleh...@chromium.org>
Gerrit-Reviewer: Daniel Lehmann <dleh...@chromium.org>
Gerrit-Reviewer: Matthias Liedtke <mlie...@chromium.org>
Gerrit-CC: Thibaud Michaud <thib...@chromium.org>
open
diffy
satisfied_requirement
Reply all
Reply to author
Forward
0 new messages