Bug 197131 – Start using C++17

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Alex Christensen]

Start using C++17

[Alex Christensen]

Created attachment 367859 [details]
Patch

[Alex Christensen]

Created attachment 367867 [details]
Patch

[EWS Watchlist]

Comment on attachment 367867 [details]
Patch

Attachment 367867 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11939188

New failing tests:
http/tests/paymentrequest/payment-response-payerName-attribute.https.html
http/tests/paymentrequest/payment-response-payerPhone-attribute.https.html
http/tests/paymentrequest/updateWith-method-pmi-handling.https.html
http/tests/paymentrequest/payment-request-change-shipping-address.https.html
http/tests/paymentrequest/payment-response-complete-method.https.html
http/tests/paymentrequest/payment-address-attributes-and-toJSON-method.https.html
http/tests/paymentrequest/payment-request-change-shipping-option.https.html
http/tests/paymentrequest/payment-response-payerEmail-attribute.https.html
http/tests/paymentrequest/payment-response-methodName-attribute.https.html
http/tests/paymentrequest/payment-response-retry-method.https.html
http/tests/paymentrequest/payment-request-merchant-validation.https.html
http/tests/paymentrequest/payment-request-show-method.https.html

[EWS Watchlist]

Created attachment 367883 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[Darin Adler]

Comment on attachment 367867 [details]
Patch

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

The "payment request" crashes in the iOS Simulator EWS results look like they might be real results of this change.

> Source/JavaScriptCore/inspector/InspectorBackendDispatcher.h:-114
> -    // This is necessary for some versions of Safari. Remove it when those versions of Safari are no longer supported.
> -    void reportProtocolError(WTF::DeprecatedOptional<long> relatedRequestId, CommonErrorCode, const String& errorMessage);

Does this really have a dependency on C++17? Can we land this change first/separately?

> Source/WTF/wtf/DeprecatedOptional.h:-48
> -template<typename T> using DeprecatedOptional = std::optional<T>;

Same question. Can we remove this header separately, first?

[Alex Christensen]

(In reply to Darin Adler from comment #5)
> Does this really have a dependency on C++17? Can we land this change
> first/separately?
I think this is caused by there being some C++17 headers that newly include <optional>, which conflicts with this implementation of std::optional.  I don't think it's still needed, so I just removed it.  I'll actually remove it in a separate patch.

[Michael Catanzaro]

I guess removing WTF::Optional will come next, right?

Also, it looks like you are building XCode with GNU language extensions enabled (-std=gnu++17) while we build CMake with GNU extensions disabled (-std=c++17). It makes sense to use the same language standard for all ports, so I'd suggest switching XCode to use -std=c++17 unless you are intentionally using GNU language extensions somewhere in Apple-specific code, in which case we could switch CMake ports to -std=gnu++17. (Clearly, none of the cross-platform code needs this, since CMake ports build without. Just slightly funny for the Apple ports to use GNU stuff that the Linux ports have disabled.)

[Alex Christensen]

That is indeed ironic.  I'm not aware of any intentional use of gnu extensions.  I agree we should all try to use the same language type.

Unfortunately, there are some who want to continue using WTF::Optional because it resets to WTF::nullopt when moved from, while std::optional typically does not.

[Alex Christensen]

Mac doesn't build with non-gnu c++17 right now.  We can make that change later.

[Alex Christensen]

Created attachment 368065 [details]
Patch

[Darin Adler]

Comment on attachment 368065 [details]
Patch

I’m not sure I understand the status of ports that are compiling with older versions of gcc, otherwise looks fine.

[Alex Christensen]

(In reply to Darin Adler from comment #11)
> Comment on attachment 368065 [details]
> Patch
> 
> I’m not sure I understand the status of ports that are compiling with older
> versions of gcc, otherwise looks fine.

This will allow them to use C++17.  If they want to use -std=c++1z instead of -std=c++17 on linux and use GCC 6 with partial support of C++17, they are free to do so.  As it is on Mac when I build with this patch it uses -std=gnu++1z

[Alex Christensen]

http://trac.webkit.org/r244653

[Radar WebKit Bug Importer]

<rdar://problem/50210496>

[Alex Christensen]

http://trac.webkit.org/r244654 :(

[Alex Christensen]

http://trac.webkit.org/r244656

[Alex Christensen]

https://trac.webkit.org/changeset/244662/webkit

[Alex Christensen]

This also required the internal change ad34b33778bdc8b8f7644540ed72d8b11c51ab0e

[Alex Christensen]

http://trac.webkit.org/r244692

[Alexey Proskuryakov]

*** Bug 185176 has been marked as a duplicate of this bug. ***