[MSVC] Add /experimental:newLambdaProcessor switch for better C++ conformance MSVC has a bug of lambda capture of 'this'. It has caused compilation errors repeatedly. Bug 204437 – [MSVC] error C2039: 'weakPtrFactory': is not a member of 'WebCore::DocumentStorageAccess::requestStorageAccess::<lambda_3f2cfd7704f93d8fe19d5b5f064f8add>' Bug 187035 – [Win] 'deref': is not a member of 'WebKit::WebResourceLoadStatisticsStore::updateCookiePartitioningForDomains::<lambda_9d761a6dc12d95db7fa2d6f3f5aa26fa>' Bug 184120 – [Win] MSVC can't compile WebResourceLoadStatisticsStore::scheduleClearInMemoryAndPersistent r251999 /experimental:newLambdaProcessor https://docs.microsoft.com/en-us/cpp/overview/cpp-conformance-improvements?view=vs-2019#improvements_161
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).
I've had to fix this in r250793, r240797, and r251999 as well.
(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>
See also: Bug 209358 – [MSVC] Remove experimental lambda processor usage