[PATCH] CodeGen for CapturedStmt

Langmuir, Ben ben.langmuir at intel.com
Tue Apr 30 06:47:59 PDT 2013


> From: John McCall [mailto:rjmccall at apple.com]
> +  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




More information about the cfe-commits mailing list