ahatanak marked an inline comment as done.
ahatanak added inline comments.
================
Comment at: lib/Sema/SemaExpr.cpp:16218
+ checkNonTrivialCUnion(E->getType(), E->getExprLoc(),
+ Sema::NTCUC_LValueToRValueVolatile);
+
----------------
rjmccall wrote:
> It looks like you're generally warning about this based on the specific
> context that forced an lvalue-to-rvalue conversion. I'm not sure `volatile`
> is special except that we actually perform the load even in unused-value
> contexts. Is the assumption that you've exhaustively covered all the other
> contexts of lvalue-to-rvalue conversions whose values will actually be used?
> That seems questionable to me.
Yes, that was my assumption. All the other contexts where lvalue-to-rvalue
conversion is performed and the result is used are already covered by other
calls sites of `checkNonTrivialCUnion`, which informs the users that the
struct/union is being used in an invalid context.
Do you have a case in mind that I didn't think of where a lvalue-to-rvalue
conversion requires a non-trivial initialization/destruction/copying of a union
but clang fails to emit any diagnostics?
Also I realized that lvalue-to-rvalue conversion of volatile types doesn't
always require non-trivial destruction, so I think `CheckDestruct` shouldn't be
set in this case.
```
void foo(U0 *a, volatile U0 *b) {
// this doesn't require destruction.
// this is perfectly valid if U0 is non-trivial to destruct but trivial to
copy.
*a = *b;
}
```
For the same reason, I think `CheckDestruct` shouldn't be set for function
returns (but it should be set for function parameters since they are destructed
by the callee).
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63753/new/
https://reviews.llvm.org/D63753
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
