...
Created attachment 371185 [details] Patch
Created attachment 371189 [details] Patch Fix windows build
Comment on attachment 371189 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=371189&action=review r=me > Source/JavaScriptCore/runtime/CachedTypes.cpp:176 > + Ref<CachedBytecode> releaseMapped(BytecodeCacheError& error) Kinda weird this returns ref instead of ptr. Why not RefPtr and make nullptr mean there was an error? I feel like it's weird to construct an empty CachedBytecode here. > Source/JavaScriptCore/runtime/CachedTypes.cpp:180 > + error.standardError(errno); nit: This style feels kinda weird. Why not just make error something you assign to? And by default it's empty? And you can make this a static function that is a factory constructor. I feel like that's more in line with the style of error handling I've seen elsewhere in WK. So something like: ``` error = BytecodeCacheError::standardError(errno) return nullptr ``` > Source/JavaScriptCore/runtime/CachedTypes.cpp:2411 > + Encoder encoder(vm, -1); You could also make -1 the default argument of this function.
Created attachment 371264 [details] Patch for landing
Comment on attachment 371264 [details] Patch for landing Clearing flags on attachment: 371264 Committed r246060: <https://trac.webkit.org/changeset/246060>
All reviewed patches have been landed. Closing bug.
<rdar://problem/51392697>