Bug 202132 - Refactor cellSize() out of VMInspector::verifyCellSize().
Summary: Refactor cellSize() out of VMInspector::verifyCellSize().
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-09-23 23:05 PDT by Mark Lam
Modified: 2019-09-24 00:45 PDT (History)
11 users (show)

See Also:


Attachments
proposed patch. (12.25 KB, patch)
2019-09-23 23:46 PDT, Mark Lam
saam: review+
Details | Formatted Diff | Diff
patch for landing. (13.65 KB, patch)
2019-09-24 00:08 PDT, Mark Lam
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2019-09-23 23:05:06 PDT
See https://bugs.webkit.org/show_bug.cgi?id=202085#c14.
Comment 1 Mark Lam 2019-09-23 23:46:09 PDT
Created attachment 379436 [details]
proposed patch.
Comment 2 Saam Barati 2019-09-23 23:52:12 PDT
Comment on attachment 379436 [details]
proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=379436&action=review

> Source/JavaScriptCore/runtime/CellSize.h:36
> +inline size_t cellSize(VM& vm, JSCell* cell)

why not put isDynamicallySizedType in here too?

> Source/JavaScriptCore/runtime/CellSize.h:65
> +    default:

ASSERT(!isDynamicallySizedType(cell))?
Comment 3 Mark Lam 2019-09-24 00:01:54 PDT
Comment on attachment 379436 [details]
proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=379436&action=review

>> Source/JavaScriptCore/runtime/CellSize.h:36
>> +inline size_t cellSize(VM& vm, JSCell* cell)
> 
> why not put isDynamicallySizedType in here too?

I was thinking it belongs in JSType.h, but now that you mentioned it, it does pertain to cell sizing.  I'll move it here.

>> Source/JavaScriptCore/runtime/CellSize.h:65
>> +    default:
> 
> ASSERT(!isDynamicallySizedType(cell))?

I'll just put this whole switch statement in a "if (isDynamicallySizedType(cell))" instead.  That will allow me to put a RELEASE_ASSERT_NOT_REACHED() in the default case.
Comment 4 Mark Lam 2019-09-24 00:08:08 PDT
Created attachment 379438 [details]
patch for landing.
Comment 5 Mark Lam 2019-09-24 00:44:15 PDT
Thanks for the review.  Landed in r250289: <http://trac.webkit.org/r250289>.
Comment 6 Radar WebKit Bug Importer 2019-09-24 00:45:20 PDT
<rdar://problem/55653382>