[cfe-commits] r147723 - in /cfe/trunk: include/clang/AST/DeclCXX.h include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/ScopeInfo.h include/clang/Sema/Sema.h lib/AST/DeclCXX.cpp lib/Sema/Sema.cpp lib/Sema/SemaCodeComplete.cpp lib/Sema/Se

[Eli Friedman]

On Mon, Jan 9, 2012 at 11:09 AM, John McCall <rjmccall at apple.com> wrote:
> On Jan 9, 2012, at 10:44 AM, Eli Friedman wrote:
>> On Mon, Jan 9, 2012 at 9:26 AM, John McCall <rjmccall at apple.com> wrote:
>>> On Jan 6, 2012, at 8:59 PM, Eli Friedman wrote:
>>>> Modified: cfe/trunk/include/clang/Sema/ScopeInfo.h
>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/ScopeInfo.h?rev=147723&r1=147722&r2=147723&view=diff
>>>> ==============================================================================
>>>> --- cfe/trunk/include/clang/Sema/ScopeInfo.h (original)
>>>> +++ cfe/trunk/include/clang/Sema/ScopeInfo.h Fri Jan  6 22:59:52 2012
>>>> @@ -189,15 +189,20 @@
>>>>   /// \brief A mapping from the set of captured variables to the
>>>>   /// fields (within the lambda class) that represent the captured variables.
>>>>   llvm::DenseMap<VarDecl *, FieldDecl *> CapturedVariables;
>>>> -
>>>> +
>>>>   /// \brief The list of captured variables, starting with the explicit
>>>>   /// captures and then finishing with any implicit captures.
>>>>   llvm::SmallVector<Capture, 4> Captures;
>>>> -
>>>> +
>>>> +  // \brief Whether we have already captured 'this'.
>>>> +  bool CapturesCXXThis;
>>>> +
>>>>   /// \brief The number of captures in the \c Captures list that are
>>>>   /// explicit captures.
>>>>   unsigned NumExplicitCaptures;
>>>> -
>>>> +
>>>> +  LambdaCaptureDefault Default;
>>>> +
>>>>   /// \brief The field associated with the captured 'this' pointer.
>>>>   FieldDecl *ThisCapture;
>>>>
>>>> @@ -208,8 +213,9 @@
>>>>   QualType ReturnType;
>>>>
>>>>   LambdaScopeInfo(DiagnosticsEngine &Diag, CXXRecordDecl *Lambda)
>>>> -    : FunctionScopeInfo(Diag), Lambda(Lambda),
>>>> -      NumExplicitCaptures(0), ThisCapture(0) , HasImplicitReturnType(false)
>>>> +    : FunctionScopeInfo(Diag), Lambda(Lambda), CapturesCXXThis(false),
>>>> +      NumExplicitCaptures(0), Default(LCD_None), ThisCapture(0),
>>>> +      HasImplicitReturnType(false)
>>>>   {
>>>>     Kind = SK_Lambda;
>>>>   }
>>>
>>> Please do abstract out the common capture functionality between this and
>>> BlockScopeInfo.  There's at least one more kind of capture (VLA typedef)
>>> that really ought to go here in both cases.
>>
>> I don't think this will be much of a benefit, given that I can't think
>> of any code that can handle both blocks and lambdas without caring
>> which one is in use.  I can try it out anyway, I guess.
>>
>> I do not follow how VLA typedefs are in any way related; they don't
>> provide a function-like context (with labels etc.), and they don't
>> capture local variables.
>
> VLA typedefs capture values.  If you use a VLA typedef within a
> block/lambda, those captured values need to be captured by the block/lambda.

In that sense, any statement which can contain an expression captures
values.  That isn't relevant here.

>>> The isLambda() check should be sufficient and much cheaper.
>>
>> It apparently isn't sufficient... I see regression test failures
>> without that check.  I suppose you might consider it a bug in the
>> isLambda implementation?
>
> It certainly suggests a bug *somewhere*.  There are no forward-declarations
> of lambda classes.

The issue is that calling isLambda() with the current implementation
asserts if you call it on a forward-declaration of any class.  I guess
I'll just fix that.

-Eli