Bug 203351 - [JSC] Remove wasmAwareLexicalGlobalObject
Summary: [JSC] Remove wasmAwareLexicalGlobalObject
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-10-23 18:49 PDT by Yusuke Suzuki
Modified: 2019-10-23 22:35 PDT (History)
14 users (show)

See Also:

Attachments
Patch (25.88 KB, patch)
2019-10-23 18:50 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (25.85 KB, patch)
2019-10-23 18:51 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (25.86 KB, patch)
2019-10-23 19:09 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (25.92 KB, patch)
2019-10-23 20:15 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (25.95 KB, patch)
2019-10-23 20:25 PDT, Yusuke Suzuki 		mark.lam: review+
Details | Formatted Diff | Diff
Show Obsolete (4) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2019-10-23 18:49:28 PDT 
[JSC] Remove wasmAwareLexicalGlobalObject
Comment 1 Yusuke Suzuki 2019-10-23 18:50:11 PDT 
Created attachment 381761 [details]
Patch
Comment 2 Yusuke Suzuki 2019-10-23 18:51:06 PDT 
Created attachment 381762 [details]
Patch
Comment 3 Yusuke Suzuki 2019-10-23 19:09:49 PDT 
Created attachment 381764 [details]
Patch
Comment 4 Yusuke Suzuki 2019-10-23 20:15:11 PDT 
Created attachment 381769 [details]
Patch
Comment 5 Yusuke Suzuki 2019-10-23 20:25:14 PDT 
Created attachment 381770 [details]
Patch
Comment 6 Mark Lam 2019-10-23 21:56:05 PDT 
Comment on attachment 381770 [details]
Patch

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

r=me with suggested improvement.

> Source/JavaScriptCore/interpreter/CallFrame.cpp:356
> +JSGlobalObject* CallFrame::lexicalGlobalObjectFromWasmCallee(VM& vm) const
> +{
> +#if ENABLE(WEBASSEMBLY)
> +    return vm.wasmContext.load()->owner<JSWebAssemblyInstance>()->globalObject();
> +#else
> +    UNUSED_PARAM(vm);
> +    return nullptr;
> +#endif
> +}

Why not wrap the whole function in #if ENABLE(WEBASSEMBLY) and remove the #else case altogether?  After all, this function can only be called if ENABLE(WEBASSEMBLY).

> Source/JavaScriptCore/interpreter/CallFrame.h:170
> +        JS_EXPORT_PRIVATE JSGlobalObject* lexicalGlobalObjectFromWasmCallee(VM&) const;

Wrap in #if ENABLE(WEBASSEMBLY).
Comment 7 Yusuke Suzuki 2019-10-23 22:31:23 PDT 
Comment on attachment 381770 [details]
Patch

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

Thanks

>> Source/JavaScriptCore/interpreter/CallFrame.cpp:356
>> +}
> 
> Why not wrap the whole function in #if ENABLE(WEBASSEMBLY) and remove the #else case altogether?  After all, this function can only be called if ENABLE(WEBASSEMBLY).

I don't have strong preference here. OK, I'll just wrap it.
Comment 8 Yusuke Suzuki 2019-10-23 22:34:27 PDT 
Committed r251529: <https://trac.webkit.org/changeset/251529>
Comment 9 Radar WebKit Bug Importer 2019-10-23 22:35:12 PDT 
<rdar://problem/56568321>