[PATCH] D45643: [Failing one test] Reword [-Wreturn-type] messages to "non-void x does not return a value"

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

Quuxplusone added inline comments.


================
Comment at: bindings/python/tests/cindex/test_diagnostics.py:18
         self.assertEqual(tu.diagnostics[0].spelling,
-                'control reaches end of non-void function')
+                'non-void function does not return a value')
 
----------------
rsmith wrote:
> It seems like part of the problem here is that "non-void function" is sort-of nonsense due to a few missing words. How about:
> 
> > "control can reach end of function with non-void return type"
> 
> or similar?
> 
> I think we still need the mention of control flow, because we're *not* saying the function contains no return statements, we're saying there's a control flow path that reaches the end of the function.
I think OP's issue here is that the current message about "control" is intrinsically confusing. //Of course// "control can reach end of function"; if control never reaches the end of the function, you must have an infinite loop somewhere! The important missing piece of the current message is that control reaches the end of the function //without encountering any return statement//.

> we're *not* saying the function contains no return statements

Not sure, but I think in this case we //are// saying that. There's a different message, "non-void function //might// not return a value," for those other cases.

However, if I'm wrong about that, then another wording option close to OP's suggestion would be "non-void function does not //always// return a value" / "non-void function might not //always// return a value."


================
Comment at: include/clang/Basic/DiagnosticASTKinds.td:33
 def note_constexpr_no_return : Note<
-  "control reached end of constexpr function">;
+  "constexpr function does not return a value">;
 def note_constexpr_virtual_call : Note<
----------------
I think this should remain past-tense, since it's describing a situation that actually //did// happen during constexpr evaluation. That is, "constexpr function did not return a value" or "control fell off the end of this function" or some such.


Repository:
  rC Clang

https://reviews.llvm.org/D45643