Bug 185176 – Use more C++17

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: Tools / Tests

[JF Bastien]

Move the Xcode builds to C++17.

[JF Bastien]

Created attachment 339244 [details]
WIP

WIP, contains changes from https://bugs.webkit.org/show_bug.cgi?id=185159

[EWS Watchlist]

Attachment 339244 [details] did not pass style-queue:


ERROR: Source/WebKit/Shared/SandboxExtension.h:123:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebKit/Shared/SandboxExtension.h:123:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WTF/wtf/Optional.h:61:  in_place_t is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WTF/wtf/Optional.h:63:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WTF/wtf/Optional.h:63:  Missing space inside { }.  [whitespace/braces] [5]
ERROR: Source/WTF/wtf/Optional.h:1071:  This { should be at the end of the previous line  [whitespace/braces] [4]
Total errors found: 6 in 33 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[JF Bastien]

Created attachment 339245 [details]
WIP

Fix Linux issue.

[EWS Watchlist]

Attachment 339245 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/StdLibExtras.h:555:  in_place_t is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WTF/wtf/StdLibExtras.h:557:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WTF/wtf/StdLibExtras.h:557:  Missing space inside { }.  [whitespace/braces] [5]
ERROR: Source/WebKit/Shared/SandboxExtension.h:123:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebKit/Shared/SandboxExtension.h:123:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WTF/wtf/Optional.h:1057:  This { should be at the end of the previous line  [whitespace/braces] [4]
Total errors found: 6 in 34 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Arunprasad]

@jfbastien, What is the minimum Xcode version required to build with this change?

[Yusuke Suzuki]

Created attachment 340503 [details]
Patch

[JF Bastien]

Comment on attachment 340503 [details]
Patch

Is that all that was missing? I had to work around `register` and other issues too. Or was that fixed in other patches?

Anyhow, lgtm if bots happy, assuming Linux also works.

[EWS Watchlist]

Note that there are important steps to take when updating ANGLE. See http://trac.webkit.org/wiki/UpdatingANGLE

[EWS Watchlist]

Attachment 340503 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 33 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Yusuke Suzuki]

Created attachment 340508 [details]
Trial #2

[Yusuke Suzuki]

(In reply to JF Bastien from comment #7)
> Comment on attachment 340503 [details]
> Patch
> 
> Is that all that was missing? I had to work around `register` and other
> issues too. Or was that fixed in other patches?
> 
> Anyhow, lgtm if bots happy, assuming Linux also works.

The patch is a bit shrunk: Just enabling C++17 for Xcode.
In my local environment, it works fine. But the bot cannot use gnu++17 option...
Can we upgrade the bot's clang?

[Yusuke Suzuki]

Currently I'm using C++14 for ANGLE and libwebrtc. libwebrtc requires a bit change to adopt C++17. The change is posted to the upstream[1].

[1]: https://webrtc-review.googlesource.com/c/src/+/77280

[EWS Watchlist]

Comment on attachment 340503 [details]
Patch

Attachment 340503 [details] did not pass win-ews (win):
Output: http://webkit-queues.webkit.org/results/7705670

New failing tests:
http/tests/security/canvas-remote-read-remote-video-blocked-no-crossorigin.html

[EWS Watchlist]

Created attachment 340557 [details]
Archive of layout-test-results from ews202 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews202  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Yusuke Suzuki]

I think the bot is using clang-4 or so. I'll change xcode option to gnu++1z to make EWS green. Please change this option once build bot is upgraded.

[Yusuke Suzuki]

Created attachment 340611 [details]
Patch for landing

[Yusuke Suzuki]

Created attachment 340624 [details]
Patch for landing

[EWS Watchlist]

Comment on attachment 340611 [details]
Patch for landing

Attachment 340611 [details] did not pass win-ews (win):
Output: http://webkit-queues.webkit.org/results/7722016

New failing tests:
http/tests/security/local-video-source-from-remote.html

[EWS Watchlist]

Created attachment 340697 [details]
Archive of layout-test-results from ews202 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews202  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Yusuke Suzuki]

Created attachment 340858 [details]
Patch

[Yusuke Suzuki]

EWS becomes green, let's land it.

[Yusuke Suzuki]

Committed r232052: <https://trac.webkit.org/changeset/232052>

[Radar WebKit Bug Importer]

<rdar://problem/40442803>

[Ryan Haddad]

Though this change is green on EWS and on the webkit.org bots, it causes multiple Apple internal builds to fail. I need to roll this out for now.

Jonathan is going to try to work through some of the build errors that we are seeing so that we can attempt to land this again without any unexpected breakage.

[Ryan Haddad]

Reverted r232052 for reason:

Breaks internal builds.

Committed r232069: <https://trac.webkit.org/changeset/232069>

[Alexey Proskuryakov]

This did happen via bug 197131.

*** This bug has been marked as a duplicate of bug 197131 ***