On 10/30/24 12:34 PM, Simon Martin wrote:
Hi Jason,
On 22 Oct 2024, at 17:07, Jason Merrill wrote:
On 10/17/24 10:30 AM, Simon Martin wrote:
Hi,
The issue reported in PR117129 is pretty similar to the one in
PR117099,
so here’s an updated version of the patch that fixes both issues,
by
ensuring that finish_return_expr sets current_function_return_value
to
error_mark_node in case of error with said return value.
permerror (input_location, "return-statement with no value, in "
"function returning %qT", valtype);
- /* Remember that this function did return. */
+ /* Remember that this function returns an error. */
current_function_returns_value = 1;
+ retval = error_mark_node;
We shouldn't do this if -fpermissive made the permerror into a
warning; let's just set current_function_return_value.
Right, thanks for calling this out. The updated patch addresses this,
and also adds an execution test case that was regressing with my initial
version. Note that it relies on undefined behaviour, so I’m curious
whether I should include it or not?
I think let's include it, but add a comment that the return is UB in
case it breaks later.
@@ -11236,8 +11236,9 @@ check_return_expr (tree retval, bool *no_warning, bool
*dangling)
if (functype != error_mark_node)
permerror (input_location, "return-statement with no value, in "
"function returning %qT", valtype);
- /* Remember that this function did return. */
+ /* Remember that this function returns an error. */
...
- /* If the conversion failed, treat this just like `return;'. */
+ /* If the conversion failed, treat this just like
+ `return <ERROR>;'. */
Let's leave these comments alone, but comment on the
current_function_return_value assignments that we are suppressing NRV.
OK with those comment adjustments.
Jason
