Bug 204443 – [MSVC] Add /experimental:newLambdaProcessor switch for better C++ conformance

• Status: RESOLVED
• Resolution: FIXED
• Component: CMake

[Fujii Hironori]

[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

[Fujii Hironori]

Created attachment 384030 [details]
Patch

[Fujii Hironori]

I expect this change will break Apple Win7 bots because they are still using VS2017.
They should be shut down (Bug 197288 Comment 12).

[Ross Kirsling]

I've had to fix this in r250793, r240797, and r251999 as well.

[Don Olmstead]

(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.

[Konstantin Tokarev]

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

[Fujii Hironori]

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?

[Fujii Hironori]

(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.

[Ross Kirsling]

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.

[Fujii Hironori]

review?

[Don Olmstead]

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

[Fujii Hironori]

Let's do that in: Bug 202842 – MSVC reports "wtf/RetainPtr.h(196): error C3861: 'CFAutorelease': identifier not found " with /permissive- on Windows

[Fujii Hironori]

Comment on attachment 384030 [details]
Patch

Clearing flags on attachment: 384030

Committed r253020: <https://trac.webkit.org/changeset/253020>

[Fujii Hironori]

All reviewed patches have been landed.  Closing bug.

[Fujii Hironori]

(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).

[Fujii Hironori]

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?

[Fujii Hironori]

Committed r253034: <https://trac.webkit.org/changeset/253034>

[Fujii Hironori]

Reverted in r253034. Reopened.

[Fujii Hironori]

WinCairo buildbot updated MSVC, and AppleWin removes Win7 bots (Bug 206702).
I'm going to land the patch again by fixing clang-cl issue.

[Fujii Hironori]

Created attachment 389225 [details]
Patch for landing

[Fujii Hironori]

Comment on attachment 389225 [details]
Patch for landing

Clearing flags on attachment: 389225

Committed r255418: <https://trac.webkit.org/changeset/255418>

[Fujii Hironori]

All reviewed patches have been landed.  Closing bug.

[Fujii Hironori]

See also: Bug 209358 – [MSVC] Remove experimental lambda processor usage