rjmccall added inline comments.

================
Comment at: lib/CodeGen/CodeGenFunction.cpp:1078
+  QualType T = FD->getReturnType();
+  if (T.isTriviallyCopyableType(Context)) {
+    // Avoid the optimization for functions that return trivially copyable
----------------
arphaman wrote:
> Quuxplusone wrote:
> > Peanut-gallery question: Does `isTriviallyCopyableType` also check that the 
> > type is trivially destructible and/or default-constructible? Do those 
> > things matter?
> Yes for trivially destructible, since it calls 
> `CXXRecordDecl::isTriviallyCopyable()` which checks. No for trivially 
> default-constructible, I fixed that. 
> 
> I think that for us it doesn't really matter that much, since we're mostly 
> worried about C code compiled in C++ mode.
The right condition is just hasTrivialDestructor().  The goal of 
-fno-strict-return is to not treat falling off the end of a function as 
undefined behavior in itself: it might still be a *problem* if users don't 
ignore the result, but we shouldn't escalate to UB in the callee because they 
legitimately might ignore the result.  The argument for having an exception 
around non-trivial types is that callers never *really* ignore the result when 
there's a non-trivial destructor.  Calling a destructor on storage that doesn't 
have a valid object of that type constructed there is already definitely U.B. 
in the caller, so it's fine to also treat it as U.B. on the callee side.  That 
reasoning is entirely based on the behavior of the destructor, though, not any 
of the copy/move constructors, and definitely not the presence or absence of a 
trivial default constructor.

In practice, all of these things tend to vary together, of course, except that 
sometimes trivial types do have non-trivial default constructors.  We should 
not treat that as a UB case.


Repository:
  rL LLVM

https://reviews.llvm.org/D27163



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to