[PATCH] Fix PR17877

[Richard Smith]

LGTM as a solution to the immediate issue, at least.


On Mon, Nov 11, 2013 at 7:10 PM, Faisal Vali <faisalv at yahoo.com> 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);
>  }
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131111/999cb7ef/attachment.html>