[sandbox] Convert usages for FixedArray::length to ulength [v8/v8 : main]

[Arash Kazemi (Gerrit)]

Attention needed from Leszek Swirski and Omer Katz

New activity on the change

Open in Gerrit

Related details

Attention is currently required from:

• Leszek Swirski
• Omer Katz

Submit Requirements:

• Code-Owners
• Code-Review
• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: v8/v8

Gerrit-Branch: main

Gerrit-Change-Id: I61efc4afbd34ca6bf55997207aa1e7dc53147ca6

Gerrit-Change-Number: 7548790

Gerrit-PatchSet: 11

Gerrit-Owner: Arash Kazemi <ara...@chromium.org>

Gerrit-Reviewer: Arash Kazemi <ara...@chromium.org>

Gerrit-Reviewer: Leszek Swirski <les...@chromium.org>

Gerrit-Reviewer: Omer Katz <omer...@chromium.org>

Gerrit-CC: AyeAye <android-bu...@system.gserviceaccount.com>

Gerrit-CC: V8 LUCI CQ <v8-s...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Attention: Omer Katz <omer...@chromium.org>

Gerrit-Attention: Leszek Swirski <les...@chromium.org>

Gerrit-Comment-Date: Mon, 09 Feb 2026 16:31:34 +0000

Gerrit-HasComments: No

Gerrit-Has-Labels: No

[Omer Katz (Gerrit)]

Attention needed from Arash Kazemi and Leszek Swirski

Omer Katz added 1 comment

File src/diagnostics/objects-printer.cc

Line 725, Patchset 11 (Latest): uint32_t array_len) {

Omer Katz . unresolved

I think it's good to pass the length here if we already read it, but we should add a DCHECK that `array->ulength().value()` is still equal to `array_len`.
Same for `PrintWeakArrayElements` and `ExtractWeakArrayReferences` below.

Open in Gerrit

Related details

Attention is currently required from:

• Arash Kazemi
• Leszek Swirski

Submit Requirements:

• Code-Owners
• Code-Review

• No-Unresolved-Comments

• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: v8/v8

Gerrit-Branch: main

Gerrit-Change-Id: I61efc4afbd34ca6bf55997207aa1e7dc53147ca6

Gerrit-Change-Number: 7548790

Gerrit-PatchSet: 11

Gerrit-Owner: Arash Kazemi <ara...@chromium.org>

Gerrit-Reviewer: Arash Kazemi <ara...@chromium.org>

Gerrit-Reviewer: Leszek Swirski <les...@chromium.org>

Gerrit-Reviewer: Omer Katz <omer...@chromium.org>

Gerrit-CC: AyeAye <android-bu...@system.gserviceaccount.com>

Gerrit-CC: V8 LUCI CQ <v8-s...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Attention: Arash Kazemi <ara...@chromium.org>

Gerrit-Attention: Leszek Swirski <les...@chromium.org>

Gerrit-Comment-Date: Tue, 10 Feb 2026 09:33:29 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Arash Kazemi (Gerrit)]

Attention needed from Leszek Swirski and Omer Katz

Arash Kazemi added 1 comment

File src/diagnostics/objects-printer.cc

Line 725, Patchset 11 (Latest): uint32_t array_len) {

Omer Katz . unresolved

I think it's good to pass the length here if we already read it, but we should add a DCHECK that `array->ulength().value()` is still equal to `array_len`.
Same for `PrintWeakArrayElements` and `ExtractWeakArrayReferences` below.

Arash Kazemi

I changed `array_len` to be a parameter because not all types passed to this function derive from `FixedArray`, so `ulength()` is not always defined or could return uint32_t instead. My plan was to look at those after fixing all instances of FixedArray but since I am splitting the work into multiple CLs, I might do it in this one.

Should I add similar methods for WeakArrayList, PropertyArray, Context, etc?

Open in Gerrit

Related details

Attention is currently required from:

• Leszek Swirski
• Omer Katz

Submit Requirements:

• Code-Owners
• Code-Review
• No-Unresolved-Comments
• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: v8/v8

Gerrit-Branch: main

Gerrit-Change-Id: I61efc4afbd34ca6bf55997207aa1e7dc53147ca6

Gerrit-Change-Number: 7548790

Gerrit-PatchSet: 11

Gerrit-Owner: Arash Kazemi <ara...@chromium.org>

Gerrit-Reviewer: Arash Kazemi <ara...@chromium.org>

Gerrit-Reviewer: Leszek Swirski <les...@chromium.org>

Gerrit-Reviewer: Omer Katz <omer...@chromium.org>

Gerrit-CC: AyeAye <android-bu...@system.gserviceaccount.com>

Gerrit-CC: V8 LUCI CQ <v8-s...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Attention: Omer Katz <omer...@chromium.org>

Gerrit-Attention: Leszek Swirski <les...@chromium.org>

Gerrit-Comment-Date: Tue, 10 Feb 2026 10:38:14 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

Comment-In-Reply-To: Omer Katz <omer...@chromium.org>

[Omer Katz (Gerrit)]

Attention needed from Arash Kazemi and Leszek Swirski

Omer Katz added 1 comment

File src/diagnostics/objects-printer.cc

Line 725, Patchset 11 (Latest): uint32_t array_len) {

Omer Katz . unresolved

I think it's good to pass the length here if we already read it, but we should add a DCHECK that `array->ulength().value()` is still equal to `array_len`.
Same for `PrintWeakArrayElements` and `ExtractWeakArrayReferences` below.

Arash Kazemi

I changed `array_len` to be a parameter because not all types passed to this function derive from `FixedArray`, so `ulength()` is not always defined or could return uint32_t instead. My plan was to look at those after fixing all instances of FixedArray but since I am splitting the work into multiple CLs, I might do it in this one.

Should I add similar methods for WeakArrayList, PropertyArray, Context, etc?

Omer Katz

Sounds good to me. Let's try adding `ulength` to the other types that need it. If it's not a huge change, we can inline it to this CL.

Open in Gerrit

Related details

Attention is currently required from:

• Arash Kazemi
• Leszek Swirski

Submit Requirements:

• Code-Owners
• Code-Review
• No-Unresolved-Comments
• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: v8/v8

Gerrit-Branch: main

Gerrit-Change-Id: I61efc4afbd34ca6bf55997207aa1e7dc53147ca6

Gerrit-Change-Number: 7548790

Gerrit-PatchSet: 11

Gerrit-Owner: Arash Kazemi <ara...@chromium.org>

Gerrit-Reviewer: Arash Kazemi <ara...@chromium.org>

Gerrit-Reviewer: Leszek Swirski <les...@chromium.org>

Gerrit-Reviewer: Omer Katz <omer...@chromium.org>

Gerrit-CC: AyeAye <android-bu...@system.gserviceaccount.com>

Gerrit-CC: V8 LUCI CQ <v8-s...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Attention: Arash Kazemi <ara...@chromium.org>

Gerrit-Attention: Leszek Swirski <les...@chromium.org>

Gerrit-Comment-Date: Tue, 10 Feb 2026 10:57:41 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

Comment-In-Reply-To: Omer Katz <omer...@chromium.org>

Comment-In-Reply-To: Arash Kazemi <ara...@chromium.org>

[Arash Kazemi (Gerrit)]

Attention needed from Leszek Swirski and Omer Katz

Arash Kazemi added 1 comment

File src/diagnostics/objects-printer.cc

Line 725, Patchset 11: uint32_t array_len) {

Omer Katz . resolved

I think it's good to pass the length here if we already read it, but we should add a DCHECK that `array->ulength().value()` is still equal to `array_len`.
Same for `PrintWeakArrayElements` and `ExtractWeakArrayReferences` below.

Arash Kazemi

I changed `array_len` to be a parameter because not all types passed to this function derive from `FixedArray`, so `ulength()` is not always defined or could return uint32_t instead. My plan was to look at those after fixing all instances of FixedArray but since I am splitting the work into multiple CLs, I might do it in this one.

Should I add similar methods for WeakArrayList, PropertyArray, Context, etc?

Omer Katz

Sounds good to me. Let's try adding `ulength` to the other types that need it. If it's not a huge change, we can inline it to this CL.

Arash Kazemi

I added the methods and left TODOs to change the callers of length(). I also reverted the change on `ExtractWeakArrayReferences` as I don't see possible double fetches in the caller of the function.

Open in Gerrit

Related details

Attention is currently required from:

• Leszek Swirski
• Omer Katz

Submit Requirements:

• Code-Owners
• Code-Review

• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: v8/v8

Gerrit-Branch: main

Gerrit-Change-Id: I61efc4afbd34ca6bf55997207aa1e7dc53147ca6

Gerrit-Change-Number: 7548790

Gerrit-PatchSet: 12

Gerrit-Owner: Arash Kazemi <ara...@chromium.org>

Gerrit-Reviewer: Arash Kazemi <ara...@chromium.org>

Gerrit-Reviewer: Leszek Swirski <les...@chromium.org>

Gerrit-Reviewer: Omer Katz <omer...@chromium.org>

Gerrit-CC: AyeAye <android-bu...@system.gserviceaccount.com>

Gerrit-CC: V8 LUCI CQ <v8-s...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Attention: Omer Katz <omer...@chromium.org>

Gerrit-Attention: Leszek Swirski <les...@chromium.org>

Gerrit-Comment-Date: Tue, 10 Feb 2026 12:42:51 +0000

[Omer Katz (Gerrit)]

Attention needed from Arash Kazemi and Leszek Swirski

Omer Katz voted and added 1 comment

Votes added by Omer Katz

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 12 (Latest):

Omer Katz . resolved

lgtm

Open in Gerrit

Related details

Attention is currently required from:

• Arash Kazemi
• Leszek Swirski

Submit Requirements:

• Code-Owners
• Code-Review

• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: v8/v8

Gerrit-Branch: main

Gerrit-Change-Id: I61efc4afbd34ca6bf55997207aa1e7dc53147ca6

Gerrit-Change-Number: 7548790

Gerrit-PatchSet: 12

Gerrit-Owner: Arash Kazemi <ara...@chromium.org>

Gerrit-Reviewer: Arash Kazemi <ara...@chromium.org>

Gerrit-Reviewer: Leszek Swirski <les...@chromium.org>

Gerrit-Reviewer: Omer Katz <omer...@chromium.org>

Gerrit-CC: AyeAye <android-bu...@system.gserviceaccount.com>

Gerrit-CC: V8 LUCI CQ <v8-s...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Attention: Arash Kazemi <ara...@chromium.org>

Gerrit-Attention: Leszek Swirski <les...@chromium.org>

Gerrit-Comment-Date: Tue, 10 Feb 2026 13:35:43 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: Yes