> From: John McCall [mailto:[email protected]]
> +  if (CapturedStmtInfo && CapturedStmtInfo->isThisParmVarDecl(&D))
> +    CapturedStmtInfo->setThisValue(Builder.CreateLoad(DeclPtr));
> +
> 
> Is there a reason you can't do this in GenerateCapturedFunction?

Will do.

> +    CXXThisValue = EmitLoadOfLValue(ThisLValue).getScalarVal();
> +  }
> +
> 
> Similarly, this seems like something to do in GenerateCapturedFunction.

Will do.

> 
> +      } else if (CapturedStmtInfo) {
> +        if (const FieldDecl *FD = CapturedStmtInfo->lookup(VD)) {
> +          QualType TagType = getContext().getTagDeclType(FD->getParent());
> +          LValue LV
> +            = MakeNaturalAlignAddrLValue(CapturedStmtInfo->getThisValue(),
> +                                         TagType);
> +
> +          return EmitLValueForField(LV, FD);
> +        }
> 
> Please extract out a function to do this;  it's basically the same as the 
> lambda
> case right above it.

Will do.

> 
> +  AggValueSlot Slot = CreateAggTemp(RecordTy, "agg.captured");  LValue
> + SlotLV = MakeAddrLValue(Slot.getAddr(), RecordTy,
> + Slot.getAlignment());
> 
> This is a weird use of AggValueSlot.  There should probably be a way to
> create a variable as an l-value, but for now, please just do:
>   SlotLV = MakeNaturalAlignAddrLValue(CreateMemTemp(...))

Okay.

> 
> +    ArrayRef<VarDecl *> ArrayIndexes;
> +    EmitInitializerForField(*CurField, LV, *I, ArrayIndexes);
> 
> Just pass ArrayRef<VarDecl*>() as an argument.

Will do.

> I'd look at what gets done for the start of a lambda function instead; it's
> closer to the model you have.

I don't see much special handling for the start of lambdas - they just 
initialize the captured struct and set up the decl map.  Is there something 
I've missed here?

> It doesn't look like you map the ObjC 'self' variable.

Should I?  I don't know much about ObjC.

> Otherwise, this looks good, but please add some more recursive-capture
> tests, most notably CapturedStmts inside CapturedStmts, but also (for
> completeness) CapturedStmts inside blocks and vice-versa.

Will do.

Thanks for the suggestions,

Ben

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to