Jason Merrill <ja...@redhat.com> writes: > On 8/28/24 3:59 PM, Arsen Arsenović wrote: >> This patch fixes a gcc-15 regression (PR116502) and an inelegance in my >> earlier patch related to converting CO_AWAIT_EXPRs to void. >> This wasn't noticed anywhere AFAICT, but it was a bit unfortunate. >> The other two patches fix an actual regression as well as provide >> handling for converting INDIRECT_REFs wrapping CO_AWAIT_EXPRs (which I >> noticed was lacking while triaging the regression). >> WRT INDIRECT_REFs, I've considered making convert_to_void recurse on the >> INDIRECT_REF case, so that special handling doesn't need to be added >> when some expression might end up getting wrapped in a INDIRECT_REF. Is >> there a reason why we don't do that? > > Primarily because that isn't what the language says. And for instance if I > have > > int *volatile p; > *p; > > I think just recursing will wrongly warn about not accessing the value of p.
Hmm, with the change in the OP I didn't get that warning, but that
reason is compelling enough for me.
> Better, I think, to teach maybe_warn_nodiscard to look through CO_AWAIT_EXPR.
The reason I didn't originally extend maybe_warn_nodiscard to also cover
CO_AWAIT_EXPR is because the await-resume of a CO_AWAIT_EXPR sometimes
isn't a (possibly wrapped) CALL_EXPR, and might contain a COMPOUND_EXPR,
for instance, when it is the initial suspend await (see 'if
(initial_await != error_mark_node)' in wrap_original_function_body), so
I wanted to avoid duplicating that logic. I could extend
maybe_warn_nodiscard to handle CO_AWAIT_EXPRs if that's preferred,
though.
>> diff --git a/gcc/cp/cvt.cc b/gcc/cp/cvt.cc
>> index 7b4bd8a9dc44..b6b35ef9cacf 100644
>> --- a/gcc/cp/cvt.cc
>> +++ b/gcc/cp/cvt.cc
>> @@ -1505,8 +1505,8 @@ convert_to_void (tree expr, impl_conv_void implicit,
>> tsubst_flags_t complain)
>> case CO_AWAIT_EXPR:
>> {
>> auto awr = co_await_get_resume_call (expr);
>> - if (awr && *awr)
>> - *awr = convert_to_void (*awr, implicit, complain);
>> + if (awr)
>> + awr = convert_to_void (awr, implicit, complain);
>
> After this change you don't need the assignment at all, it's a dead store.
Ah, yeah, whoops. Seems that I removed it later (when extracting this
into its own function) but never amended the original. Will fix.
--
Arsen Arsenović
signature.asc
Description: PGP signature
