erichkeane added inline comments.
================
Comment at: clang/lib/Sema/SemaLambda.cpp:1278
+ if (CallOpCC == DefaultMember)
+ return DefaultFree;
+ return CallOpCC;
----------------
rsmith wrote:
> 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.
> 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.
Do we have a good way to determining which of the 3 mechanisms the user used to
set the calling convention? (Attribute vs -fdefault-calling-conv= vs just
using the defualt?)
It would be easy enough to ALWAYS have the static-invoker match the
calling-convention of the operator(), as long as having it as a 'this' call
isn't a problem, which given what you said above, I don't think thats a
problem. The implementation is trivial, since it is just removing the
condition above.
I guess I'm just asking which you'd like, and if it is the "only have invoker
match if user requested explicitly", if you had any hints on how to figure that
out.
Thanks!
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
