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).
> 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ć
signature.asc
Description: PGP signature
