| Message ID | 7e951363-a1a6-ff72-ae93-ba08ab796616@oracle.com |
|---|---|
| State | New |
| Headers | show |
| Series | [C++] PR 88969 ("[9 Regression] ICE in build_op_delete_call, at cp/call.c:6509") | expand |
On 1/24/19 2:53 PM, Paolo Carlini wrote: > Hi, > > as far as I can see this ICE on invalid points to a substantive, if > minor, weakness of our implementation of the destroying operator delete > facility: we aren't implementing the bits, per 7.6.2.5/(10.1), about > destroying operator delete having precedence over any other operator > delete. Thus the below, which is the most straightforward implementation > I have been able to figure out given the current infrastructure. Tested > x86_64-linux. OK, thanks. Jason
Hi, On 24/01/19 23:21, Jason Merrill wrote: > On 1/24/19 2:53 PM, Paolo Carlini wrote: >> Hi, >> >> as far as I can see this ICE on invalid points to a substantive, if >> minor, weakness of our implementation of the destroying operator >> delete facility: we aren't implementing the bits, per 7.6.2.5/(10.1), >> about destroying operator delete having precedence over any other >> operator delete. Thus the below, which is the most straightforward >> implementation I have been able to figure out given the current >> infrastructure. Tested x86_64-linux. > > OK, thanks. Thanks you. Yesterday I didn't notice that the bug report includes another testcase, for an unrelated buglet: if the destroying operator delete is wrongly specified as not-taking a pointer to the class type as first argument, TYPE_POINTER_TO may not be set yet in coerce_delete_type and we crash later on. Thus the below, which changes it to build_pointer_type. Thanks, Paolo. ////////////////////////// /cp 2019-01-25 Paolo Carlini <paolo.carlini@oracle.com> PR c++/88969 * call.c (build_op_delete_call): Implement 7.6.2.5/(10.1). * decl2.c (coerce_delete_type): Use build_pointer_type instead of TYPE_POINTER_TO. /testsuite 2019-01-25 Paolo Carlini <paolo.carlini@oracle.com> PR c++/88969 * g++.dg/cpp2a/destroying-delete2.C: New. * g++.dg/cpp2a/destroying-delete3.C: Likewise. Index: cp/call.c =================================================================== --- cp/call.c (revision 268257) +++ cp/call.c (working copy) @@ -6461,6 +6461,19 @@ build_op_delete_call (enum tree_code code, tree ad continue; } + /* -- If any of the deallocation functions is a destroying + operator delete, all deallocation functions that are not + destroying operator deletes are eliminated from further + consideration. */ + bool fn_destroying = destroying_delete_p (fn); + bool elt_destroying = destroying_delete_p (elt); + if (elt_destroying != fn_destroying) + { + if (elt_destroying) + fn = elt; + continue; + } + /* -- If the type has new-extended alignment, a function with a parameter of type std::align_val_t is preferred; otherwise a function without such a parameter is preferred. If exactly one Index: cp/decl2.c =================================================================== --- cp/decl2.c (revision 268257) +++ cp/decl2.c (working copy) @@ -1757,9 +1757,9 @@ coerce_delete_type (tree decl, location_t loc) if (destroying_delete_p (decl)) { if (DECL_CLASS_SCOPE_P (decl)) - /* If the function is a destroying operator delete declared in class type - C, the type of its first parameter shall be C*. */ - ptrtype = TYPE_POINTER_TO (DECL_CONTEXT (decl)); + /* If the function is a destroying operator delete declared in class + type C, the type of its first parameter shall be C*. */ + ptrtype = build_pointer_type (DECL_CONTEXT (decl)); else /* A destroying operator delete shall be a class member function named operator delete. */ Index: testsuite/g++.dg/cpp2a/destroying-delete2.C =================================================================== --- testsuite/g++.dg/cpp2a/destroying-delete2.C (nonexistent) +++ testsuite/g++.dg/cpp2a/destroying-delete2.C (working copy) @@ -0,0 +1,20 @@ +// PR c++/88969 +// { dg-do compile { target c++2a } } + +#include <new> + +namespace delete_selection_d { + struct B { + void operator delete(void*) = delete; + void operator delete(B *, std::destroying_delete_t) = delete; // { dg-message "declared here" } + }; + void delete_B(B *b) { delete b; } // { dg-error "use of deleted function" } +} + +namespace delete_selection_r { + struct B { + void operator delete(B *, std::destroying_delete_t) = delete; // { dg-message "declared here" } + void operator delete(void*) = delete; + }; + void delete_B(B *b) { delete b; } // { dg-error "use of deleted function" } +} Index: testsuite/g++.dg/cpp2a/destroying-delete3.C =================================================================== --- testsuite/g++.dg/cpp2a/destroying-delete3.C (nonexistent) +++ testsuite/g++.dg/cpp2a/destroying-delete3.C (working copy) @@ -0,0 +1,8 @@ +// PR c++/88969 +// { dg-do compile { target c++2a } } + +#include <new> + +struct B { + void operator delete(void*, std::destroying_delete_t); // { dg-error ".operator delete. takes type .B*." } +};
On 1/25/19 6:20 AM, Paolo Carlini wrote: > Hi, > > On 24/01/19 23:21, Jason Merrill wrote: >> On 1/24/19 2:53 PM, Paolo Carlini wrote: >>> Hi, >>> >>> as far as I can see this ICE on invalid points to a substantive, if >>> minor, weakness of our implementation of the destroying operator >>> delete facility: we aren't implementing the bits, per 7.6.2.5/(10.1), >>> about destroying operator delete having precedence over any other >>> operator delete. Thus the below, which is the most straightforward >>> implementation I have been able to figure out given the current >>> infrastructure. Tested x86_64-linux. >> >> OK, thanks. > > Thanks you. > > Yesterday I didn't notice that the bug report includes another testcase, > for an unrelated buglet: if the destroying operator delete is wrongly > specified as not-taking a pointer to the class type as first argument, > TYPE_POINTER_TO may not be set yet in coerce_delete_type and we crash > later on. Thus the below, which changes it to build_pointer_type. Also OK. :) Jason
Index: cp/call.c =================================================================== --- cp/call.c (revision 268234) +++ cp/call.c (working copy) @@ -6461,6 +6461,19 @@ build_op_delete_call (enum tree_code code, tree ad continue; } + /* -- If any of the deallocation functions is a destroying + operator delete, all deallocation functions that are not + destroying operator deletes are eliminated from further + consideration. */ + bool fn_destroying = destroying_delete_p (fn); + bool elt_destroying = destroying_delete_p (elt); + if (elt_destroying != fn_destroying) + { + if (elt_destroying) + fn = elt; + continue; + } + /* -- If the type has new-extended alignment, a function with a parameter of type std::align_val_t is preferred; otherwise a function without such a parameter is preferred. If exactly one Index: testsuite/g++.dg/cpp2a/destroying-delete2.C =================================================================== --- testsuite/g++.dg/cpp2a/destroying-delete2.C (nonexistent) +++ testsuite/g++.dg/cpp2a/destroying-delete2.C (working copy) @@ -0,0 +1,20 @@ +// PR c++/88969 +// { dg-do compile { target c++2a } } + +#include <new> + +namespace delete_selection_d { + struct B { + void operator delete(void*) = delete; + void operator delete(B *, std::destroying_delete_t) = delete; // { dg-message "declared here" } + }; + void delete_B(B *b) { delete b; } // { dg-error "use of deleted function" } +} + +namespace delete_selection_r { + struct B { + void operator delete(B *, std::destroying_delete_t) = delete; // { dg-message "declared here" } + void operator delete(void*) = delete; + }; + void delete_B(B *b) { delete b; } // { dg-error "use of deleted function" } +}