Bug 175990

Summary: Demarcate code added due to lack of NSDMI for aggregates
Product: WebKit Reporter: Daniel Bates <dbates>
Component: WebCore Misc.Assignee: Daniel Bates <dbates>
Status: RESOLVED FIXED    
Severity: Normal CC: aestes, buildbot, darin, ggaren, jfbastien, keith_miller, pvollan, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch none

Description Daniel Bates 2017-08-25 12:03:44 PDT
I propose that we demarcate constructors added to structs with non-static data member initializer (NSDMI) only to appease Visual Studio 2015 so as to make it easier to remove such code from the codebase once we transition to Visual Studio 2017. Visual Studio 2017 (*) and the version of clang and gcc we use on the EWS bots/Buildbots implement support for C++14 NSDMI for aggregates, <http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3605.html>.

For example, we have to implement JSC::Wasm::CallableFunction as follows because of the lack of NSDMI for aggregates in Visual Studio 2015:

[[
struct CallableFunction {
    CallableFunction() = default;

    CallableFunction(SignatureIndex signatureIndex, WasmEntrypointLoadLocation code = nullptr)
        : signatureIndex(signatureIndex)
        , code(code)
    {
    }

    static ptrdiff_t offsetOfWasmEntrypointLoadLocation() { return OBJECT_OFFSETOF(CallableFunction, code); }

    // FIXME pack the SignatureIndex and the code pointer into one 64-bit value. https://bugs.webkit.org/show_bug.cgi?id=165511
    SignatureIndex signatureIndex { Signature::invalidIndex };
    WasmEntrypointLoadLocation code { nullptr };
};
]]
<http://trac.webkit.org/browser/trunk/Source/JavaScriptCore/wasm/WasmFormat.h?rev=221190#L290>

But in Visual Studio 2017 and all other compilers that support NSDMI for aggregates, including clang, this code can be simplified to:

struct CallableFunction {
    static ptrdiff_t offsetOfWasmEntrypointLoadLocation() { return OBJECT_OFFSETOF(CallableFunction, code); }

    // FIXME pack the SignatureIndex and the code pointer into one 64-bit value. https://bugs.webkit.org/show_bug.cgi?id=165511
    SignatureIndex signatureIndex { Signature::invalidIndex };
    WasmEntrypointLoadLocation code { nullptr };
};

This pattern of adding constructors to appease Visual Studio 2015 occurs in other places in the codebase. We should demarcate them to make it easier to clean up such code.

(*) <https://docs.microsoft.com/en-us/cpp/cpp-conformance-improvements-2017>
Comment 1 Daniel Bates 2017-08-25 12:15:38 PDT
Created attachment 319092 [details]
Patch
Comment 2 Build Bot 2017-08-25 12:16:46 PDT
Attachment 319092 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/wasm/js/WebAssemblyFunction.cpp:194:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Daniel Bates 2017-08-25 12:17:43 PDT
Created attachment 319093 [details]
Patch
Comment 4 Build Bot 2017-08-25 12:19:48 PDT
Attachment 319093 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/wasm/js/WebAssemblyFunction.cpp:194:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Daniel Bates 2017-08-25 16:42:06 PDT
Comment on attachment 319093 [details]
Patch

Clearing flags on attachment: 319093

Committed r221213: <http://trac.webkit.org/changeset/221213>
Comment 6 Daniel Bates 2017-08-25 16:42:07 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 Radar WebKit Bug Importer 2017-08-25 16:43:00 PDT
<rdar://problem/34092309>