Page MenuHomePhabricator

Allow nested lambdas in NSDMIs (i.e default initializers)
ClosedPublic

Authored by faisalv on Sep 27 2013, 10:45 PM.

Details

Summary

This patch fixes an infinite recursion issue when computing the linkage-visibility of nested lambdas in NSDMIs.
Allows compilation of this:
template<class T>

struct L {
    T t{};
    T t2 = ([](int a) { return [](int b) { return b; };})(t)(t);    
};
L<int> l;

}

Doug had glanced at this change in Chicago as part of my larger generic lambda patch - he thought it would be better addressed and reviewed as a separate patch (since the bug occurs with non-generic lambdas too).

Thanks!

Diff Detail

Event Timeline

faisalv updated this revision to Unknown Object (????).Sep 28 2013, 1:29 AM

A slightly different approach - instead of finding the parent of the outermost lambda and checking for its linkage, first check if the outermost lambda has a mangling number, if not, return internal linkage for the nested lambda. If the outermost lambda does have a mangling number, then lets its parent determine its linkage/visibility. This approach doesn't break any of the recent codegen tests richard smith submitted for init-captures for lambdas.

doug.gregor requested changes to this revision.Sep 28 2013, 9:25 AM

The code looks good, but please add a test that actually checks the computed linkage.

test/SemaCXX/lambda-expressions.cpp
270

We're not actually testing the linkage here. Please provide a test that generates IR and uses FileCheck to make sure the lambda gets the appropriate LLVM linkage.

faisalv updated this revision to Unknown Object (????).Sep 29 2013, 1:35 PM

I added some IRgen tests to the code and then committed it - but the tests seemed to break a bunch of buildbots - so I reverted it - please help - what am i doing wrong?

faisalv updated this revision to Unknown Object (????).Sep 29 2013, 2:26 PM

These IR tests should now work - ok to commit?

doug.gregor accepted this revision.Sep 30 2013, 3:44 PM

Yep, LGTM.

faisalv closed this revision.Oct 26 2013, 8:34 AM