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

Reply via email to