[PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.



In https://reviews.llvm.org/D27163#607078, @rsmith wrote:

> A target-specific default for this, simply because there's a lot of code on Darwin that happens to violate this language rule, doesn't make sense to me.
>
> Basing the behavior on whether a `-Wreturn-type` warning would have been emitted seems like an extremely strange heuristic: only optimizing in the cases where we provide the user no hint that we will do so seems incredibly user-hostile.
>
> Regardless of anything else, it does not make any sense to "return" stack garbage when the return type is a C++ class type, particularly one with a non-trivial copy constructor or destructor. A trap at `-O0` is the kindest thing we can do in that case.


Thanks, that makes sense. The updated patch avoids the trap and unreachable only for types that are trivially copyable and removes the Darwin specific code.

> In summary, I think it could be reasonable to have such a flag to disable the trap/unreachable *only* for scalar types (or, at a push, trivially-copyable types). But it seems unreasonable to have different defaults for Darwin, or to look at whether a `-Wreturn-type` warning would have fired.

I understand that basing the optimisation avoidance on the analysis done for the `-Wreturn-type` warning might not be the best option, and a straightforward `-fno-strict-return` might be better as it doesn't use such hidden heuristics. However, AFAIK we only had problems with code that would've hit the `-Wreturn-type` warning, so it seemed like a good idea to keep the optimisation in functions where the analyser has determined that the flow with the no return is very unlikely (like in the `alwaysOptimizedReturn` function in the test case).  I kept it in this version of the patch, but if you think that the `-Wreturn-type` heuristic shouldn't be used I would be willing to remove it and go for the straightforward behaviour.

> Alternatively, since it seems you're only interested in the behavior of cases where `-Wreturn-type` would have fired, how about using Clang's tooling support to write a utility to add a return statement to the end of every function where it would fire, and give that to your users to fix their code?

That's an interesting idea, but as John mentioned, some of the code that has these issues isn't written by Apple and it seems that it would be very difficult to convince code owners to run tooling and to fix such issues. But it still sounds like something that we should look into.


Repository:
  rL LLVM

https://reviews.llvm.org/D27163