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 cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits