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

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.


================
Comment at: lib/Sema/AnalysisBasedWarnings.cpp:530
+  // Don't need to mark Objective-C methods or blocks since the undefined
+  // behaviour optimization isn't used for them.
+}
----------------
This seems like a trap waiting to spring on someone, unless there's a technical reason that methods and blocks cannot possibly use the same optimization paths as regular functions. ("Nobody's gotten around to implementing it yet" is the most obvious nontechnical reason for the current difference.) Either way, I'd expect this patch to include test cases for both methods and blocks, to verify that the behavior you expect is actually the behavior that happens. Basically, it ought to have a regression test targeting the regression that I'm predicting is going to spring on someone as soon as they implement optimizations for methods and blocks.

Also, one dumb question: what about C++ lambdas? are they FunctionDecls too? test case?


================
Comment at: test/CodeGenCXX/return.cpp:21
+  // CHECK-NOSTRICT-NEXT: load
+  // CHECK-NOSTRICT-NEXT: ret i32
+  // CHECK-NOSTRICT-NEXT: }
----------------
Can you explain why a load from an uninitialized stack location would be *better* than a trap and/or `unreachable`? IIUC this is basically what Mehdi is asking: i.e., can you explain the rationale for this patch, because I don't get it either. It *seems* like a strict regression in code quality.


Repository:
  rL LLVM

https://reviews.llvm.org/D27163