ahatanak marked an inline comment as done. ahatanak added inline comments.
================ Comment at: clang/lib/Sema/SemaExpr.cpp:677 + E->getType().isDestructedType() == QualType::DK_nontrivial_c_struct) + Cleanup.setExprNeedsCleanups(true); + ---------------- rjmccall wrote: > ahatanak wrote: > > rjmccall wrote: > > > Why only when the l-value is volatile? > > I was trying to avoid emitting `ExprWithCleanups` when it wasn't needed and > > it seemed that it wasn't needed for non-volatile types. I'm not sure it's > > important, so I've removed the check for volatile. Also, `ExprWithCleanups` > > shouldn't' be emitted when this is in file scope, so I fixed that too. > Hmm, not sure about this file-scope thing, since the combination of C++ > dynamic initializers and statement-expressions means we can have pretty > unrestricted code there. I should have explained why this was needed, but I wanted to prevent emitting `ExprWithCleanups` in the following example: ``` struct A { id f0; }; typedef struct A A; A g = (A){ .f0 = 0 }; ``` The l-value to r-value conversion happens here because compound literals are l-values. Since `g` is a global of a non-trivial C struct type, we shouldn't try to push a cleanup and destruct the object. We don't have to think about the C++ case since the line below checks the type is a non-trivial C type. I didn't think about statement expressions, but they aren't allowed in file scope, so I guess that's not a problem either. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66094/new/ https://reviews.llvm.org/D66094 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits