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.html for
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);
}
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
