| Summary: | [MSVC] Add /experimental:newLambdaProcessor switch for better C++ conformance | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Fujii Hironori <Hironori.Fujii> | ||||||
| Component: | CMake | Assignee: | Fujii Hironori <Hironori.Fujii> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | achristensen, annulen, ap, bfulgham, cdumez, dbates, don.olmstead, esprehn+autocc, ews-watchlist, gyuyoung.kim, kangil.han, pvollan, ross.kirsling, ryuan.choi, sergio, sihui_liu | ||||||
| Priority: | P2 | ||||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=204437 | ||||||||
| Attachments: |
|
||||||||
|
Description
Fujii Hironori
2019-11-20 22:27:56 PST
Created attachment 384030 [details]
Patch
I expect this change will break Apple Win7 bots because they are still using VS2017. They should be shut down (Bug 197288 Comment 12). (In reply to Ross Kirsling from comment #3) > I've had to fix this in r250793, r240797, and r251999 as well. Pretty sure the EWS caught all the compiler failures. Ignoring them is also a problem. Note that experimental lamba processor has its own conformance issues, e.g. https://developercommunity.visualstudio.com/content/problem/729465/experimentalnewlambdaprocessor-not-able-to-process.html, so if folks are ignoring EWS they still migth break MSVC with valid C++ code Using clang-cl seems an ideal solution (Bug 171618). Seems no objection for the patch? I believe the patch reduce the maintenance for WebKit developers. Anyone can review? (In reply to Ross Kirsling from comment #3) > I've had to fix this in r250793, r240797, and r251999 as well. IIUC, these changes don't need to be reverted in this patch. I don't object as long as the conformance is better, but I agree that having folks not ignore EWS is the most important regardless. review? Are we really sure this is a good idea? I'd feel better if this was added to /permissive- https://docs.microsoft.com/en-us/cpp/build/reference/permissive-standards-conformance?view=vs-2019 or something along those lines Let's do that in: Bug 202842 – MSVC reports "wtf/RetainPtr.h(196): error C3861: 'CFAutorelease': identifier not found " with /permissive- on Windows Comment on attachment 384030 [details] Patch Clearing flags on attachment: 384030 Committed r253020: <https://trac.webkit.org/changeset/253020> All reviewed patches have been landed. Closing bug. (In reply to Fujii Hironori from comment #2) > I expect this change will break Apple Win7 bots because they are still using > VS2017. > They should be shut down (Bug 197288 Comment 12). This change didn't actually break the Win7 bots, but a lot of warning messages are started reporting. > cl : Command line warning D9002: ignoring unknown option '/experimental:newLambdaProcessor' [C:\cygwin\worker\win7-release-i386\build\WebKitBuild\Release\Tools\TestWebKitAPI\TestWTF.vcxproj] https://build.webkit.org/builders/Apple%20Win%207%20Release%20%28Build%29/builds/8953 They should be shut down (Bug 197288 Comment 12). Oops. My patch breaks WinCairo bots which is using VS2019 16.0. But, /experimental:newLambdaProcessor needs VS2019 16.1 or newer. Don, can you upgrade your Docker image? Committed r253034: <https://trac.webkit.org/changeset/253034> Reverted in r253034. Reopened. WinCairo buildbot updated MSVC, and AppleWin removes Win7 bots (Bug 206702). I'm going to land the patch again by fixing clang-cl issue. Created attachment 389225 [details]
Patch for landing
Comment on attachment 389225 [details] Patch for landing Clearing flags on attachment: 389225 Committed r255418: <https://trac.webkit.org/changeset/255418> All reviewed patches have been landed. Closing bug. See also: Bug 209358 – [MSVC] Remove experimental lambda processor usage |