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:
> > > 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.
> I would hope that the constant-evaluator here might be smart enough to ignore
> some elidable temporaries.
Do you mean the constant-evaluator should evaluate initializers that are
`ExprWithCleanups` to constants in a case like this? It's possible to do so by
seeing whether the sub-expression of `ExprWithCleanups` is constant, but it
looks like we also have to set the `CleanupInfo::CleanupsHaveSideEffects` flag
to false when it's a file scope expression so that
`ConstExprEmitter::VisitExprWithCleanups` can constant-fold the
`ExprWithCleanups` initializer.
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
