> On 9 Aug 2024, at 11:03, Arsen Arsenović <ar...@aarsen.me> wrote:
>
> Jason Merrill <ja...@redhat.com> writes:
>
>> On 8/8/24 4:29 PM, Arsen Arsenović wrote:
>>> Tested on x86_64-pc-linux-gnu. I have blinking tsan test results again,
>>> but I think they're bogus (I'll re-test on physical hardware before
>>> pushing if needed).
>>> I'm somewhat curious of we should do a similar change WRT RETURN_EXPRs
>>> in the C FE (currently, the C FE uses the operand location for its
>>> RETURN_EXPR locations, or the return kw if missing, which I suspect is
>>> why malloc-CWE-401-example.c fails in C today, which appears confirmed
>>> by -Wsystem-headers de-suppressing it).
>>
>> Sounds plausible, but you'd have to ask the C maintainers.
>>
>>> It also might be worth splitting this patch into two (change RETURN_EXPR
>>> location, change diagnostic in coroutines.cc), now that I think about
>>> it. I'll do that before pushing or sending a v3.
>>
>> Sure.
>>
>>> One thing to consider is the argument to finish_co_return_stmt. It is
>>> called kw, but I changed its location to be stmt_loc. It is down the
>>> line passed to coro_promise_type_found_p which saves it as
>>> first_coro_keyword. I was thinking to maybe pass both the full
>>> statement location (to use in the built expr) and kw location (to use
>>> for first_coro_keyword). Iain, what do you think of that?
>>
>> That seems unnecessary to me, I don't see any uses of first_coro_keyword that
>> need it to be just the keyword; I'd rather change the name to
>> first_coro_expr.
>
> Hmm, fair. I'll let Iain weigh in just in case he had some plans with
> the field and do that if not (but it seems that way to me also).
Noting the first coroutine expression is fine with me - the main thing is to
point
the user to an explanation for the diagnostic,
Iain
>
>> Please also pass the new loc argument to finish_return_stmt in tsubst_stmt,
>> and
>> check the printed location in a template.
>
> Seems that the tsubst_stmt setup that changes 'input_location' made it
> work as-is, but I can make the argument explicit there if you prefer.
>
> test.cc:43:3: note: loc of return in tsubst_stmt::case RETURN_EXPR
> 43 | { return x; }
> | ^~~~~~~~
> --
> Arsen Arsenović
