r194448 - A quick fix to PR17877 that was introduced by r194188 (generic-lambda-capturing) that broke libc++.

Faisal Vali faisalv at yahoo.com
Mon Nov 11 19:48:27 PST 2013


Author: faisalv
Date: Mon Nov 11 21:48:27 2013
New Revision: 194448

URL: http://llvm.org/viewvc/llvm-project?rev=194448&view=rev
Log:
A quick fix to PR17877 that was introduced by r194188 (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 a lambda can capture the potential-captures of the innermost lambda.

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.

Patch was LGTM'd by Richard (just barely :)

http://llvm-reviews.chandlerc.com/D2144

Modified:
    cfe/trunk/lib/Sema/SemaExprCXX.cpp
    cfe/trunk/test/SemaCXX/cxx1y-generic-lambdas.cpp

Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=194448&r1=194447&r2=194448&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Mon Nov 11 21:48:27 2013
@@ -5988,9 +5988,24 @@ ExprResult Sema::ActOnFinishFullExpr(Exp
   //   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 getCurLambda() can return a valid pointer 
+  // even if CurContext is not a lambda call operator. Refer to that Bug Report
+  // for an example of the code that might cause this asynchrony.  
+  // By ensuring 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);
 }

Modified: cfe/trunk/test/SemaCXX/cxx1y-generic-lambdas.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/cxx1y-generic-lambdas.cpp?rev=194448&r1=194447&r2=194448&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/cxx1y-generic-lambdas.cpp (original)
+++ cfe/trunk/test/SemaCXX/cxx1y-generic-lambdas.cpp Mon Nov 11 21:48:27 2013
@@ -824,4 +824,25 @@ void finalizeDefaultAtomValues() {
 
 void f() { finalizeDefaultAtomValues<char>(); }
 
-} 
\ No newline at end of file
+} 
+
+namespace PR17877_lambda_declcontext_and_get_cur_lambda_disconnect {
+
+
+template<class T> struct U {
+  int t = 0;
+};
+
+template<class T>
+struct V { 
+  U<T> size() const { return U<T>{}; }
+};
+
+template<typename T>
+void Do() {
+  V<int> v{};
+  [=] { v.size(); };
+}
+
+
+}
\ No newline at end of file





More information about the cfe-commits mailing list