Bug 202132

Summary: Refactor cellSize() out of VMInspector::verifyCellSize().
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: annulen, ews-watchlist, gyuyoung.kim, keith_miller, msaboff, rakuco, ryuan.choi, saam, sergio, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=202085
Attachments:
Description Flags
proposed patch.
saam: review+
patch for landing. none

Mark Lam
Reported 2019-09-23 23:05:06 PDT
See https://bugs.webkit.org/show_bug.cgi?id=202085#c14.
Attachments
proposed patch. (12.25 KB, patch)
2019-09-23 23:46 PDT, Mark Lam 		saam: review+
DetailsFormatted DiffDiff
patch for landing. (13.65 KB, patch)
2019-09-24 00:08 PDT, Mark Lam 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Mark Lam
Comment 1 2019-09-23 23:46:09 PDT
Created attachment 379436 [details] proposed patch.
Saam Barati
Comment 2 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))?
Mark Lam
Comment 3 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.
Mark Lam
Comment 4 2019-09-24 00:08:08 PDT
Created attachment 379438 [details] patch for landing.
Mark Lam
Comment 5 2019-09-24 00:44:15 PDT
Thanks for the review. Landed in r250289: <http://trac.webkit.org/r250289>.
Radar WebKit Bug Importer
Comment 6 2019-09-24 00:45:20 PDT
<rdar://problem/55653382>
Note You need to log in before you can comment on or make changes to this bug.