Bug 175990 – Demarcate code added due to lack of NSDMI for aggregates

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Daniel Bates]

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>

[Daniel Bates]

Created attachment 319092 [details]
Patch

[Build Bot]

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.

[Daniel Bates]

Created attachment 319093 [details]
Patch

[Build Bot]

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.

[Daniel Bates]

Comment on attachment 319093 [details]
Patch

Clearing flags on attachment: 319093

Committed r221213: <http://trac.webkit.org/changeset/221213>

[Daniel Bates]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/34092309>