Hi, Arsen Arsenović <ar...@aarsen.me> writes:
>> The && should not be left of the =; if the initializer needs to span multiple
>> lines, it's usually best to wrap it in ().
>
> Okay - done.
[...]
>>> + foo::operator delete (new (123, true) foo, 123, true);
>>> + foo::operator delete[] (new (123, true) foo[123], 123, true);
>>
>> Instead of passing the result of a new-expression directly to operator
>> delete,
>> you should also call operator new directly.
>>
>>> + foo::operator delete (new (123, true) foo[123], 123, true);
>>> + // { dg-warning "Wmismatched-new-delete" "" { target *-*-* } {.-1} }
>>> + // { dg-note "returned from" "" { target *-*-* } {.-2} }
>>> + foo::operator delete[] (new (123, true) foo, 123, true);
>>
>> Likewise.
>
> Ah, good point - adjusted.
>
> Here are the changes to v1 I've made as well as the full patch below
> them. Does this look OK?
>
> --8<---------------cut here---------------start------------->8---
> modified gcc/gimple-ssa-warn-access.cc
> @@ -1776,9 +1776,9 @@ new_delete_mismatch_p (tree new_decl, tree delete_decl)
> return dc;
> };
>
> - bool mismatch = ndc && ddc
> - && new_delete_mismatch_p (*strip_dc_template (ndc),
> - *strip_dc_template (ddc));
> + bool mismatch = (ndc && ddc
> + && new_delete_mismatch_p (*strip_dc_template (ndc),
> + *strip_dc_template (ddc)));
> free (np);
> free (dp);
> return mismatch;
> modified gcc/testsuite/g++.dg/warn/Wmismatched-new-delete-9.C
> @@ -35,13 +35,13 @@ f ()
> // { dg-warning "Wmismatched-new-delete" "" { target *-*-* } {.-1} }
> // { dg-note "returned from" "" { target *-*-* } {.-2} }
>
> - foo::operator delete (new (123, true) foo, 123, true);
> - foo::operator delete[] (new (123, true) foo[123], 123, true);
> + foo::operator delete (foo::operator new (1, 123, true), 123, true);
> + foo::operator delete[] (foo::operator new[] (123, 123, true), 123, true);
>
> - foo::operator delete (new (123, true) foo[123], 123, true);
> + foo::operator delete (foo::operator new[] (123, 123, true), 123, true);
> // { dg-warning "Wmismatched-new-delete" "" { target *-*-* } {.-1} }
> // { dg-note "returned from" "" { target *-*-* } {.-2} }
> - foo::operator delete[] (new (123, true) foo, 123, true);
> + foo::operator delete[] (foo::operator new (1, 123, true), 123, true);
> // { dg-warning "Wmismatched-new-delete" "" { target *-*-* } {.-1} }
> // { dg-note "returned from" "" { target *-*-* } {.-2} }
> }
> --8<---------------cut here---------------end--------------->8---
Gentle ping on this adjustment to the patch (full reproduction of the
adjusted patch is below).
TIA, have a lovely evening.
> ---------- >8 ----------
> Template parameters on a member operator new cannot affect its member
> status nor whether it is a singleton or array operator new, hence, we
> can ignore it for purposes of matching. Similar logic applies to the
> placement operator delete.
>
> In the PR (and a lot of idiomatic coroutine code generally), operator
> new is templated in order to be able to inspect (some of) the arguments
> passed to the coroutine, to make allocation-related decisions. However,
> the coroutine implementation will not call a placement delete form, so
> it cannot get templated. As a result, when demangling, we have an extra
> template DEMANGLE_COMPONENT_TEMPLATE around the actual operator new, but
> not operator delete. This terminates new_delete_mismatch_p early.
>
> PR middle-end/109224 - Wmismatched-new-delete false positive with a
> templated operator new (common with coroutines)
>
> gcc/ChangeLog:
>
> * gimple-ssa-warn-access.cc (new_delete_mismatch_p): Strip
> DEMANGLE_COMPONENT_TEMPLATE from the operator new and operator
> after demangling.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/warn/Wmismatched-new-delete-9.C: New test.
> ---
> gcc/gimple-ssa-warn-access.cc | 18 ++++++-
> .../g++.dg/warn/Wmismatched-new-delete-9.C | 47 +++++++++++++++++++
> gcc/testsuite/g++.dg/warn/pr109224.C | 25 ++++++++++
> 3 files changed, 89 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/g++.dg/warn/Wmismatched-new-delete-9.C
> create mode 100644 gcc/testsuite/g++.dg/warn/pr109224.C
>
> diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc
> index 61f9f0f3d310..fcce63ee332d 100644
> --- a/gcc/gimple-ssa-warn-access.cc
> +++ b/gcc/gimple-ssa-warn-access.cc
> @@ -1762,7 +1762,23 @@ new_delete_mismatch_p (tree new_decl, tree delete_decl)
> void *np = NULL, *dp = NULL;
> demangle_component *ndc = cplus_demangle_v3_components (new_str, 0, &np);
> demangle_component *ddc = cplus_demangle_v3_components (del_str, 0, &dp);
> - bool mismatch = ndc && ddc && new_delete_mismatch_p (*ndc, *ddc);
> +
> + /* Sometimes, notably quite often with coroutines, 'operator new' is
> + templated. However, template arguments can't change whether a given
> + new/delete is a singleton or array one, nor what it is a member of, so
> + the template arguments can be safely ignored for the purposes of
> checking
> + for mismatches. */
> +
> + auto strip_dc_template = [] (demangle_component* dc)
> + {
> + if (dc->type == DEMANGLE_COMPONENT_TEMPLATE)
> + dc = dc->u.s_binary.left;
> + return dc;
> + };
> +
> + bool mismatch = (ndc && ddc
> + && new_delete_mismatch_p (*strip_dc_template (ndc),
> + *strip_dc_template (ddc)));
> free (np);
> free (dp);
> return mismatch;
> diff --git a/gcc/testsuite/g++.dg/warn/Wmismatched-new-delete-9.C
> b/gcc/testsuite/g++.dg/warn/Wmismatched-new-delete-9.C
> new file mode 100644
> index 000000000000..d431f4049e87
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/Wmismatched-new-delete-9.C
> @@ -0,0 +1,47 @@
> +/* { dg-do compile { target c++11 } } */
> +/* { dg-additional-options "-Wmismatched-new-delete" } */
> +/* PR middle-end/109224 */
> +/* Verify that we consider templated operator new matching with its operator
> + delete. */
> +
> +#include <new>
> +
> +struct foo
> +{
> + template<typename... Args>
> + void* operator new (std::size_t sz, Args&&...);
> + template<typename... Args>
> + void* operator new[] (std::size_t sz, Args&&...);
> +
> + void operator delete (void* x);
> + void operator delete[] (void* x);
> +
> + template<typename... Args>
> + void operator delete (void* x, Args&&...);
> + template<typename... Args>
> + void operator delete[] (void* x, Args&&...);
> +};
> +
> +void
> +f ()
> +{
> + delete (new (123, true) foo);
> + delete[] (new (123, true) foo[123]);
> +
> + delete (new (123, true) foo[123]);
> + // { dg-warning "Wmismatched-new-delete" "" { target *-*-* } {.-1} }
> + // { dg-note "returned from" "" { target *-*-* } {.-2} }
> + delete[] (new (123, true) foo);
> + // { dg-warning "Wmismatched-new-delete" "" { target *-*-* } {.-1} }
> + // { dg-note "returned from" "" { target *-*-* } {.-2} }
> +
> + foo::operator delete (foo::operator new (1, 123, true), 123, true);
> + foo::operator delete[] (foo::operator new[] (123, 123, true), 123, true);
> +
> + foo::operator delete (foo::operator new[] (123, 123, true), 123, true);
> + // { dg-warning "Wmismatched-new-delete" "" { target *-*-* } {.-1} }
> + // { dg-note "returned from" "" { target *-*-* } {.-2} }
> + foo::operator delete[] (foo::operator new (1, 123, true), 123, true);
> + // { dg-warning "Wmismatched-new-delete" "" { target *-*-* } {.-1} }
> + // { dg-note "returned from" "" { target *-*-* } {.-2} }
> +}
> diff --git a/gcc/testsuite/g++.dg/warn/pr109224.C
> b/gcc/testsuite/g++.dg/warn/pr109224.C
> new file mode 100644
> index 000000000000..4b6102226ffc
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/pr109224.C
> @@ -0,0 +1,25 @@
> +// { dg-do compile { target c++20 } }
> +#include <coroutine>
> +
> +struct Task {
> + struct promise_type {
> + std::suspend_never initial_suspend() { return {}; }
> + std::suspend_never final_suspend() noexcept { return {}; }
> + void unhandled_exception() { throw; }
> + Task get_return_object() { return {}; }
> + void return_void() {}
> +
> + template<class I>
> + void* operator new(std::size_t sz, I);
> +
> + void operator delete(void* ptr, std::size_t);
> + };
> +};
> +
> +Task f(int) {
> + co_return;
> +}
> +
> +int main() {
> + f(42);
> +}
--
Arsen Arsenović
signature.asc
Description: PGP signature
