Bug 175990 - Demarcate code added due to lack of NSDMI for aggregates
Summary: Demarcate code added due to lack of NSDMI for aggregates
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Local Build
Hardware: All All
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-08-25 12:03 PDT by Daniel Bates
Modified: 2017-08-25 16:43 PDT (History)
8 users (show)

See Also:


Attachments
Patch (20.18 KB, patch)
2017-08-25 12:15 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch (20.35 KB, patch)
2017-08-25 12:17 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>