[sandbox] Use SafeHeapObjectSize for WeakArrayList::length [v8/v8 : main]
Arash Kazemi (Gerrit)
Arash Kazemi voted and added 1 comment![]( "Open in Gerrit")
Votes added by Arash Kazemi
| Commit-Queue | +1 |
1 comment
// LINT.ThenChange(/src/objects/map.cc:get_visitor_id, /src/objects/js-objects.cc:get_header_size, /src/compiler/turbofan-types.cc:bitset_type_lub)Unless I am mistaken, WeakArrayList is already present in all of those.
Related details
- Maksim Ivanov
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Maksim Ivanov (Gerrit)
Maksim Ivanov voted and added 3 comments![]( "Open in Gerrit")
Votes added by Maksim Ivanov
| Code-Review | +1 |
3 comments
I see some potentially dangerous arithmetics done, but I couldn't spot any specific flaws (and new CHECKing will likely be expensive). So LGTM % nit.
const uint32_t len = length().value();nit: `length` or `ulength`? It's unclear which naming convention this chain of CLs want to follow.
const uint32_t derived_length = derived_list->length().value();nit: I think the variable isn't needed here, as the length can be calculated inside the `DCHECK`. And then `#ifdef` won't be required.
Ditto in the next function below.
Related details
- Arash Kazemi
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Arash Kazemi (Gerrit)
Arash Kazemi added 2 comments![]( "Open in Gerrit")
const uint32_t len = length().value();nit: `length` or `ulength`? It's unclear which naming convention this chain of CLs want to follow.
My plan is to remove `ulength` in a follow-up CL: crrev.com/c/7643769. We could also remove `length` and change all instances to `ulength`. I don't have a preference since either way we would be returning `SafeHeapObjectSize`, `ulength` would possibly make it even more explicit that it should be treated as unsigned.
const uint32_t derived_length = derived_list->length().value();nit: I think the variable isn't needed here, as the length can be calculated inside the `DCHECK`. And then `#ifdef` won't be required.
Ditto in the next function below.
- Maksim Ivanov
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |