This is an archive of the discontinued LLVM Phabricator instance.

Implement Lambda Conversion Operators for All CCs for MSVC.
ClosedPublic

Authored by erichkeane on Nov 2 2020, 11:11 AM.

Details

Summary

As described here:
https://devblogs.microsoft.com/oldnewthing/20150220-00/?p=44623

In order to allow Lambdas to be used with traditional Win32 APIs, they
emit a conversion function for (what Raymond Chen claims is all) a
number of the calling conventions. Through experimentation, we
discovered that the list isn't quite 'all'.

This patch implements this by taking the list of conversions that MSVC
emits (across 'all' architectures, I don't see any CCs on ARM), then
emits them if they are supported by the current target.

However, we also add 3 other options (which may be duplicates):
free-function, member-function, and operator() calling conventions. We
do this because we have an extension where we generate both free and
member for these cases so th at people specifying a calling convention
on the lambda will have the expected behavior when specifying one of
those two.

MSVC doesn't seem to permit specifying calling-convention on lambdas,
but we do, so we need to make sure those are emitted as well. We do this
so that clang-only conventions are supported if the user specifies them.

Diff Detail

Event Timeline

erichkeane created this revision.Nov 2 2020, 11:11 AM
erichkeane requested review of this revision.Nov 2 2020, 11:11 AM
aaron.ballman added inline comments.Nov 3 2020, 5:07 AM
clang/lib/Sema/SemaLambda.cpp
1281

Should we call out that one of the existing member functions is likely to be thiscall which means we'll generate a version of the operator for that calling convention even though MSVC doesn't, but we want to do this because users can explicitly write the CC on the lambda (unlike in MSVC)? (I'm worried that lack of mention about thiscall may look like a bug to someone a few years down the line.)

erichkeane updated this revision to Diff 302562.Nov 3 2020, 5:58 AM

update comment as @aaron.ballman requested.

aaron.ballman added inline comments.Nov 3 2020, 8:37 AM
clang/lib/Sema/SemaLambda.cpp
1282

If we're intentionally generating it, should it be listed explicitly in Convs below? (If not, then perhaps change the comment somewhat to "We intentionally generate a 'thiscall' (on Win32) implicitly from the default member call's calling convention..."?)

erichkeane updated this revision to Diff 302595.Nov 3 2020, 8:43 AM
erichkeane marked 2 inline comments as done.

Made another attempt at fixing the comment. Wordsmithing welcomed/encouraged :)

aaron.ballman accepted this revision.Nov 3 2020, 8:57 AM

This LGTM but you should wait a day or so in case @rjmccall has opinions.

This revision is now accepted and ready to land.Nov 3 2020, 8:57 AM

This LGTM but you should wait a day or so in case @rjmccall has opinions.

Thanks, will do!

clang/lib/Sema/SemaLambda.cpp
1281

Can do! I put it in the commit message as well,

rsmith added a subscriber: rsmith.Nov 3 2020, 2:35 PM
rsmith added inline comments.
clang/lib/Sema/SemaLambda.cpp
1285
clang/test/CodeGenCXX/lambda-conversion-op-cc.cpp
10

Does lambda-to-function-pointer decay still work (eg, +lambda or *lambda)? It'd be good to test that, since it's a fairly common idiom.

erichkeane added inline comments.Nov 3 2020, 5:43 PM
clang/test/CodeGenCXX/lambda-conversion-op-cc.cpp
10

It does! I tested that in the last patch, but don't seem to have a codegen test for it, so I'll make sure to add it.

I ended up having to do a tiebreaker as you suggested in that patch as well.

erichkeane updated this revision to Diff 302827.Nov 4 2020, 6:24 AM

Added test for +lambda as @rsmith requested.

Herald added a project: Restricted Project. · View Herald TranscriptNov 5 2020, 7:26 AM