[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