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