LGTM as a solution to the immediate issue, at least.
On Mon, Nov 11, 2013 at 7:10 PM, Faisal Vali <[email protected]> wrote: > Hi rsmith, doug.gregor, > > A quick fix to the bug introduced by generic-lambda-capturing that broke > libc++. > See http://lists.cs.uiuc.edu/pipermail/cfe-dev/2013-November/033369.htmlfor > discussion on cfe-dev. > > This fix explicitly checks whether we are within the declcontext of a > lambda's call operator - which is what I had intended to be true (and > assumed would be true if getCurLambda returns a valid pointer) before > checking whether we should check to see if the lambda can capture. > > A deeper fix (that addresses why getCurLambda() returns a valid pointer > when perhaps it shouldn't?) - as proposed by Richard Smith in > http://llvm.org/bugs/show_bug.cgi?id=17877 - has been suggested as a > FIXME. > > Will try and implement that at a later date - figured it is prolly > important to not leave libc++ broken for too long (sorry about that). > > > > > http://llvm-reviews.chandlerc.com/D2144 > > Files: > lib/Sema/SemaExprCXX.cpp > > Index: lib/Sema/SemaExprCXX.cpp > =================================================================== > --- lib/Sema/SemaExprCXX.cpp > +++ lib/Sema/SemaExprCXX.cpp > @@ -5988,9 +5988,24 @@ > // full-expression +n + ({ 0; }); ends, but it's too late for us to > see that > // we need to capture n after all. > > - LambdaScopeInfo *const CurrentLSI = getCurLambda(); > - if (CurrentLSI && CurrentLSI->hasPotentialCaptures() && > - !FullExpr.isInvalid()) > + LambdaScopeInfo *const CurrentLSI = getCurLambda(); > + // FIXME: PR 17877 showed that CurrentLSI can exist even when the > + // CurContext is not a lambda call operator. Refer to that Bug Report > + // for an example of the code that might cause this asynchrony. > + // By checking whether we are in the context of a lambda's call operator > + // we can fix the bug (we only need to check whether we need to capture > + // if we are within a lambda's body); but per the comments in that > + // PR, a proper fix would entail : > + // "Alternative suggestion: > + // - Add to Sema an integer holding the smallest (outermost) scope > + // index that we are *lexically* within, and save/restore/set to > + // FunctionScopes.size() in InstantiatingTemplate's > + // constructor/destructor. > + // - Teach the handful of places that iterate over FunctionScopes to > + // stop at the outermost enclosing lexical scope." > + const bool IsInLambdaDeclContext = isLambdaCallOperator(CurContext); > + if (IsInLambdaDeclContext && CurrentLSI && > + CurrentLSI->hasPotentialCaptures() && !FullExpr.isInvalid()) > CheckLambdaCaptures(FE, CurrentLSI, *this); > return MaybeCreateExprWithCleanups(FullExpr); > } >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
