[cxx-abi-dev] lambda ABI inline function ODR compatibility issue

[John McCall]

On Jan 17, 2013, at 6:38 PM, Richard Smith <richardsmith at google.com> wrote:
> On Thu, Jan 17, 2013 at 6:01 PM, John McCall <rjmccall at apple.com> wrote:
> On Jan 17, 2013, at 4:12 PM, Richard Smith <richardsmith at google.com> wrote:
>> On Thu, Jan 17, 2013 at 2:35 PM, Michael Wong <michaelw at ca.ibm.com> wrote:
>> Lawrence Crowl <crowl at googlers.com> wrote on 01/17/2013 04:50:52 PM:
>> 
>> > From:
>> > 
>> > Lawrence Crowl <crowl at googlers.com>
>> > 
>> > To:
>> > 
>> > Michael Wong/Toronto/IBM at IBMCA
>> > 
>> > Cc:
>> > 
>> > cxx-abi-dev at codesourcery.com
>> > 
>> > Date:
>> > 
>> > 01/17/2013 04:50 PM
>> > 
>> > Subject:
>> > 
>> > Re: [cxx-abi-dev] lambda ABI inline function ODR compatibility issue
>> 
>> > 
>> > On 1/16/13, Michael Wong <michaelw at ca.ibm.com> wrote:
>> > > Does the C++ Standard committee intend for the ODR to imply that lambdas
>> > > need to have an ABI specified layout in order to deal with inline
>> > > functions.
>> > 
>> > I believe that we thought it was not an issue.
>> 
>> > 
>> > >
>> > > Consider the following with one object compiled with -DMAIN and another
>> > > without:
>> > > - in one case the layout needs to be compatible between different
>> > > implementations since the static local is shared between translation units
>> > >
>> > > /data/a.o: In function `main':
>> > > a.cpp:(.text+0x18): undefined reference to `bar()'
>> > > collect2: error: ld returned 1 exit status
>> > >
>> > > - in the other case, the layout needs to be compatible between different
>> > > implementations in order to satisfy the ODR requirement that the program
>> > > behave as if there was only one definition of the inline function
>> > >
>> > > extern "C" int printf(const char *, ...);
>> > > extern long gz;
>> > >
>> > > inline void foo() {
>> > >    long x = 0, q = 0, &z = gz;
>> > >    static auto f = [=, &z]() mutable { q += ++x; gz = q + x; };
>> > >
>> > >    long a, b;
>> > >    auto ff = [=]{ sizeof(a /*not an odr-use*/), printf("%u\n", &b < &a); };
>> > >    f();
>> > >    ff();
>> > > }
>> > >
>> > > void bar();
>> > >
>> > > #if ! MAIN
>> > > void bar() { foo(); }
>> > > #else
>> > > long gz;
>> > > int main() {
>> > >    foo();
>> > >    bar();
>> > >    foo();
>> > >    return gz;
>> > > }
>> > > #endif
>> > 
>> > And this code demonstrates that it is an issue.
>> Agreed.
>> 
>> > 
>> > Do you have a proposal?
>> 
>> Probably 2 thoughts:
>> 1. Make a static in an inline a violation of the ODR rule in the C++ Std (add in suitable wording covering all uses with external linkage)or 
>> 2. create an ABI binding in the C++ ABI such that all vendors follow the same behavior in this case
>> 
>> I think I am OK with either solution.
>> There may be other solutions I have not entertained yet ...
>> 
>> For option 1, we would only need to disallow static local variables from having types involving local lambdas with captures, right? All the other problems I can think of would be handled by putting the lambda's symbols in a COMDAT with the containing function.
> 
> We could also have problems with template specializations involving the lambda type, no?
> 
> template <class T> InstanceCount {
>   static int count;
> };
> template <class T> int InstanceCount<T>::count = 0;
> 
> inline void foo(int x) {
>   auto lambda = [=] { return x; };
>   InstanceCount<decltype(lambda)>::count++; // is this count consistent across translation units?
> }
> 
> That's OK; it just needs the mangling of the lambda's type.
> 
> This case seems more problematic:
> 
> void *p;
> inline void f(int a, int b) {
>   auto lambda = [=] { return a + b; };
>   if (p)
>     (*reinterpret_cast<decltype(lambda)*>(p))();
>   else
>     p = new auto(lambda);
> }

Right, sorry, I assumed it would be taken as given that a template
specialization could do something that actually relied on the layout
of the lambda type.

Basically, different translation units either agree that the lambda type
is the same or they don't.  If they don't, my example breaks.  If they do,
then (almost) any template specialization used which involves that
type or properties thereof (char_buffer<sizeof(lambda)>!  Enjoy
reasoning about that!) becomes an ODR violation if the translation
units disagree.

Normally, in a situation like this, I would say that the right thing to do
is to make strong guarantees but liberally exploit as-if.  Unfortunately,
as-if reasoning about lambda types is probably infeasible for nearly
every interesting use case.  By design, the feature just bleeds the
lambda type onto everything;  it escapes in ways that would be challenging
to limit.  Those few library features (like std::function) that do erase the
lambda type necessarily use things like polymorphic classes that are
quite challenging to reason about.  We'd likely end up just white-listing
those templates, maybe with an attribute.

I see three options:

1.  Guarantee the layout of lambdas in functions with weak linkage.
We'd still be able to optimize all other lambdas, so this isn't really that
bad;  it's just a bit disappointing for us compiler hackers and (a subset
of) our users.

2.  Ban lambdas in functions with weak linkage, similar to how C bans
static variables in (C's definition of) inline functions.  Of course, "weak
linkage" is not a concept in the standard, and you'd have to formalize
that quite carefully to avoid sweeping up a ton of interesting cases
involving anonymous namespaces.  And, of course, this would mean
banning a bunch of code that doesn't actually run afoul of this.

3.  Give lambdas internal linkage by fiat and hack the ODR to make
that work out.  I imagine this rule would come across like "lambdas in
inline functions will behave like they have different types in different
translation units, and that's not a formal ODR violation, but if it affects
the semantics of your program, tough cookies."

I tend to favor #3, but I'll admit to not having really considered the
consequences.

John.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://sourcerytools.com/pipermail/cxx-abi-dev/attachments/20130117/429a5095/attachment-0001.html>