Quuxplusone added inline comments.

================
Comment at: include/clang/Sema/AnalysisBasedWarnings.h:93
 
-  void IssueWarnings(Policy P, FunctionScopeInfo *fscope,
-                     const Decl *D, const BlockExpr *blkExpr);
+  void IssueWarnings(Policy P, FunctionScopeInfo *fscope, Decl *D,
+                     const BlockExpr *blkExpr);
----------------
If you remove the coupling between -fno-strict-return and -Wreturn-type, then 
you won't need to remove `const` in all these places. I think that alone is a 
good enough reason not to couple them.


================
Comment at: lib/CodeGen/CodeGenFunction.cpp:1078
+  QualType T = FD->getReturnType();
+  if (T.isTriviallyCopyableType(Context)) {
+    // Avoid the optimization for functions that return trivially copyable
----------------
Peanut-gallery question: Does `isTriviallyCopyableType` also check that the 
type is trivially destructible and/or default-constructible? Do those things 
matter?


================
Comment at: lib/CodeGen/CodeGenFunction.cpp:1173
+    } else if (CGM.getCodeGenOpts().StrictReturn ||
+               shouldUseUndefinedBehaviorReturnOptimization(FD, getContext())) 
{
+      if (CGM.getCodeGenOpts().OptimizationLevel == 0)
----------------
I'd expect this to look more like

    bool ShouldOmitUnreachable = !CGM.getCodeGenOpts().StrictReturn && 
FD->getReturnType().isTriviallyCopyableType(Context);
    // same ifs as the old code
    if (!ShouldOmitUnreachable) {
        Builder.CreateUnreachable();
        Builder.ClearInsertionPoint();
    }

Or in fact, is there a good reason this codepath isn't piggybacking on 
`FD->hasImplicitReturnZero()`?  Why not just give every function 
`hasImplicitReturnZero` when `-fno-strict-return` is in effect?

At which point, `-fno-strict-return` might seem like the wrong name; you could 
just call it something like `-fimplicit-return-zero`, which would also have the 
minor benefit of making the `-fno-*` option the default.


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