RESOLVED FIXED 202132
Refactor cellSize() out of VMInspector::verifyCellSize().
https://bugs.webkit.org/show_bug.cgi?id=202132
Summary Refactor cellSize() out of VMInspector::verifyCellSize().
Mark Lam
Reported 2019-09-23 23:05:06 PDT
Attachments
proposed patch. (12.25 KB, patch)
2019-09-23 23:46 PDT, Mark Lam
saam: review+
patch for landing. (13.65 KB, patch)
2019-09-24 00:08 PDT, Mark Lam
no flags
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
Note You need to log in before you can comment on or make changes to this bug.