rsmith added inline comments.
================
Comment at: clang/lib/Sema/SemaLambda.cpp:1278
+ if (CallOpCC == DefaultMember)
+ return DefaultFree;
+ return CallOpCC;
----------------
rsmith wrote:
> erichkeane wrote:
> > rjmccall wrote:
> > > ...I made this comment in my first review, but Phabricator threw it away.
> > >
> > > The attributes let you explicitly request the default method CC, right?
> > > I think you need to check for an explicit attribute rather than just
> > > checking CC identity. There should be an AttributedType in the sugar.
> > They do, but I can't seem to find a way to find them. The calling
> > convention is already merged into the functiontype by the time we get here,
> > the AttributedType isn't accessible.
> >
> > So it seems we don't distinguish between "modified by attribute",
> > "modified-default by command line", and "modified-default by TargetInfo."
> >
> > That said, I somewhat think this is the right thing to do anyway. If
> > you're on a platform where the default call convention is different between
> > a free-function and member-function, I'd think that this is what you MEAN...
> The `AttributedType` should be present in the type on the `TypeSourceInfo`
> for the call operator. It might not be present on the type retrieved by
> `getType()`, though.
>
> Concretely, what targets have different calling conventions for member versus
> non-member functions, and what do those calling conventions do differently?
> (For example, if the default member calling convention treats `this`
> differently, it doesn't seem reasonable to apply that to the static
> invoker...)
Answering my own question: the only case where the default member calling
convention is different from the default non-member calling convention is for
MinGW on 32-bit x86, where members use `thiscall` by default (which passes the
first parameter in `%ecx`).
Is it reasonable for `[] [[clang::thiscall]] {}` to result in a static invoker
using the `thiscall` calling convention? Intuitively I'd say no, but there's
not really anything specific to member functions in `thiscall` -- it just means
that we pass the first argument in a register. (However, the first argument of
the lambda and the first argument of its static invoker are different, so it's
still somewhat unclear whether inheriting `thiscall` is appropriate. But
usually for any given lambda only one of the two is actually used.)
But there's another quirk here: the default non-member calling convention can
be set on the command line with `-fdefault-calling-conv=` (and a few other
flags such as `-mrtd`). This doesn't affect member functions by default. So
what should happen if this is compiled with `-fdefault-calling-conv=stdcall`
(assuming the default calling convention is otherwise `cdecl`):
```
auto *p0 = [] [[clang::stdcal]] {};
auto *p1 = [] {};
auto *p2 = [] [[clang::cdecl]] {};
```
`p0` seems easy: the default non-member calling convention and the explicit
calling convention are the same. The invoker should be `stdcall`.
For `p1`, the default member calling convention is `cdecl` but the default
non-member calling convention is `stdcall`. In this case, conformance requires
us to use `stdcall` for the pointer type, because `p1` is required to have type
`void (*)()`, which is a `stdcall` function pointer in this configuration.
For `p2`, the call operator's calling convention has been explicitly set to the
default member calling convention. I think in this case I'd expect a `cdecl`
static invoker.
So I think I'm inclined to say we should always apply an explicit calling
convention to both the call operator and the static invoker -- that seems like
the simplest and easiest to explain rule, and probably matches user
expectations most of the time, especially given the observation that you're
usually writing a lambda only for one or the other of the call operator and the
static invoker, so if you explicitly write a calling convention attribute, you
presumably want it to apply to the part of the lambda's interface that you're
using.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89559/new/
https://reviews.llvm.org/D89559
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
