ahatanak marked an inline comment as done. ahatanak added inline comments.
================ Comment at: lib/CodeGen/CGExpr.cpp:4100 + if (E->getType().isDestructedType() == QualType::DK_nontrivial_c_struct) + pushDestroy(QualType::DK_nontrivial_c_struct, DeclPtr, E->getType()); + ---------------- rjmccall wrote: > ahatanak wrote: > > rjmccall wrote: > > > ahatanak wrote: > > > > rjmccall wrote: > > > > > rjmccall wrote: > > > > > > Unfortunately, the lifetime of compound literals in C is not this > > > > > > simple; they're like blocks in that they're destroyed at the end of > > > > > > the enclosing scope rather than at the end of the current > > > > > > statement. (The cleanup here will be popped at the end of the > > > > > > full-expression if we've entered an `ExprWithCleanups`.) And the > > > > > > l-value case is exactly the case where this matters. > > > > > > > > > > > > I think you need to do something like what we do with blocks, where > > > > > > we record all the blocks in the full-expression on the > > > > > > `ExprWithCleanups` so that we can push an inactive cleanup for them > > > > > > and then activate it when we emit the block. > > > > > Can we make the check here something like (1) this is a block-scope > > > > > compound literal and (2) it has a non-trivially-destructed type (of > > > > > any kind)? That way we're not conflating two potentially unrelated > > > > > elements, the lifetime of the object and the kinds of types that can > > > > > be constructed by the literal. > > > > > > > > > > Oh, actually, there's a concrete reason to do this: C99 compound > > > > > literals are not required to have struct type; they can have any > > > > > object type, including arrays but also scalars. So we could, even > > > > > without non-trivial C structs, have a block-scope compound of type > > > > > `__strong id[]`; I guess we've always just gotten this wrong. Please > > > > > add tests for this case. :) > > > > There is a check `E->isFileScope()` above this. Is that sufficient to > > > > check for block-scoped compound literals? > > > That plus the C/C++ difference; compound literals in C++ are just > > > temporaries. > > I haven't been able to come up with a piece of C++ code that executes > > `EmitCompoundLiteralLValue`. The following code gets rejected because you > > can't take the address of a temporary object in C++: > > > > ``` > > StrongSmall *p = &(StrongSmall){ 1, 0 }; > > ``` > > > > If a bind a reference to it, `AggExprEmitter::VisitCompoundLiteralExpr` is > > called. > That makes sense; they're not gl-values in C++. It would be reasonable to > assert that. But the C++ point does apply elsewhere. It turns out this function is called in C++ when the compound literal is a vector type, so I've just added a check for C++ instead of an assert. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64464/new/ https://reviews.llvm.org/D64464 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits