Summary: | Start using C++17 | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alex Christensen <achristensen> | ||||||||||
Component: | New Bugs | Assignee: | Alex Christensen <achristensen> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | darin, ews-watchlist, Hironori.Fujii, jfbastien, mcatanzaro, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Alex Christensen
2019-04-19 18:16:35 PDT
Created attachment 367859 [details]
Patch
Created attachment 367867 [details]
Patch
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 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
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? (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. 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.) 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. Mac doesn't build with non-gnu c++17 right now. We can make that change later. Created attachment 368065 [details]
Patch
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.
(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 This also required the internal change ad34b33778bdc8b8f7644540ed72d8b11c51ab0e *** Bug 185176 has been marked as a duplicate of this bug. *** |