Re: [PATCH] reject scalar array initialization with nullptr [PR94510]
On Wed, 15 Apr 2020, Patrick Palka wrote: > On Wed, 15 Apr 2020, Martin Sebor via Gcc-patches wrote: > > On 4/13/20 8:43 PM, Jason Merrill wrote: > > > On 4/12/20 5:49 PM, Martin Sebor wrote: > > > > On 4/10/20 8:52 AM, Jason Merrill wrote: > > > > > On 4/9/20 4:23 PM, Martin Sebor wrote: > > > > > > On 4/9/20 1:32 PM, Jason Merrill wrote: > > > > > > > On 4/9/20 3:24 PM, Martin Sebor wrote: > > > > > > > > On 4/9/20 1:03 PM, Jason Merrill wrote: > > > > > > > > > On 4/8/20 1:23 PM, Martin Sebor wrote: > > > > > > > > > > On 4/7/20 3:36 PM, Marek Polacek wrote: > > > > > > > > > > > On Tue, Apr 07, 2020 at 02:46:52PM -0600, Martin Sebor > > > > > > > > > > > wrote: > > > > > > > > > > > > On 4/7/20 1:50 PM, Marek Polacek wrote: > > > > > > > > > > > > > On Tue, Apr 07, 2020 at 12:50:48PM -0600, Martin Sebor > > > > > > > > > > > > > via Gcc-patches wrote: > > > > > > > > > > > > > > Among the numerous regressions introduced by the > > > > > > > > > > > > > > change committed > > > > > > > > > > > > > > to GCC 9 to allow string literals as template > > > > > > > > > > > > > > arguments is a failure > > > > > > > > > > > > > > to recognize the C++ nullptr and GCC's __null > > > > > > > > > > > > > > constants as pointers. > > > > > > > > > > > > > > For one, I didn't realize that nullptr, being a null > > > > > > > > > > > > > > pointer constant, > > > > > > > > > > > > > > doesn't have a pointer type, and two, I didn't think > > > > > > > > > > > > > > of __null (which > > > > > > > > > > > > > > is a special integer constant that NULL sometimes > > > > > > > > > > > > > > expands to). > > > > > > > > > > > > > > > > > > > > > > > > > > > > The attached patch adjusts the special handling of > > > > > > > > > > > > > > trailing zero > > > > > > > > > > > > > > initializers in reshape_init_array_1 to recognize > > > > > > > > > > > > > > both > > > > > > > > > > > > > > kinds of > > > > > > > > > > > > > > constants and avoid treating them as zeros of the > > > > > > > > > > > > > > array integer > > > > > > > > > > > > > > element type. This restores the expected > > > > > > > > > > > > > > diagnostics > > > > > > > > > > > > > > when either > > > > > > > > > > > > > > constant is used in the initializer list. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Martin > > > > > > > > > > > > > > > > > > > > > > > > > > > PR c++/94510 - nullptr_t implicitly cast to zero > > > > > > > > > > > > > > twice > > > > > > > > > > > > > > in std::array > > > > > > > > > > > > > > > > > > > > > > > > > > > > gcc/cp/ChangeLog: > > > > > > > > > > > > > > > > > > > > > > > > > > > > PR c++/94510 > > > > > > > > > > > > > > * decl.c (reshape_init_array_1): Exclude > > > > > > > > > > > > > > mismatches with all kinds > > > > > > > > > > > > > > of pointers. > > > > > > > > > > > > > > > > > > > > > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > > > > > > > > > > > > > > > > > > > > > PR c++/94510 > > > > > > > > > > > > > > * g++.dg/init/array57.C: New test. > > > > > > > > > > > > > > * g++.dg/init/array58.C: New test. > > > > > > > > > > > > > > > > > > > > > > > > > > > > diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c > > > > > > > > > > > > > > index a127734af69..692c8ed73f4 100644 > > > > > > > > > > > > > > --- a/gcc/cp/decl.c > > > > > > > > > > > > > > +++ b/gcc/cp/decl.c > > > > > > > > > > > > > > @@ -6041,9 +6041,14 @@ reshape_init_array_1 (tree > > > > > > > > > > > > > > elt_type, tree max_index, reshape_iter *d, > > > > > > > > > > > > > > TREE_CONSTANT (new_init) = false; > > > > > > > > > > > > > > /* Pointers initialized to strings must be > > > > > > > > > > > > > > treated as non-zero > > > > > > > > > > > > > > - even if the string is empty. */ > > > > > > > > > > > > > > + even if the string is empty. Handle all kinds > > > > > > > > > > > > > > of pointers, > > > > > > > > > > > > > > + including std::nullptr and GCC's __nullptr, > > > > > > > > > > > > > > neither of which > > > > > > > > > > > > > > + has a pointer type. */ > > > > > > > > > > > > > > tree init_type = TREE_TYPE (elt_init); > > > > > > > > > > > > > > - if (POINTER_TYPE_P (elt_type) != > > > > > > > > > > > > > > POINTER_TYPE_P > > > > > > > > > > > > > > (init_type) > > > > > > > > > > > > > > + bool init_is_ptr = (POINTER_TYPE_P > > > > > > > > > > > > > > (init_type) > > > > > > > > > > > > > > + || NULLPTR_TYPE_P (init_type) > > > > > > > > > > > > > > + || null_node_p (elt_init)); > > > > > > > > > > > > > > + if (POINTER_TYPE_P (elt_type) != init_is_ptr > > > > > > > > > > > > > > || !type_initializer_zero_p (elt_type, > > > > > > > > > > > > > > elt_init)) > > > > > > > > > > > > > > last_nonzero = index; > > > > > > > > > > > > > > > > > > > > > > > > > > It looks like this still won't handle e.g. pointers to > > > > > > > > > > > > > member functions, > > > > > > > > > > > > >
Re: [PATCH] reject scalar array initialization with nullptr [PR94510]
On 4/21/20 2:33 PM, Jason Merrill wrote: On 4/17/20 5:18 PM, Martin Sebor wrote: On 4/17/20 12:19 AM, Jason Merrill wrote: On 4/15/20 1:30 PM, Martin Sebor wrote: On 4/13/20 8:43 PM, Jason Merrill wrote: On 4/12/20 5:49 PM, Martin Sebor wrote: On 4/10/20 8:52 AM, Jason Merrill wrote: On 4/9/20 4:23 PM, Martin Sebor wrote: On 4/9/20 1:32 PM, Jason Merrill wrote: On 4/9/20 3:24 PM, Martin Sebor wrote: On 4/9/20 1:03 PM, Jason Merrill wrote: On 4/8/20 1:23 PM, Martin Sebor wrote: On 4/7/20 3:36 PM, Marek Polacek wrote: On Tue, Apr 07, 2020 at 02:46:52PM -0600, Martin Sebor wrote: On 4/7/20 1:50 PM, Marek Polacek wrote: On Tue, Apr 07, 2020 at 12:50:48PM -0600, Martin Sebor via Gcc-patches wrote: Among the numerous regressions introduced by the change committed to GCC 9 to allow string literals as template arguments is a failure to recognize the C++ nullptr and GCC's __null constants as pointers. For one, I didn't realize that nullptr, being a null pointer constant, doesn't have a pointer type, and two, I didn't think of __null (which is a special integer constant that NULL sometimes expands to). The attached patch adjusts the special handling of trailing zero initializers in reshape_init_array_1 to recognize both kinds of constants and avoid treating them as zeros of the array integer element type. This restores the expected diagnostics when either constant is used in the initializer list. Martin PR c++/94510 - nullptr_t implicitly cast to zero twice in std::array gcc/cp/ChangeLog: PR c++/94510 * decl.c (reshape_init_array_1): Exclude mismatches with all kinds of pointers. gcc/testsuite/ChangeLog: PR c++/94510 * g++.dg/init/array57.C: New test. * g++.dg/init/array58.C: New test. diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index a127734af69..692c8ed73f4 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -6041,9 +6041,14 @@ reshape_init_array_1 (tree elt_type, tree max_index, reshape_iter *d, TREE_CONSTANT (new_init) = false; /* Pointers initialized to strings must be treated as non-zero - even if the string is empty. */ + even if the string is empty. Handle all kinds of pointers, + including std::nullptr and GCC's __nullptr, neither of which + has a pointer type. */ tree init_type = TREE_TYPE (elt_init); - if (POINTER_TYPE_P (elt_type) != POINTER_TYPE_P (init_type) + bool init_is_ptr = (POINTER_TYPE_P (init_type) + || NULLPTR_TYPE_P (init_type) + || null_node_p (elt_init)); + if (POINTER_TYPE_P (elt_type) != init_is_ptr || !type_initializer_zero_p (elt_type, elt_init)) last_nonzero = index; It looks like this still won't handle e.g. pointers to member functions, e.g. struct S { }; int arr[3] = { (void (S::*) ()) 0, 0, 0 }; would still be accepted. You could use TYPE_PTR_OR_PTRMEM_P instead of POINTER_TYPE_P to catch this case. Good catch! That doesn't fail because unlike null data member pointers which are represented as -1, member function pointers are represented as a zero. I had looked for an API that would answer the question: "is this expression a pointer?" without having to think of all the different kinds of them but all I could find was null_node_p(). Is this a rare, isolated case that having an API like that wouldn't be worth having or should I add one like in the attached update? Martin PR c++/94510 - nullptr_t implicitly cast to zero twice in std::array gcc/cp/ChangeLog: PR c++/94510 * decl.c (reshape_init_array_1): Exclude mismatches with all kinds of pointers. * gcc/cp/cp-tree.h (null_pointer_constant_p): New function. (Drop the gcc/cp/.) +/* Returns true if EXPR is a null pointer constant of any type. */ + +inline bool +null_pointer_constant_p (tree expr) +{ + STRIP_ANY_LOCATION_WRAPPER (expr); + if (expr == null_node) + return true; + tree type = TREE_TYPE (expr); + if (NULLPTR_TYPE_P (type)) + return true; + if (POINTER_TYPE_P (type)) + return integer_zerop (expr); + return null_member_pointer_value_p (expr); +} + We already have a null_ptr_cst_p so it would be sort of confusing to have this as well. But are you really interested in whether it's a null pointer, not just a pointer? The goal of the code is to detect a mismatch in "pointerness" between an initializer expression and the type of the initialized element, so it needs to know if the expression is a pointer (non-nulls pointers are detected in type_initializer_zero_p). That means testing a number of IMO unintuitive conditions: TYPE_PTR_OR_PTRMEM_P (TREE_TYPE (expr)) || NULLPTR_TYPE_P (TREE_TYPE (expr)) || null_node_p (expr) I don't know if this type of a query is common in the C++ FE but unless this is an isolated use case then besides fixing the bug I thought it would be nice to make it easier to get the test above right, or at least come close to it. Since
Re: [PATCH] reject scalar array initialization with nullptr [PR94510]
On 4/17/20 5:18 PM, Martin Sebor wrote: On 4/17/20 12:19 AM, Jason Merrill wrote: On 4/15/20 1:30 PM, Martin Sebor wrote: On 4/13/20 8:43 PM, Jason Merrill wrote: On 4/12/20 5:49 PM, Martin Sebor wrote: On 4/10/20 8:52 AM, Jason Merrill wrote: On 4/9/20 4:23 PM, Martin Sebor wrote: On 4/9/20 1:32 PM, Jason Merrill wrote: On 4/9/20 3:24 PM, Martin Sebor wrote: On 4/9/20 1:03 PM, Jason Merrill wrote: On 4/8/20 1:23 PM, Martin Sebor wrote: On 4/7/20 3:36 PM, Marek Polacek wrote: On Tue, Apr 07, 2020 at 02:46:52PM -0600, Martin Sebor wrote: On 4/7/20 1:50 PM, Marek Polacek wrote: On Tue, Apr 07, 2020 at 12:50:48PM -0600, Martin Sebor via Gcc-patches wrote: Among the numerous regressions introduced by the change committed to GCC 9 to allow string literals as template arguments is a failure to recognize the C++ nullptr and GCC's __null constants as pointers. For one, I didn't realize that nullptr, being a null pointer constant, doesn't have a pointer type, and two, I didn't think of __null (which is a special integer constant that NULL sometimes expands to). The attached patch adjusts the special handling of trailing zero initializers in reshape_init_array_1 to recognize both kinds of constants and avoid treating them as zeros of the array integer element type. This restores the expected diagnostics when either constant is used in the initializer list. Martin PR c++/94510 - nullptr_t implicitly cast to zero twice in std::array gcc/cp/ChangeLog: PR c++/94510 * decl.c (reshape_init_array_1): Exclude mismatches with all kinds of pointers. gcc/testsuite/ChangeLog: PR c++/94510 * g++.dg/init/array57.C: New test. * g++.dg/init/array58.C: New test. diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index a127734af69..692c8ed73f4 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -6041,9 +6041,14 @@ reshape_init_array_1 (tree elt_type, tree max_index, reshape_iter *d, TREE_CONSTANT (new_init) = false; /* Pointers initialized to strings must be treated as non-zero - even if the string is empty. */ + even if the string is empty. Handle all kinds of pointers, + including std::nullptr and GCC's __nullptr, neither of which + has a pointer type. */ tree init_type = TREE_TYPE (elt_init); - if (POINTER_TYPE_P (elt_type) != POINTER_TYPE_P (init_type) + bool init_is_ptr = (POINTER_TYPE_P (init_type) + || NULLPTR_TYPE_P (init_type) + || null_node_p (elt_init)); + if (POINTER_TYPE_P (elt_type) != init_is_ptr || !type_initializer_zero_p (elt_type, elt_init)) last_nonzero = index; It looks like this still won't handle e.g. pointers to member functions, e.g. struct S { }; int arr[3] = { (void (S::*) ()) 0, 0, 0 }; would still be accepted. You could use TYPE_PTR_OR_PTRMEM_P instead of POINTER_TYPE_P to catch this case. Good catch! That doesn't fail because unlike null data member pointers which are represented as -1, member function pointers are represented as a zero. I had looked for an API that would answer the question: "is this expression a pointer?" without having to think of all the different kinds of them but all I could find was null_node_p(). Is this a rare, isolated case that having an API like that wouldn't be worth having or should I add one like in the attached update? Martin PR c++/94510 - nullptr_t implicitly cast to zero twice in std::array gcc/cp/ChangeLog: PR c++/94510 * decl.c (reshape_init_array_1): Exclude mismatches with all kinds of pointers. * gcc/cp/cp-tree.h (null_pointer_constant_p): New function. (Drop the gcc/cp/.) +/* Returns true if EXPR is a null pointer constant of any type. */ + +inline bool +null_pointer_constant_p (tree expr) +{ + STRIP_ANY_LOCATION_WRAPPER (expr); + if (expr == null_node) + return true; + tree type = TREE_TYPE (expr); + if (NULLPTR_TYPE_P (type)) + return true; + if (POINTER_TYPE_P (type)) + return integer_zerop (expr); + return null_member_pointer_value_p (expr); +} + We already have a null_ptr_cst_p so it would be sort of confusing to have this as well. But are you really interested in whether it's a null pointer, not just a pointer? The goal of the code is to detect a mismatch in "pointerness" between an initializer expression and the type of the initialized element, so it needs to know if the expression is a pointer (non-nulls pointers are detected in type_initializer_zero_p). That means testing a number of IMO unintuitive conditions: TYPE_PTR_OR_PTRMEM_P (TREE_TYPE (expr)) || NULLPTR_TYPE_P (TREE_TYPE (expr)) || null_node_p (expr) I don't know if this type of a query is common in the C++ FE but unless this is an isolated use case then besides fixing the bug I thought it would be nice to make it easier to get the test above right, or at least come close to it. Since null_pointer_constant_p already exists (but isn't
Re: [PATCH] reject scalar array initialization with nullptr [PR94510]
On 17 April 2020 23:18:05 CEST, Martin Sebor via Gcc-patches wrote: + zero-initialialization for its type, taking pointers to members s/initialialization/initialization/
Re: [PATCH] reject scalar array initialization with nullptr [PR94510]
On 4/17/20 12:19 AM, Jason Merrill wrote: On 4/15/20 1:30 PM, Martin Sebor wrote: On 4/13/20 8:43 PM, Jason Merrill wrote: On 4/12/20 5:49 PM, Martin Sebor wrote: On 4/10/20 8:52 AM, Jason Merrill wrote: On 4/9/20 4:23 PM, Martin Sebor wrote: On 4/9/20 1:32 PM, Jason Merrill wrote: On 4/9/20 3:24 PM, Martin Sebor wrote: On 4/9/20 1:03 PM, Jason Merrill wrote: On 4/8/20 1:23 PM, Martin Sebor wrote: On 4/7/20 3:36 PM, Marek Polacek wrote: On Tue, Apr 07, 2020 at 02:46:52PM -0600, Martin Sebor wrote: On 4/7/20 1:50 PM, Marek Polacek wrote: On Tue, Apr 07, 2020 at 12:50:48PM -0600, Martin Sebor via Gcc-patches wrote: Among the numerous regressions introduced by the change committed to GCC 9 to allow string literals as template arguments is a failure to recognize the C++ nullptr and GCC's __null constants as pointers. For one, I didn't realize that nullptr, being a null pointer constant, doesn't have a pointer type, and two, I didn't think of __null (which is a special integer constant that NULL sometimes expands to). The attached patch adjusts the special handling of trailing zero initializers in reshape_init_array_1 to recognize both kinds of constants and avoid treating them as zeros of the array integer element type. This restores the expected diagnostics when either constant is used in the initializer list. Martin PR c++/94510 - nullptr_t implicitly cast to zero twice in std::array gcc/cp/ChangeLog: PR c++/94510 * decl.c (reshape_init_array_1): Exclude mismatches with all kinds of pointers. gcc/testsuite/ChangeLog: PR c++/94510 * g++.dg/init/array57.C: New test. * g++.dg/init/array58.C: New test. diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index a127734af69..692c8ed73f4 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -6041,9 +6041,14 @@ reshape_init_array_1 (tree elt_type, tree max_index, reshape_iter *d, TREE_CONSTANT (new_init) = false; /* Pointers initialized to strings must be treated as non-zero - even if the string is empty. */ + even if the string is empty. Handle all kinds of pointers, + including std::nullptr and GCC's __nullptr, neither of which + has a pointer type. */ tree init_type = TREE_TYPE (elt_init); - if (POINTER_TYPE_P (elt_type) != POINTER_TYPE_P (init_type) + bool init_is_ptr = (POINTER_TYPE_P (init_type) + || NULLPTR_TYPE_P (init_type) + || null_node_p (elt_init)); + if (POINTER_TYPE_P (elt_type) != init_is_ptr || !type_initializer_zero_p (elt_type, elt_init)) last_nonzero = index; It looks like this still won't handle e.g. pointers to member functions, e.g. struct S { }; int arr[3] = { (void (S::*) ()) 0, 0, 0 }; would still be accepted. You could use TYPE_PTR_OR_PTRMEM_P instead of POINTER_TYPE_P to catch this case. Good catch! That doesn't fail because unlike null data member pointers which are represented as -1, member function pointers are represented as a zero. I had looked for an API that would answer the question: "is this expression a pointer?" without having to think of all the different kinds of them but all I could find was null_node_p(). Is this a rare, isolated case that having an API like that wouldn't be worth having or should I add one like in the attached update? Martin PR c++/94510 - nullptr_t implicitly cast to zero twice in std::array gcc/cp/ChangeLog: PR c++/94510 * decl.c (reshape_init_array_1): Exclude mismatches with all kinds of pointers. * gcc/cp/cp-tree.h (null_pointer_constant_p): New function. (Drop the gcc/cp/.) +/* Returns true if EXPR is a null pointer constant of any type. */ + +inline bool +null_pointer_constant_p (tree expr) +{ + STRIP_ANY_LOCATION_WRAPPER (expr); + if (expr == null_node) + return true; + tree type = TREE_TYPE (expr); + if (NULLPTR_TYPE_P (type)) + return true; + if (POINTER_TYPE_P (type)) + return integer_zerop (expr); + return null_member_pointer_value_p (expr); +} + We already have a null_ptr_cst_p so it would be sort of confusing to have this as well. But are you really interested in whether it's a null pointer, not just a pointer? The goal of the code is to detect a mismatch in "pointerness" between an initializer expression and the type of the initialized element, so it needs to know if the expression is a pointer (non-nulls pointers are detected in type_initializer_zero_p). That means testing a number of IMO unintuitive conditions: TYPE_PTR_OR_PTRMEM_P (TREE_TYPE (expr)) || NULLPTR_TYPE_P (TREE_TYPE (expr)) || null_node_p (expr) I don't know if this type of a query is common in the C++ FE but unless this is an isolated use case then besides fixing the bug I thought it would be nice to make it easier to get the test above right, or at least come close to it. Since null_pointer_constant_p already exists (but isn't suitable here because it returns true for
Re: [PATCH] reject scalar array initialization with nullptr [PR94510]
On 4/15/20 1:30 PM, Martin Sebor wrote: On 4/13/20 8:43 PM, Jason Merrill wrote: On 4/12/20 5:49 PM, Martin Sebor wrote: On 4/10/20 8:52 AM, Jason Merrill wrote: On 4/9/20 4:23 PM, Martin Sebor wrote: On 4/9/20 1:32 PM, Jason Merrill wrote: On 4/9/20 3:24 PM, Martin Sebor wrote: On 4/9/20 1:03 PM, Jason Merrill wrote: On 4/8/20 1:23 PM, Martin Sebor wrote: On 4/7/20 3:36 PM, Marek Polacek wrote: On Tue, Apr 07, 2020 at 02:46:52PM -0600, Martin Sebor wrote: On 4/7/20 1:50 PM, Marek Polacek wrote: On Tue, Apr 07, 2020 at 12:50:48PM -0600, Martin Sebor via Gcc-patches wrote: Among the numerous regressions introduced by the change committed to GCC 9 to allow string literals as template arguments is a failure to recognize the C++ nullptr and GCC's __null constants as pointers. For one, I didn't realize that nullptr, being a null pointer constant, doesn't have a pointer type, and two, I didn't think of __null (which is a special integer constant that NULL sometimes expands to). The attached patch adjusts the special handling of trailing zero initializers in reshape_init_array_1 to recognize both kinds of constants and avoid treating them as zeros of the array integer element type. This restores the expected diagnostics when either constant is used in the initializer list. Martin PR c++/94510 - nullptr_t implicitly cast to zero twice in std::array gcc/cp/ChangeLog: PR c++/94510 * decl.c (reshape_init_array_1): Exclude mismatches with all kinds of pointers. gcc/testsuite/ChangeLog: PR c++/94510 * g++.dg/init/array57.C: New test. * g++.dg/init/array58.C: New test. diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index a127734af69..692c8ed73f4 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -6041,9 +6041,14 @@ reshape_init_array_1 (tree elt_type, tree max_index, reshape_iter *d, TREE_CONSTANT (new_init) = false; /* Pointers initialized to strings must be treated as non-zero - even if the string is empty. */ + even if the string is empty. Handle all kinds of pointers, + including std::nullptr and GCC's __nullptr, neither of which + has a pointer type. */ tree init_type = TREE_TYPE (elt_init); - if (POINTER_TYPE_P (elt_type) != POINTER_TYPE_P (init_type) + bool init_is_ptr = (POINTER_TYPE_P (init_type) + || NULLPTR_TYPE_P (init_type) + || null_node_p (elt_init)); + if (POINTER_TYPE_P (elt_type) != init_is_ptr || !type_initializer_zero_p (elt_type, elt_init)) last_nonzero = index; It looks like this still won't handle e.g. pointers to member functions, e.g. struct S { }; int arr[3] = { (void (S::*) ()) 0, 0, 0 }; would still be accepted. You could use TYPE_PTR_OR_PTRMEM_P instead of POINTER_TYPE_P to catch this case. Good catch! That doesn't fail because unlike null data member pointers which are represented as -1, member function pointers are represented as a zero. I had looked for an API that would answer the question: "is this expression a pointer?" without having to think of all the different kinds of them but all I could find was null_node_p(). Is this a rare, isolated case that having an API like that wouldn't be worth having or should I add one like in the attached update? Martin PR c++/94510 - nullptr_t implicitly cast to zero twice in std::array gcc/cp/ChangeLog: PR c++/94510 * decl.c (reshape_init_array_1): Exclude mismatches with all kinds of pointers. * gcc/cp/cp-tree.h (null_pointer_constant_p): New function. (Drop the gcc/cp/.) +/* Returns true if EXPR is a null pointer constant of any type. */ + +inline bool +null_pointer_constant_p (tree expr) +{ + STRIP_ANY_LOCATION_WRAPPER (expr); + if (expr == null_node) + return true; + tree type = TREE_TYPE (expr); + if (NULLPTR_TYPE_P (type)) + return true; + if (POINTER_TYPE_P (type)) + return integer_zerop (expr); + return null_member_pointer_value_p (expr); +} + We already have a null_ptr_cst_p so it would be sort of confusing to have this as well. But are you really interested in whether it's a null pointer, not just a pointer? The goal of the code is to detect a mismatch in "pointerness" between an initializer expression and the type of the initialized element, so it needs to know if the expression is a pointer (non-nulls pointers are detected in type_initializer_zero_p). That means testing a number of IMO unintuitive conditions: TYPE_PTR_OR_PTRMEM_P (TREE_TYPE (expr)) || NULLPTR_TYPE_P (TREE_TYPE (expr)) || null_node_p (expr) I don't know if this type of a query is common in the C++ FE but unless this is an isolated use case then besides fixing the bug I thought it would be nice to make it easier to get the test above right, or at least come close to it. Since null_pointer_constant_p already exists (but isn't suitable here because it returns true for plain literal zeros) Why is that
Re: [PATCH] reject scalar array initialization with nullptr [PR94510]
On Wed, 15 Apr 2020, Martin Sebor via Gcc-patches wrote: > On 4/13/20 8:43 PM, Jason Merrill wrote: > > On 4/12/20 5:49 PM, Martin Sebor wrote: > > > On 4/10/20 8:52 AM, Jason Merrill wrote: > > > > On 4/9/20 4:23 PM, Martin Sebor wrote: > > > > > On 4/9/20 1:32 PM, Jason Merrill wrote: > > > > > > On 4/9/20 3:24 PM, Martin Sebor wrote: > > > > > > > On 4/9/20 1:03 PM, Jason Merrill wrote: > > > > > > > > On 4/8/20 1:23 PM, Martin Sebor wrote: > > > > > > > > > On 4/7/20 3:36 PM, Marek Polacek wrote: > > > > > > > > > > On Tue, Apr 07, 2020 at 02:46:52PM -0600, Martin Sebor > > > > > > > > > > wrote: > > > > > > > > > > > On 4/7/20 1:50 PM, Marek Polacek wrote: > > > > > > > > > > > > On Tue, Apr 07, 2020 at 12:50:48PM -0600, Martin Sebor > > > > > > > > > > > > via Gcc-patches wrote: > > > > > > > > > > > > > Among the numerous regressions introduced by the > > > > > > > > > > > > > change committed > > > > > > > > > > > > > to GCC 9 to allow string literals as template > > > > > > > > > > > > > arguments is a failure > > > > > > > > > > > > > to recognize the C++ nullptr and GCC's __null > > > > > > > > > > > > > constants as pointers. > > > > > > > > > > > > > For one, I didn't realize that nullptr, being a null > > > > > > > > > > > > > pointer constant, > > > > > > > > > > > > > doesn't have a pointer type, and two, I didn't think > > > > > > > > > > > > > of __null (which > > > > > > > > > > > > > is a special integer constant that NULL sometimes > > > > > > > > > > > > > expands to). > > > > > > > > > > > > > > > > > > > > > > > > > > The attached patch adjusts the special handling of > > > > > > > > > > > > > trailing zero > > > > > > > > > > > > > initializers in reshape_init_array_1 to recognize both > > > > > > > > > > > > > kinds of > > > > > > > > > > > > > constants and avoid treating them as zeros of the > > > > > > > > > > > > > array integer > > > > > > > > > > > > > element type. This restores the expected diagnostics > > > > > > > > > > > > > when either > > > > > > > > > > > > > constant is used in the initializer list. > > > > > > > > > > > > > > > > > > > > > > > > > > Martin > > > > > > > > > > > > > > > > > > > > > > > > > PR c++/94510 - nullptr_t implicitly cast to zero twice > > > > > > > > > > > > > in std::array > > > > > > > > > > > > > > > > > > > > > > > > > > gcc/cp/ChangeLog: > > > > > > > > > > > > > > > > > > > > > > > > > > PR c++/94510 > > > > > > > > > > > > > * decl.c (reshape_init_array_1): Exclude > > > > > > > > > > > > > mismatches with all kinds > > > > > > > > > > > > > of pointers. > > > > > > > > > > > > > > > > > > > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > > > > > > > > > > > > > > > > > > > PR c++/94510 > > > > > > > > > > > > > * g++.dg/init/array57.C: New test. > > > > > > > > > > > > > * g++.dg/init/array58.C: New test. > > > > > > > > > > > > > > > > > > > > > > > > > > diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c > > > > > > > > > > > > > index a127734af69..692c8ed73f4 100644 > > > > > > > > > > > > > --- a/gcc/cp/decl.c > > > > > > > > > > > > > +++ b/gcc/cp/decl.c > > > > > > > > > > > > > @@ -6041,9 +6041,14 @@ reshape_init_array_1 (tree > > > > > > > > > > > > > elt_type, tree max_index, reshape_iter *d, > > > > > > > > > > > > > TREE_CONSTANT (new_init) = false; > > > > > > > > > > > > > /* Pointers initialized to strings must be > > > > > > > > > > > > > treated as non-zero > > > > > > > > > > > > > - even if the string is empty. */ > > > > > > > > > > > > > + even if the string is empty. Handle all kinds > > > > > > > > > > > > > of pointers, > > > > > > > > > > > > > + including std::nullptr and GCC's __nullptr, > > > > > > > > > > > > > neither of which > > > > > > > > > > > > > + has a pointer type. */ > > > > > > > > > > > > > tree init_type = TREE_TYPE (elt_init); > > > > > > > > > > > > > - if (POINTER_TYPE_P (elt_type) != POINTER_TYPE_P > > > > > > > > > > > > > (init_type) > > > > > > > > > > > > > + bool init_is_ptr = (POINTER_TYPE_P (init_type) > > > > > > > > > > > > > + || NULLPTR_TYPE_P (init_type) > > > > > > > > > > > > > + || null_node_p (elt_init)); > > > > > > > > > > > > > + if (POINTER_TYPE_P (elt_type) != init_is_ptr > > > > > > > > > > > > > || !type_initializer_zero_p (elt_type, > > > > > > > > > > > > > elt_init)) > > > > > > > > > > > > > last_nonzero = index; > > > > > > > > > > > > > > > > > > > > > > > > It looks like this still won't handle e.g. pointers to > > > > > > > > > > > > member functions, > > > > > > > > > > > > e.g. > > > > > > > > > > > > > > > > > > > > > > > > struct S { }; > > > > > > > > > > > > int arr[3] = { (void (S::*) ()) 0, 0, 0 }; > > > > > > > > > > > > > > > > > > > > > > > > would still be accepted. You could use > > > > > > > > > > > > TYPE_PTR_OR_PTRMEM_P instead of > > > > > > > > > > > > POINTER_TYPE_P to catch this case. > > > > > > > > >
Re: [PATCH] reject scalar array initialization with nullptr [PR94510]
On 4/13/20 8:43 PM, Jason Merrill wrote: On 4/12/20 5:49 PM, Martin Sebor wrote: On 4/10/20 8:52 AM, Jason Merrill wrote: On 4/9/20 4:23 PM, Martin Sebor wrote: On 4/9/20 1:32 PM, Jason Merrill wrote: On 4/9/20 3:24 PM, Martin Sebor wrote: On 4/9/20 1:03 PM, Jason Merrill wrote: On 4/8/20 1:23 PM, Martin Sebor wrote: On 4/7/20 3:36 PM, Marek Polacek wrote: On Tue, Apr 07, 2020 at 02:46:52PM -0600, Martin Sebor wrote: On 4/7/20 1:50 PM, Marek Polacek wrote: On Tue, Apr 07, 2020 at 12:50:48PM -0600, Martin Sebor via Gcc-patches wrote: Among the numerous regressions introduced by the change committed to GCC 9 to allow string literals as template arguments is a failure to recognize the C++ nullptr and GCC's __null constants as pointers. For one, I didn't realize that nullptr, being a null pointer constant, doesn't have a pointer type, and two, I didn't think of __null (which is a special integer constant that NULL sometimes expands to). The attached patch adjusts the special handling of trailing zero initializers in reshape_init_array_1 to recognize both kinds of constants and avoid treating them as zeros of the array integer element type. This restores the expected diagnostics when either constant is used in the initializer list. Martin PR c++/94510 - nullptr_t implicitly cast to zero twice in std::array gcc/cp/ChangeLog: PR c++/94510 * decl.c (reshape_init_array_1): Exclude mismatches with all kinds of pointers. gcc/testsuite/ChangeLog: PR c++/94510 * g++.dg/init/array57.C: New test. * g++.dg/init/array58.C: New test. diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index a127734af69..692c8ed73f4 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -6041,9 +6041,14 @@ reshape_init_array_1 (tree elt_type, tree max_index, reshape_iter *d, TREE_CONSTANT (new_init) = false; /* Pointers initialized to strings must be treated as non-zero - even if the string is empty. */ + even if the string is empty. Handle all kinds of pointers, + including std::nullptr and GCC's __nullptr, neither of which + has a pointer type. */ tree init_type = TREE_TYPE (elt_init); - if (POINTER_TYPE_P (elt_type) != POINTER_TYPE_P (init_type) + bool init_is_ptr = (POINTER_TYPE_P (init_type) + || NULLPTR_TYPE_P (init_type) + || null_node_p (elt_init)); + if (POINTER_TYPE_P (elt_type) != init_is_ptr || !type_initializer_zero_p (elt_type, elt_init)) last_nonzero = index; It looks like this still won't handle e.g. pointers to member functions, e.g. struct S { }; int arr[3] = { (void (S::*) ()) 0, 0, 0 }; would still be accepted. You could use TYPE_PTR_OR_PTRMEM_P instead of POINTER_TYPE_P to catch this case. Good catch! That doesn't fail because unlike null data member pointers which are represented as -1, member function pointers are represented as a zero. I had looked for an API that would answer the question: "is this expression a pointer?" without having to think of all the different kinds of them but all I could find was null_node_p(). Is this a rare, isolated case that having an API like that wouldn't be worth having or should I add one like in the attached update? Martin PR c++/94510 - nullptr_t implicitly cast to zero twice in std::array gcc/cp/ChangeLog: PR c++/94510 * decl.c (reshape_init_array_1): Exclude mismatches with all kinds of pointers. * gcc/cp/cp-tree.h (null_pointer_constant_p): New function. (Drop the gcc/cp/.) +/* Returns true if EXPR is a null pointer constant of any type. */ + +inline bool +null_pointer_constant_p (tree expr) +{ + STRIP_ANY_LOCATION_WRAPPER (expr); + if (expr == null_node) + return true; + tree type = TREE_TYPE (expr); + if (NULLPTR_TYPE_P (type)) + return true; + if (POINTER_TYPE_P (type)) + return integer_zerop (expr); + return null_member_pointer_value_p (expr); +} + We already have a null_ptr_cst_p so it would be sort of confusing to have this as well. But are you really interested in whether it's a null pointer, not just a pointer? The goal of the code is to detect a mismatch in "pointerness" between an initializer expression and the type of the initialized element, so it needs to know if the expression is a pointer (non-nulls pointers are detected in type_initializer_zero_p). That means testing a number of IMO unintuitive conditions: TYPE_PTR_OR_PTRMEM_P (TREE_TYPE (expr)) || NULLPTR_TYPE_P (TREE_TYPE (expr)) || null_node_p (expr) I don't know if this type of a query is common in the C++ FE but unless this is an isolated use case then besides fixing the bug I thought it would be nice to make it easier to get the test above right, or at least come close to it. Since null_pointer_constant_p already exists (but isn't suitable here because it returns true for plain literal zeros) Why is that unsuitable? A literal zero is a perfectly good
Re: [PATCH] reject scalar array initialization with nullptr [PR94510]
On 4/12/20 5:49 PM, Martin Sebor wrote: On 4/10/20 8:52 AM, Jason Merrill wrote: On 4/9/20 4:23 PM, Martin Sebor wrote: On 4/9/20 1:32 PM, Jason Merrill wrote: On 4/9/20 3:24 PM, Martin Sebor wrote: On 4/9/20 1:03 PM, Jason Merrill wrote: On 4/8/20 1:23 PM, Martin Sebor wrote: On 4/7/20 3:36 PM, Marek Polacek wrote: On Tue, Apr 07, 2020 at 02:46:52PM -0600, Martin Sebor wrote: On 4/7/20 1:50 PM, Marek Polacek wrote: On Tue, Apr 07, 2020 at 12:50:48PM -0600, Martin Sebor via Gcc-patches wrote: Among the numerous regressions introduced by the change committed to GCC 9 to allow string literals as template arguments is a failure to recognize the C++ nullptr and GCC's __null constants as pointers. For one, I didn't realize that nullptr, being a null pointer constant, doesn't have a pointer type, and two, I didn't think of __null (which is a special integer constant that NULL sometimes expands to). The attached patch adjusts the special handling of trailing zero initializers in reshape_init_array_1 to recognize both kinds of constants and avoid treating them as zeros of the array integer element type. This restores the expected diagnostics when either constant is used in the initializer list. Martin PR c++/94510 - nullptr_t implicitly cast to zero twice in std::array gcc/cp/ChangeLog: PR c++/94510 * decl.c (reshape_init_array_1): Exclude mismatches with all kinds of pointers. gcc/testsuite/ChangeLog: PR c++/94510 * g++.dg/init/array57.C: New test. * g++.dg/init/array58.C: New test. diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index a127734af69..692c8ed73f4 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -6041,9 +6041,14 @@ reshape_init_array_1 (tree elt_type, tree max_index, reshape_iter *d, TREE_CONSTANT (new_init) = false; /* Pointers initialized to strings must be treated as non-zero - even if the string is empty. */ + even if the string is empty. Handle all kinds of pointers, + including std::nullptr and GCC's __nullptr, neither of which + has a pointer type. */ tree init_type = TREE_TYPE (elt_init); - if (POINTER_TYPE_P (elt_type) != POINTER_TYPE_P (init_type) + bool init_is_ptr = (POINTER_TYPE_P (init_type) + || NULLPTR_TYPE_P (init_type) + || null_node_p (elt_init)); + if (POINTER_TYPE_P (elt_type) != init_is_ptr || !type_initializer_zero_p (elt_type, elt_init)) last_nonzero = index; It looks like this still won't handle e.g. pointers to member functions, e.g. struct S { }; int arr[3] = { (void (S::*) ()) 0, 0, 0 }; would still be accepted. You could use TYPE_PTR_OR_PTRMEM_P instead of POINTER_TYPE_P to catch this case. Good catch! That doesn't fail because unlike null data member pointers which are represented as -1, member function pointers are represented as a zero. I had looked for an API that would answer the question: "is this expression a pointer?" without having to think of all the different kinds of them but all I could find was null_node_p(). Is this a rare, isolated case that having an API like that wouldn't be worth having or should I add one like in the attached update? Martin PR c++/94510 - nullptr_t implicitly cast to zero twice in std::array gcc/cp/ChangeLog: PR c++/94510 * decl.c (reshape_init_array_1): Exclude mismatches with all kinds of pointers. * gcc/cp/cp-tree.h (null_pointer_constant_p): New function. (Drop the gcc/cp/.) +/* Returns true if EXPR is a null pointer constant of any type. */ + +inline bool +null_pointer_constant_p (tree expr) +{ + STRIP_ANY_LOCATION_WRAPPER (expr); + if (expr == null_node) + return true; + tree type = TREE_TYPE (expr); + if (NULLPTR_TYPE_P (type)) + return true; + if (POINTER_TYPE_P (type)) + return integer_zerop (expr); + return null_member_pointer_value_p (expr); +} + We already have a null_ptr_cst_p so it would be sort of confusing to have this as well. But are you really interested in whether it's a null pointer, not just a pointer? The goal of the code is to detect a mismatch in "pointerness" between an initializer expression and the type of the initialized element, so it needs to know if the expression is a pointer (non-nulls pointers are detected in type_initializer_zero_p). That means testing a number of IMO unintuitive conditions: TYPE_PTR_OR_PTRMEM_P (TREE_TYPE (expr)) || NULLPTR_TYPE_P (TREE_TYPE (expr)) || null_node_p (expr) I don't know if this type of a query is common in the C++ FE but unless this is an isolated use case then besides fixing the bug I thought it would be nice to make it easier to get the test above right, or at least come close to it. Since null_pointer_constant_p already exists (but isn't suitable here because it returns true for plain literal zeros) Why is that unsuitable? A literal zero is a perfectly good zero-initializer for a pointer. Right, that's
Re: [PATCH] reject scalar array initialization with nullptr [PR94510]
On 4/10/20 8:52 AM, Jason Merrill wrote: On 4/9/20 4:23 PM, Martin Sebor wrote: On 4/9/20 1:32 PM, Jason Merrill wrote: On 4/9/20 3:24 PM, Martin Sebor wrote: On 4/9/20 1:03 PM, Jason Merrill wrote: On 4/8/20 1:23 PM, Martin Sebor wrote: On 4/7/20 3:36 PM, Marek Polacek wrote: On Tue, Apr 07, 2020 at 02:46:52PM -0600, Martin Sebor wrote: On 4/7/20 1:50 PM, Marek Polacek wrote: On Tue, Apr 07, 2020 at 12:50:48PM -0600, Martin Sebor via Gcc-patches wrote: Among the numerous regressions introduced by the change committed to GCC 9 to allow string literals as template arguments is a failure to recognize the C++ nullptr and GCC's __null constants as pointers. For one, I didn't realize that nullptr, being a null pointer constant, doesn't have a pointer type, and two, I didn't think of __null (which is a special integer constant that NULL sometimes expands to). The attached patch adjusts the special handling of trailing zero initializers in reshape_init_array_1 to recognize both kinds of constants and avoid treating them as zeros of the array integer element type. This restores the expected diagnostics when either constant is used in the initializer list. Martin PR c++/94510 - nullptr_t implicitly cast to zero twice in std::array gcc/cp/ChangeLog: PR c++/94510 * decl.c (reshape_init_array_1): Exclude mismatches with all kinds of pointers. gcc/testsuite/ChangeLog: PR c++/94510 * g++.dg/init/array57.C: New test. * g++.dg/init/array58.C: New test. diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index a127734af69..692c8ed73f4 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -6041,9 +6041,14 @@ reshape_init_array_1 (tree elt_type, tree max_index, reshape_iter *d, TREE_CONSTANT (new_init) = false; /* Pointers initialized to strings must be treated as non-zero - even if the string is empty. */ + even if the string is empty. Handle all kinds of pointers, + including std::nullptr and GCC's __nullptr, neither of which + has a pointer type. */ tree init_type = TREE_TYPE (elt_init); - if (POINTER_TYPE_P (elt_type) != POINTER_TYPE_P (init_type) + bool init_is_ptr = (POINTER_TYPE_P (init_type) + || NULLPTR_TYPE_P (init_type) + || null_node_p (elt_init)); + if (POINTER_TYPE_P (elt_type) != init_is_ptr || !type_initializer_zero_p (elt_type, elt_init)) last_nonzero = index; It looks like this still won't handle e.g. pointers to member functions, e.g. struct S { }; int arr[3] = { (void (S::*) ()) 0, 0, 0 }; would still be accepted. You could use TYPE_PTR_OR_PTRMEM_P instead of POINTER_TYPE_P to catch this case. Good catch! That doesn't fail because unlike null data member pointers which are represented as -1, member function pointers are represented as a zero. I had looked for an API that would answer the question: "is this expression a pointer?" without having to think of all the different kinds of them but all I could find was null_node_p(). Is this a rare, isolated case that having an API like that wouldn't be worth having or should I add one like in the attached update? Martin PR c++/94510 - nullptr_t implicitly cast to zero twice in std::array gcc/cp/ChangeLog: PR c++/94510 * decl.c (reshape_init_array_1): Exclude mismatches with all kinds of pointers. * gcc/cp/cp-tree.h (null_pointer_constant_p): New function. (Drop the gcc/cp/.) +/* Returns true if EXPR is a null pointer constant of any type. */ + +inline bool +null_pointer_constant_p (tree expr) +{ + STRIP_ANY_LOCATION_WRAPPER (expr); + if (expr == null_node) + return true; + tree type = TREE_TYPE (expr); + if (NULLPTR_TYPE_P (type)) + return true; + if (POINTER_TYPE_P (type)) + return integer_zerop (expr); + return null_member_pointer_value_p (expr); +} + We already have a null_ptr_cst_p so it would be sort of confusing to have this as well. But are you really interested in whether it's a null pointer, not just a pointer? The goal of the code is to detect a mismatch in "pointerness" between an initializer expression and the type of the initialized element, so it needs to know if the expression is a pointer (non-nulls pointers are detected in type_initializer_zero_p). That means testing a number of IMO unintuitive conditions: TYPE_PTR_OR_PTRMEM_P (TREE_TYPE (expr)) || NULLPTR_TYPE_P (TREE_TYPE (expr)) || null_node_p (expr) I don't know if this type of a query is common in the C++ FE but unless this is an isolated use case then besides fixing the bug I thought it would be nice to make it easier to get the test above right, or at least come close to it. Since null_pointer_constant_p already exists (but isn't suitable here because it returns true for plain literal zeros) Why is that unsuitable? A literal zero is a perfectly good zero-initializer for a pointer. Right, that's why it's not suitable here. Because a literal
Re: [PATCH] reject scalar array initialization with nullptr [PR94510]
On 4/9/20 4:23 PM, Martin Sebor wrote: On 4/9/20 1:32 PM, Jason Merrill wrote: On 4/9/20 3:24 PM, Martin Sebor wrote: On 4/9/20 1:03 PM, Jason Merrill wrote: On 4/8/20 1:23 PM, Martin Sebor wrote: On 4/7/20 3:36 PM, Marek Polacek wrote: On Tue, Apr 07, 2020 at 02:46:52PM -0600, Martin Sebor wrote: On 4/7/20 1:50 PM, Marek Polacek wrote: On Tue, Apr 07, 2020 at 12:50:48PM -0600, Martin Sebor via Gcc-patches wrote: Among the numerous regressions introduced by the change committed to GCC 9 to allow string literals as template arguments is a failure to recognize the C++ nullptr and GCC's __null constants as pointers. For one, I didn't realize that nullptr, being a null pointer constant, doesn't have a pointer type, and two, I didn't think of __null (which is a special integer constant that NULL sometimes expands to). The attached patch adjusts the special handling of trailing zero initializers in reshape_init_array_1 to recognize both kinds of constants and avoid treating them as zeros of the array integer element type. This restores the expected diagnostics when either constant is used in the initializer list. Martin PR c++/94510 - nullptr_t implicitly cast to zero twice in std::array gcc/cp/ChangeLog: PR c++/94510 * decl.c (reshape_init_array_1): Exclude mismatches with all kinds of pointers. gcc/testsuite/ChangeLog: PR c++/94510 * g++.dg/init/array57.C: New test. * g++.dg/init/array58.C: New test. diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index a127734af69..692c8ed73f4 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -6041,9 +6041,14 @@ reshape_init_array_1 (tree elt_type, tree max_index, reshape_iter *d, TREE_CONSTANT (new_init) = false; /* Pointers initialized to strings must be treated as non-zero - even if the string is empty. */ + even if the string is empty. Handle all kinds of pointers, + including std::nullptr and GCC's __nullptr, neither of which + has a pointer type. */ tree init_type = TREE_TYPE (elt_init); - if (POINTER_TYPE_P (elt_type) != POINTER_TYPE_P (init_type) + bool init_is_ptr = (POINTER_TYPE_P (init_type) + || NULLPTR_TYPE_P (init_type) + || null_node_p (elt_init)); + if (POINTER_TYPE_P (elt_type) != init_is_ptr || !type_initializer_zero_p (elt_type, elt_init)) last_nonzero = index; It looks like this still won't handle e.g. pointers to member functions, e.g. struct S { }; int arr[3] = { (void (S::*) ()) 0, 0, 0 }; would still be accepted. You could use TYPE_PTR_OR_PTRMEM_P instead of POINTER_TYPE_P to catch this case. Good catch! That doesn't fail because unlike null data member pointers which are represented as -1, member function pointers are represented as a zero. I had looked for an API that would answer the question: "is this expression a pointer?" without having to think of all the different kinds of them but all I could find was null_node_p(). Is this a rare, isolated case that having an API like that wouldn't be worth having or should I add one like in the attached update? Martin PR c++/94510 - nullptr_t implicitly cast to zero twice in std::array gcc/cp/ChangeLog: PR c++/94510 * decl.c (reshape_init_array_1): Exclude mismatches with all kinds of pointers. * gcc/cp/cp-tree.h (null_pointer_constant_p): New function. (Drop the gcc/cp/.) +/* Returns true if EXPR is a null pointer constant of any type. */ + +inline bool +null_pointer_constant_p (tree expr) +{ + STRIP_ANY_LOCATION_WRAPPER (expr); + if (expr == null_node) + return true; + tree type = TREE_TYPE (expr); + if (NULLPTR_TYPE_P (type)) + return true; + if (POINTER_TYPE_P (type)) + return integer_zerop (expr); + return null_member_pointer_value_p (expr); +} + We already have a null_ptr_cst_p so it would be sort of confusing to have this as well. But are you really interested in whether it's a null pointer, not just a pointer? The goal of the code is to detect a mismatch in "pointerness" between an initializer expression and the type of the initialized element, so it needs to know if the expression is a pointer (non-nulls pointers are detected in type_initializer_zero_p). That means testing a number of IMO unintuitive conditions: TYPE_PTR_OR_PTRMEM_P (TREE_TYPE (expr)) || NULLPTR_TYPE_P (TREE_TYPE (expr)) || null_node_p (expr) I don't know if this type of a query is common in the C++ FE but unless this is an isolated use case then besides fixing the bug I thought it would be nice to make it easier to get the test above right, or at least come close to it. Since null_pointer_constant_p already exists (but isn't suitable here because it returns true for plain literal zeros) Why is that unsuitable? A literal zero is a perfectly good zero-initializer for a pointer. Right, that's why it's not suitable here. Because a literal zero is also not a pointer. The question the code
Re: [PATCH] reject scalar array initialization with nullptr [PR94510]
On 4/9/20 1:32 PM, Jason Merrill wrote: On 4/9/20 3:24 PM, Martin Sebor wrote: On 4/9/20 1:03 PM, Jason Merrill wrote: On 4/8/20 1:23 PM, Martin Sebor wrote: On 4/7/20 3:36 PM, Marek Polacek wrote: On Tue, Apr 07, 2020 at 02:46:52PM -0600, Martin Sebor wrote: On 4/7/20 1:50 PM, Marek Polacek wrote: On Tue, Apr 07, 2020 at 12:50:48PM -0600, Martin Sebor via Gcc-patches wrote: Among the numerous regressions introduced by the change committed to GCC 9 to allow string literals as template arguments is a failure to recognize the C++ nullptr and GCC's __null constants as pointers. For one, I didn't realize that nullptr, being a null pointer constant, doesn't have a pointer type, and two, I didn't think of __null (which is a special integer constant that NULL sometimes expands to). The attached patch adjusts the special handling of trailing zero initializers in reshape_init_array_1 to recognize both kinds of constants and avoid treating them as zeros of the array integer element type. This restores the expected diagnostics when either constant is used in the initializer list. Martin PR c++/94510 - nullptr_t implicitly cast to zero twice in std::array gcc/cp/ChangeLog: PR c++/94510 * decl.c (reshape_init_array_1): Exclude mismatches with all kinds of pointers. gcc/testsuite/ChangeLog: PR c++/94510 * g++.dg/init/array57.C: New test. * g++.dg/init/array58.C: New test. diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index a127734af69..692c8ed73f4 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -6041,9 +6041,14 @@ reshape_init_array_1 (tree elt_type, tree max_index, reshape_iter *d, TREE_CONSTANT (new_init) = false; /* Pointers initialized to strings must be treated as non-zero - even if the string is empty. */ + even if the string is empty. Handle all kinds of pointers, + including std::nullptr and GCC's __nullptr, neither of which + has a pointer type. */ tree init_type = TREE_TYPE (elt_init); - if (POINTER_TYPE_P (elt_type) != POINTER_TYPE_P (init_type) + bool init_is_ptr = (POINTER_TYPE_P (init_type) + || NULLPTR_TYPE_P (init_type) + || null_node_p (elt_init)); + if (POINTER_TYPE_P (elt_type) != init_is_ptr || !type_initializer_zero_p (elt_type, elt_init)) last_nonzero = index; It looks like this still won't handle e.g. pointers to member functions, e.g. struct S { }; int arr[3] = { (void (S::*) ()) 0, 0, 0 }; would still be accepted. You could use TYPE_PTR_OR_PTRMEM_P instead of POINTER_TYPE_P to catch this case. Good catch! That doesn't fail because unlike null data member pointers which are represented as -1, member function pointers are represented as a zero. I had looked for an API that would answer the question: "is this expression a pointer?" without having to think of all the different kinds of them but all I could find was null_node_p(). Is this a rare, isolated case that having an API like that wouldn't be worth having or should I add one like in the attached update? Martin PR c++/94510 - nullptr_t implicitly cast to zero twice in std::array gcc/cp/ChangeLog: PR c++/94510 * decl.c (reshape_init_array_1): Exclude mismatches with all kinds of pointers. * gcc/cp/cp-tree.h (null_pointer_constant_p): New function. (Drop the gcc/cp/.) +/* Returns true if EXPR is a null pointer constant of any type. */ + +inline bool +null_pointer_constant_p (tree expr) +{ + STRIP_ANY_LOCATION_WRAPPER (expr); + if (expr == null_node) + return true; + tree type = TREE_TYPE (expr); + if (NULLPTR_TYPE_P (type)) + return true; + if (POINTER_TYPE_P (type)) + return integer_zerop (expr); + return null_member_pointer_value_p (expr); +} + We already have a null_ptr_cst_p so it would be sort of confusing to have this as well. But are you really interested in whether it's a null pointer, not just a pointer? The goal of the code is to detect a mismatch in "pointerness" between an initializer expression and the type of the initialized element, so it needs to know if the expression is a pointer (non-nulls pointers are detected in type_initializer_zero_p). That means testing a number of IMO unintuitive conditions: TYPE_PTR_OR_PTRMEM_P (TREE_TYPE (expr)) || NULLPTR_TYPE_P (TREE_TYPE (expr)) || null_node_p (expr) I don't know if this type of a query is common in the C++ FE but unless this is an isolated use case then besides fixing the bug I thought it would be nice to make it easier to get the test above right, or at least come close to it. Since null_pointer_constant_p already exists (but isn't suitable here because it returns true for plain literal zeros) Why is that unsuitable? A literal zero is a perfectly good zero-initializer for a pointer. Right, that's why it's not suitable here. Because a literal zero is also not a pointer. The question the code asks is: "is the initializer expression a
Re: [PATCH] reject scalar array initialization with nullptr [PR94510]
On 4/9/20 3:24 PM, Martin Sebor wrote: On 4/9/20 1:03 PM, Jason Merrill wrote: On 4/8/20 1:23 PM, Martin Sebor wrote: On 4/7/20 3:36 PM, Marek Polacek wrote: On Tue, Apr 07, 2020 at 02:46:52PM -0600, Martin Sebor wrote: On 4/7/20 1:50 PM, Marek Polacek wrote: On Tue, Apr 07, 2020 at 12:50:48PM -0600, Martin Sebor via Gcc-patches wrote: Among the numerous regressions introduced by the change committed to GCC 9 to allow string literals as template arguments is a failure to recognize the C++ nullptr and GCC's __null constants as pointers. For one, I didn't realize that nullptr, being a null pointer constant, doesn't have a pointer type, and two, I didn't think of __null (which is a special integer constant that NULL sometimes expands to). The attached patch adjusts the special handling of trailing zero initializers in reshape_init_array_1 to recognize both kinds of constants and avoid treating them as zeros of the array integer element type. This restores the expected diagnostics when either constant is used in the initializer list. Martin PR c++/94510 - nullptr_t implicitly cast to zero twice in std::array gcc/cp/ChangeLog: PR c++/94510 * decl.c (reshape_init_array_1): Exclude mismatches with all kinds of pointers. gcc/testsuite/ChangeLog: PR c++/94510 * g++.dg/init/array57.C: New test. * g++.dg/init/array58.C: New test. diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index a127734af69..692c8ed73f4 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -6041,9 +6041,14 @@ reshape_init_array_1 (tree elt_type, tree max_index, reshape_iter *d, TREE_CONSTANT (new_init) = false; /* Pointers initialized to strings must be treated as non-zero - even if the string is empty. */ + even if the string is empty. Handle all kinds of pointers, + including std::nullptr and GCC's __nullptr, neither of which + has a pointer type. */ tree init_type = TREE_TYPE (elt_init); - if (POINTER_TYPE_P (elt_type) != POINTER_TYPE_P (init_type) + bool init_is_ptr = (POINTER_TYPE_P (init_type) + || NULLPTR_TYPE_P (init_type) + || null_node_p (elt_init)); + if (POINTER_TYPE_P (elt_type) != init_is_ptr || !type_initializer_zero_p (elt_type, elt_init)) last_nonzero = index; It looks like this still won't handle e.g. pointers to member functions, e.g. struct S { }; int arr[3] = { (void (S::*) ()) 0, 0, 0 }; would still be accepted. You could use TYPE_PTR_OR_PTRMEM_P instead of POINTER_TYPE_P to catch this case. Good catch! That doesn't fail because unlike null data member pointers which are represented as -1, member function pointers are represented as a zero. I had looked for an API that would answer the question: "is this expression a pointer?" without having to think of all the different kinds of them but all I could find was null_node_p(). Is this a rare, isolated case that having an API like that wouldn't be worth having or should I add one like in the attached update? Martin PR c++/94510 - nullptr_t implicitly cast to zero twice in std::array gcc/cp/ChangeLog: PR c++/94510 * decl.c (reshape_init_array_1): Exclude mismatches with all kinds of pointers. * gcc/cp/cp-tree.h (null_pointer_constant_p): New function. (Drop the gcc/cp/.) +/* Returns true if EXPR is a null pointer constant of any type. */ + +inline bool +null_pointer_constant_p (tree expr) +{ + STRIP_ANY_LOCATION_WRAPPER (expr); + if (expr == null_node) + return true; + tree type = TREE_TYPE (expr); + if (NULLPTR_TYPE_P (type)) + return true; + if (POINTER_TYPE_P (type)) + return integer_zerop (expr); + return null_member_pointer_value_p (expr); +} + We already have a null_ptr_cst_p so it would be sort of confusing to have this as well. But are you really interested in whether it's a null pointer, not just a pointer? The goal of the code is to detect a mismatch in "pointerness" between an initializer expression and the type of the initialized element, so it needs to know if the expression is a pointer (non-nulls pointers are detected in type_initializer_zero_p). That means testing a number of IMO unintuitive conditions: TYPE_PTR_OR_PTRMEM_P (TREE_TYPE (expr)) || NULLPTR_TYPE_P (TREE_TYPE (expr)) || null_node_p (expr) I don't know if this type of a query is common in the C++ FE but unless this is an isolated use case then besides fixing the bug I thought it would be nice to make it easier to get the test above right, or at least come close to it. Since null_pointer_constant_p already exists (but isn't suitable here because it returns true for plain literal zeros) Why is that unsuitable? A literal zero is a perfectly good zero-initializer for a pointer. Right, that's why it's not suitable here. Because a literal zero is also not a pointer. The question the code asks is: "is the initializer expression a pointer (of any kind)?" Why is that a question we
Re: [PATCH] reject scalar array initialization with nullptr [PR94510]
On 4/9/20 1:03 PM, Jason Merrill wrote: On 4/8/20 1:23 PM, Martin Sebor wrote: On 4/7/20 3:36 PM, Marek Polacek wrote: On Tue, Apr 07, 2020 at 02:46:52PM -0600, Martin Sebor wrote: On 4/7/20 1:50 PM, Marek Polacek wrote: On Tue, Apr 07, 2020 at 12:50:48PM -0600, Martin Sebor via Gcc-patches wrote: Among the numerous regressions introduced by the change committed to GCC 9 to allow string literals as template arguments is a failure to recognize the C++ nullptr and GCC's __null constants as pointers. For one, I didn't realize that nullptr, being a null pointer constant, doesn't have a pointer type, and two, I didn't think of __null (which is a special integer constant that NULL sometimes expands to). The attached patch adjusts the special handling of trailing zero initializers in reshape_init_array_1 to recognize both kinds of constants and avoid treating them as zeros of the array integer element type. This restores the expected diagnostics when either constant is used in the initializer list. Martin PR c++/94510 - nullptr_t implicitly cast to zero twice in std::array gcc/cp/ChangeLog: PR c++/94510 * decl.c (reshape_init_array_1): Exclude mismatches with all kinds of pointers. gcc/testsuite/ChangeLog: PR c++/94510 * g++.dg/init/array57.C: New test. * g++.dg/init/array58.C: New test. diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index a127734af69..692c8ed73f4 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -6041,9 +6041,14 @@ reshape_init_array_1 (tree elt_type, tree max_index, reshape_iter *d, TREE_CONSTANT (new_init) = false; /* Pointers initialized to strings must be treated as non-zero - even if the string is empty. */ + even if the string is empty. Handle all kinds of pointers, + including std::nullptr and GCC's __nullptr, neither of which + has a pointer type. */ tree init_type = TREE_TYPE (elt_init); - if (POINTER_TYPE_P (elt_type) != POINTER_TYPE_P (init_type) + bool init_is_ptr = (POINTER_TYPE_P (init_type) + || NULLPTR_TYPE_P (init_type) + || null_node_p (elt_init)); + if (POINTER_TYPE_P (elt_type) != init_is_ptr || !type_initializer_zero_p (elt_type, elt_init)) last_nonzero = index; It looks like this still won't handle e.g. pointers to member functions, e.g. struct S { }; int arr[3] = { (void (S::*) ()) 0, 0, 0 }; would still be accepted. You could use TYPE_PTR_OR_PTRMEM_P instead of POINTER_TYPE_P to catch this case. Good catch! That doesn't fail because unlike null data member pointers which are represented as -1, member function pointers are represented as a zero. I had looked for an API that would answer the question: "is this expression a pointer?" without having to think of all the different kinds of them but all I could find was null_node_p(). Is this a rare, isolated case that having an API like that wouldn't be worth having or should I add one like in the attached update? Martin PR c++/94510 - nullptr_t implicitly cast to zero twice in std::array gcc/cp/ChangeLog: PR c++/94510 * decl.c (reshape_init_array_1): Exclude mismatches with all kinds of pointers. * gcc/cp/cp-tree.h (null_pointer_constant_p): New function. (Drop the gcc/cp/.) +/* Returns true if EXPR is a null pointer constant of any type. */ + +inline bool +null_pointer_constant_p (tree expr) +{ + STRIP_ANY_LOCATION_WRAPPER (expr); + if (expr == null_node) + return true; + tree type = TREE_TYPE (expr); + if (NULLPTR_TYPE_P (type)) + return true; + if (POINTER_TYPE_P (type)) + return integer_zerop (expr); + return null_member_pointer_value_p (expr); +} + We already have a null_ptr_cst_p so it would be sort of confusing to have this as well. But are you really interested in whether it's a null pointer, not just a pointer? The goal of the code is to detect a mismatch in "pointerness" between an initializer expression and the type of the initialized element, so it needs to know if the expression is a pointer (non-nulls pointers are detected in type_initializer_zero_p). That means testing a number of IMO unintuitive conditions: TYPE_PTR_OR_PTRMEM_P (TREE_TYPE (expr)) || NULLPTR_TYPE_P (TREE_TYPE (expr)) || null_node_p (expr) I don't know if this type of a query is common in the C++ FE but unless this is an isolated use case then besides fixing the bug I thought it would be nice to make it easier to get the test above right, or at least come close to it. Since null_pointer_constant_p already exists (but isn't suitable here because it returns true for plain literal zeros) Why is that unsuitable? A literal zero is a perfectly good zero-initializer for a pointer. Right, that's why it's not suitable here. Because a literal zero is also not a pointer. The question the code asks is: "is the initializer expression a pointer (of any kind)?" and I thought that might be common enough to justify adding a helper
Re: [PATCH] reject scalar array initialization with nullptr [PR94510]
On 4/8/20 1:23 PM, Martin Sebor wrote: On 4/7/20 3:36 PM, Marek Polacek wrote: On Tue, Apr 07, 2020 at 02:46:52PM -0600, Martin Sebor wrote: On 4/7/20 1:50 PM, Marek Polacek wrote: On Tue, Apr 07, 2020 at 12:50:48PM -0600, Martin Sebor via Gcc-patches wrote: Among the numerous regressions introduced by the change committed to GCC 9 to allow string literals as template arguments is a failure to recognize the C++ nullptr and GCC's __null constants as pointers. For one, I didn't realize that nullptr, being a null pointer constant, doesn't have a pointer type, and two, I didn't think of __null (which is a special integer constant that NULL sometimes expands to). The attached patch adjusts the special handling of trailing zero initializers in reshape_init_array_1 to recognize both kinds of constants and avoid treating them as zeros of the array integer element type. This restores the expected diagnostics when either constant is used in the initializer list. Martin PR c++/94510 - nullptr_t implicitly cast to zero twice in std::array gcc/cp/ChangeLog: PR c++/94510 * decl.c (reshape_init_array_1): Exclude mismatches with all kinds of pointers. gcc/testsuite/ChangeLog: PR c++/94510 * g++.dg/init/array57.C: New test. * g++.dg/init/array58.C: New test. diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index a127734af69..692c8ed73f4 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -6041,9 +6041,14 @@ reshape_init_array_1 (tree elt_type, tree max_index, reshape_iter *d, TREE_CONSTANT (new_init) = false; /* Pointers initialized to strings must be treated as non-zero - even if the string is empty. */ + even if the string is empty. Handle all kinds of pointers, + including std::nullptr and GCC's __nullptr, neither of which + has a pointer type. */ tree init_type = TREE_TYPE (elt_init); - if (POINTER_TYPE_P (elt_type) != POINTER_TYPE_P (init_type) + bool init_is_ptr = (POINTER_TYPE_P (init_type) + || NULLPTR_TYPE_P (init_type) + || null_node_p (elt_init)); + if (POINTER_TYPE_P (elt_type) != init_is_ptr || !type_initializer_zero_p (elt_type, elt_init)) last_nonzero = index; It looks like this still won't handle e.g. pointers to member functions, e.g. struct S { }; int arr[3] = { (void (S::*) ()) 0, 0, 0 }; would still be accepted. You could use TYPE_PTR_OR_PTRMEM_P instead of POINTER_TYPE_P to catch this case. Good catch! That doesn't fail because unlike null data member pointers which are represented as -1, member function pointers are represented as a zero. I had looked for an API that would answer the question: "is this expression a pointer?" without having to think of all the different kinds of them but all I could find was null_node_p(). Is this a rare, isolated case that having an API like that wouldn't be worth having or should I add one like in the attached update? Martin PR c++/94510 - nullptr_t implicitly cast to zero twice in std::array gcc/cp/ChangeLog: PR c++/94510 * decl.c (reshape_init_array_1): Exclude mismatches with all kinds of pointers. * gcc/cp/cp-tree.h (null_pointer_constant_p): New function. (Drop the gcc/cp/.) +/* Returns true if EXPR is a null pointer constant of any type. */ + +inline bool +null_pointer_constant_p (tree expr) +{ + STRIP_ANY_LOCATION_WRAPPER (expr); + if (expr == null_node) + return true; + tree type = TREE_TYPE (expr); + if (NULLPTR_TYPE_P (type)) + return true; + if (POINTER_TYPE_P (type)) + return integer_zerop (expr); + return null_member_pointer_value_p (expr); +} + We already have a null_ptr_cst_p so it would be sort of confusing to have this as well. But are you really interested in whether it's a null pointer, not just a pointer? The goal of the code is to detect a mismatch in "pointerness" between an initializer expression and the type of the initialized element, so it needs to know if the expression is a pointer (non-nulls pointers are detected in type_initializer_zero_p). That means testing a number of IMO unintuitive conditions: TYPE_PTR_OR_PTRMEM_P (TREE_TYPE (expr)) || NULLPTR_TYPE_P (TREE_TYPE (expr)) || null_node_p (expr) I don't know if this type of a query is common in the C++ FE but unless this is an isolated use case then besides fixing the bug I thought it would be nice to make it easier to get the test above right, or at least come close to it. Since null_pointer_constant_p already exists (but isn't suitable here because it returns true for plain literal zeros) Why is that unsuitable? A literal zero is a perfectly good zero-initializer for a pointer. Jason
Re: [PATCH] reject scalar array initialization with nullptr [PR94510]
On 4/8/20 12:43 PM, Jason Merrill wrote: On 4/7/20 2:50 PM, Martin Sebor wrote: Among the numerous regressions introduced by the change committed to GCC 9 to allow string literals as template arguments is a failure to recognize the C++ nullptr and GCC's __null constants as pointers. For one, I didn't realize that nullptr, being a null pointer constant, doesn't have a pointer type, and two, I didn't think of __null (which is a special integer constant that NULL sometimes expands to). The attached patch adjusts the special handling of trailing zero initializers in reshape_init_array_1 to recognize both kinds of constants and avoid treating them as zeros of the array integer element type. This restores the expected diagnostics when either constant is used in the initializer list. This is another problem due to doing this checking too early, as with 90938. Let's look at your other patch from the 90938 discussion to move the zero pruning to process_init_constructor_array. The patch for pr90938 handles this correctly, and I'm planning to resubmit it in stage 1 (as I thought ultimately ended up being your expectation as well). A I mentioned in the review at the end of February I had reservations about making those changes then because it seemed late to me. I'm that much less comfortable with them now, barely a month away from the anticipated release date. For reference, the last revision of the patch is here: https://gcc.gnu.org/pipermail/gcc-patches/2020-February/540792.html Martin PS As a reminder, all these bugs (94510, 94124, 90947, and 90938) were introduced by making a similar change to code I wasn't familiar with at the very end of GCC 9.
Re: [PATCH] reject scalar array initialization with nullptr [PR94510]
On 4/7/20 2:50 PM, Martin Sebor wrote: Among the numerous regressions introduced by the change committed to GCC 9 to allow string literals as template arguments is a failure to recognize the C++ nullptr and GCC's __null constants as pointers. For one, I didn't realize that nullptr, being a null pointer constant, doesn't have a pointer type, and two, I didn't think of __null (which is a special integer constant that NULL sometimes expands to). The attached patch adjusts the special handling of trailing zero initializers in reshape_init_array_1 to recognize both kinds of constants and avoid treating them as zeros of the array integer element type. This restores the expected diagnostics when either constant is used in the initializer list. This is another problem due to doing this checking too early, as with 90938. Let's look at your other patch from the 90938 discussion to move the zero pruning to process_init_constructor_array. Jason
Re: [PATCH] reject scalar array initialization with nullptr [PR94510]
On 4/7/20 3:36 PM, Marek Polacek wrote: On Tue, Apr 07, 2020 at 02:46:52PM -0600, Martin Sebor wrote: On 4/7/20 1:50 PM, Marek Polacek wrote: On Tue, Apr 07, 2020 at 12:50:48PM -0600, Martin Sebor via Gcc-patches wrote: Among the numerous regressions introduced by the change committed to GCC 9 to allow string literals as template arguments is a failure to recognize the C++ nullptr and GCC's __null constants as pointers. For one, I didn't realize that nullptr, being a null pointer constant, doesn't have a pointer type, and two, I didn't think of __null (which is a special integer constant that NULL sometimes expands to). The attached patch adjusts the special handling of trailing zero initializers in reshape_init_array_1 to recognize both kinds of constants and avoid treating them as zeros of the array integer element type. This restores the expected diagnostics when either constant is used in the initializer list. Martin PR c++/94510 - nullptr_t implicitly cast to zero twice in std::array gcc/cp/ChangeLog: PR c++/94510 * decl.c (reshape_init_array_1): Exclude mismatches with all kinds of pointers. gcc/testsuite/ChangeLog: PR c++/94510 * g++.dg/init/array57.C: New test. * g++.dg/init/array58.C: New test. diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index a127734af69..692c8ed73f4 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -6041,9 +6041,14 @@ reshape_init_array_1 (tree elt_type, tree max_index, reshape_iter *d, TREE_CONSTANT (new_init) = false; /* Pointers initialized to strings must be treated as non-zero -even if the string is empty. */ +even if the string is empty. Handle all kinds of pointers, +including std::nullptr and GCC's __nullptr, neither of which +has a pointer type. */ tree init_type = TREE_TYPE (elt_init); - if (POINTER_TYPE_P (elt_type) != POINTER_TYPE_P (init_type) + bool init_is_ptr = (POINTER_TYPE_P (init_type) + || NULLPTR_TYPE_P (init_type) + || null_node_p (elt_init)); + if (POINTER_TYPE_P (elt_type) != init_is_ptr || !type_initializer_zero_p (elt_type, elt_init)) last_nonzero = index; It looks like this still won't handle e.g. pointers to member functions, e.g. struct S { }; int arr[3] = { (void (S::*) ()) 0, 0, 0 }; would still be accepted. You could use TYPE_PTR_OR_PTRMEM_P instead of POINTER_TYPE_P to catch this case. Good catch! That doesn't fail because unlike null data member pointers which are represented as -1, member function pointers are represented as a zero. I had looked for an API that would answer the question: "is this expression a pointer?" without having to think of all the different kinds of them but all I could find was null_node_p(). Is this a rare, isolated case that having an API like that wouldn't be worth having or should I add one like in the attached update? Martin PR c++/94510 - nullptr_t implicitly cast to zero twice in std::array gcc/cp/ChangeLog: PR c++/94510 * decl.c (reshape_init_array_1): Exclude mismatches with all kinds of pointers. * gcc/cp/cp-tree.h (null_pointer_constant_p): New function. (Drop the gcc/cp/.) +/* Returns true if EXPR is a null pointer constant of any type. */ + +inline bool +null_pointer_constant_p (tree expr) +{ + STRIP_ANY_LOCATION_WRAPPER (expr); + if (expr == null_node) +return true; + tree type = TREE_TYPE (expr); + if (NULLPTR_TYPE_P (type)) +return true; + if (POINTER_TYPE_P (type)) +return integer_zerop (expr); + return null_member_pointer_value_p (expr); +} + We already have a null_ptr_cst_p so it would be sort of confusing to have this as well. But are you really interested in whether it's a null pointer, not just a pointer? The goal of the code is to detect a mismatch in "pointerness" between an initializer expression and the type of the initialized element, so it needs to know if the expression is a pointer (non-nulls pointers are detected in type_initializer_zero_p). That means testing a number of IMO unintuitive conditions: TYPE_PTR_OR_PTRMEM_P (TREE_TYPE (expr)) || NULLPTR_TYPE_P (TREE_TYPE (expr)) || null_node_p (expr) I don't know if this type of a query is common in the C++ FE but unless this is an isolated use case then besides fixing the bug I thought it would be nice to make it easier to get the test above right, or at least come close to it. Since null_pointer_constant_p already exists (but isn't suitable here because it returns true for plain literal zeros) would a function like /* Returns true if EXPR is a pointer of any type, including nullptr and __null. */ inline bool pointer_p (tree expr) { STRIP_ANY_LOCATION_WRAPPER (expr); if (expr == null_node) return true; tree type = TREE_TYPE (expr); if (NULLPTR_TYPE_P (type)) return true; return TYPE_PTR_OR_PTRMEM_P (type);
Re: [PATCH] reject scalar array initialization with nullptr [PR94510]
On Tue, Apr 07, 2020 at 02:46:52PM -0600, Martin Sebor wrote: > On 4/7/20 1:50 PM, Marek Polacek wrote: > > On Tue, Apr 07, 2020 at 12:50:48PM -0600, Martin Sebor via Gcc-patches > > wrote: > > > Among the numerous regressions introduced by the change committed > > > to GCC 9 to allow string literals as template arguments is a failure > > > to recognize the C++ nullptr and GCC's __null constants as pointers. > > > For one, I didn't realize that nullptr, being a null pointer constant, > > > doesn't have a pointer type, and two, I didn't think of __null (which > > > is a special integer constant that NULL sometimes expands to). > > > > > > The attached patch adjusts the special handling of trailing zero > > > initializers in reshape_init_array_1 to recognize both kinds of > > > constants and avoid treating them as zeros of the array integer > > > element type. This restores the expected diagnostics when either > > > constant is used in the initializer list. > > > > > > Martin > > > > > PR c++/94510 - nullptr_t implicitly cast to zero twice in std::array > > > > > > gcc/cp/ChangeLog: > > > > > > PR c++/94510 > > > * decl.c (reshape_init_array_1): Exclude mismatches with all kinds > > > of pointers. > > > > > > gcc/testsuite/ChangeLog: > > > > > > PR c++/94510 > > > * g++.dg/init/array57.C: New test. > > > * g++.dg/init/array58.C: New test. > > > > > > diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c > > > index a127734af69..692c8ed73f4 100644 > > > --- a/gcc/cp/decl.c > > > +++ b/gcc/cp/decl.c > > > @@ -6041,9 +6041,14 @@ reshape_init_array_1 (tree elt_type, tree > > > max_index, reshape_iter *d, > > > TREE_CONSTANT (new_init) = false; > > > /* Pointers initialized to strings must be treated as non-zero > > > - even if the string is empty. */ > > > + even if the string is empty. Handle all kinds of pointers, > > > + including std::nullptr and GCC's __nullptr, neither of which > > > + has a pointer type. */ > > > tree init_type = TREE_TYPE (elt_init); > > > - if (POINTER_TYPE_P (elt_type) != POINTER_TYPE_P (init_type) > > > + bool init_is_ptr = (POINTER_TYPE_P (init_type) > > > + || NULLPTR_TYPE_P (init_type) > > > + || null_node_p (elt_init)); > > > + if (POINTER_TYPE_P (elt_type) != init_is_ptr > > > || !type_initializer_zero_p (elt_type, elt_init)) > > > last_nonzero = index; > > > > It looks like this still won't handle e.g. pointers to member functions, > > e.g. > > > > struct S { }; > > int arr[3] = { (void (S::*) ()) 0, 0, 0 }; > > > > would still be accepted. You could use TYPE_PTR_OR_PTRMEM_P instead of > > POINTER_TYPE_P to catch this case. > > Good catch! That doesn't fail because unlike null data member pointers > which are represented as -1, member function pointers are represented > as a zero. > > I had looked for an API that would answer the question: "is this > expression a pointer?" without having to think of all the different > kinds of them but all I could find was null_node_p(). Is this a rare, > isolated case that having an API like that wouldn't be worth having > or should I add one like in the attached update? > > Martin > PR c++/94510 - nullptr_t implicitly cast to zero twice in std::array > > gcc/cp/ChangeLog: > > PR c++/94510 > * decl.c (reshape_init_array_1): Exclude mismatches with all kinds > of pointers. > * gcc/cp/cp-tree.h (null_pointer_constant_p): New function. (Drop the gcc/cp/.) > +/* Returns true if EXPR is a null pointer constant of any type. */ > + > +inline bool > +null_pointer_constant_p (tree expr) > +{ > + STRIP_ANY_LOCATION_WRAPPER (expr); > + if (expr == null_node) > +return true; > + tree type = TREE_TYPE (expr); > + if (NULLPTR_TYPE_P (type)) > +return true; > + if (POINTER_TYPE_P (type)) > +return integer_zerop (expr); > + return null_member_pointer_value_p (expr); > +} > + We already have a null_ptr_cst_p so it would be sort of confusing to have this as well. But are you really interested in whether it's a null pointer, not just a pointer? Marek
Re: [PATCH] reject scalar array initialization with nullptr [PR94510]
On 4/7/20 1:50 PM, Marek Polacek wrote: On Tue, Apr 07, 2020 at 12:50:48PM -0600, Martin Sebor via Gcc-patches wrote: Among the numerous regressions introduced by the change committed to GCC 9 to allow string literals as template arguments is a failure to recognize the C++ nullptr and GCC's __null constants as pointers. For one, I didn't realize that nullptr, being a null pointer constant, doesn't have a pointer type, and two, I didn't think of __null (which is a special integer constant that NULL sometimes expands to). The attached patch adjusts the special handling of trailing zero initializers in reshape_init_array_1 to recognize both kinds of constants and avoid treating them as zeros of the array integer element type. This restores the expected diagnostics when either constant is used in the initializer list. Martin PR c++/94510 - nullptr_t implicitly cast to zero twice in std::array gcc/cp/ChangeLog: PR c++/94510 * decl.c (reshape_init_array_1): Exclude mismatches with all kinds of pointers. gcc/testsuite/ChangeLog: PR c++/94510 * g++.dg/init/array57.C: New test. * g++.dg/init/array58.C: New test. diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index a127734af69..692c8ed73f4 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -6041,9 +6041,14 @@ reshape_init_array_1 (tree elt_type, tree max_index, reshape_iter *d, TREE_CONSTANT (new_init) = false; /* Pointers initialized to strings must be treated as non-zero -even if the string is empty. */ +even if the string is empty. Handle all kinds of pointers, +including std::nullptr and GCC's __nullptr, neither of which +has a pointer type. */ tree init_type = TREE_TYPE (elt_init); - if (POINTER_TYPE_P (elt_type) != POINTER_TYPE_P (init_type) + bool init_is_ptr = (POINTER_TYPE_P (init_type) + || NULLPTR_TYPE_P (init_type) + || null_node_p (elt_init)); + if (POINTER_TYPE_P (elt_type) != init_is_ptr || !type_initializer_zero_p (elt_type, elt_init)) last_nonzero = index; It looks like this still won't handle e.g. pointers to member functions, e.g. struct S { }; int arr[3] = { (void (S::*) ()) 0, 0, 0 }; would still be accepted. You could use TYPE_PTR_OR_PTRMEM_P instead of POINTER_TYPE_P to catch this case. Good catch! That doesn't fail because unlike null data member pointers which are represented as -1, member function pointers are represented as a zero. I had looked for an API that would answer the question: "is this expression a pointer?" without having to think of all the different kinds of them but all I could find was null_node_p(). Is this a rare, isolated case that having an API like that wouldn't be worth having or should I add one like in the attached update? Martin PR c++/94510 - nullptr_t implicitly cast to zero twice in std::array gcc/cp/ChangeLog: PR c++/94510 * decl.c (reshape_init_array_1): Exclude mismatches with all kinds of pointers. * gcc/cp/cp-tree.h (null_pointer_constant_p): New function. gcc/testsuite/ChangeLog: PR c++/94510 * g++.dg/init/array57.C: New test. * g++.dg/init/array58.C: New test. diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 63aaf615926..9ec6e3883c8 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -8022,6 +8022,22 @@ null_node_p (const_tree expr) return expr == null_node; } +/* Returns true if EXPR is a null pointer constant of any type. */ + +inline bool +null_pointer_constant_p (tree expr) +{ + STRIP_ANY_LOCATION_WRAPPER (expr); + if (expr == null_node) +return true; + tree type = TREE_TYPE (expr); + if (NULLPTR_TYPE_P (type)) +return true; + if (POINTER_TYPE_P (type)) +return integer_zerop (expr); + return null_member_pointer_value_p (expr); +} + /* True iff T is a variable template declaration. */ inline bool variable_template_p (tree t) diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index a127734af69..5cf5b601d29 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -6041,9 +6041,13 @@ reshape_init_array_1 (tree elt_type, tree max_index, reshape_iter *d, TREE_CONSTANT (new_init) = false; /* Pointers initialized to strings must be treated as non-zero - even if the string is empty. */ + even if the string is empty. Handle all kinds of pointers, + including std::nullptr and GCC's __nullptr, neither of which + has a pointer type. */ tree init_type = TREE_TYPE (elt_init); - if (POINTER_TYPE_P (elt_type) != POINTER_TYPE_P (init_type) + bool init_is_ptr = (POINTER_TYPE_P (init_type) + || null_pointer_constant_p (elt_init)); + if (POINTER_TYPE_P (elt_type) != init_is_ptr || !type_initializer_zero_p (elt_type, elt_init)) last_nonzero = index; diff --git a/gcc/testsuite/g++.dg/init/array57.C b/gcc/testsuite/g++.dg/init/array57.C new file mode 100644 index 000..70e86445c07 --- /dev/null +++
Re: [PATCH] reject scalar array initialization with nullptr [PR94510]
On Tue, Apr 07, 2020 at 12:50:48PM -0600, Martin Sebor via Gcc-patches wrote: > Among the numerous regressions introduced by the change committed > to GCC 9 to allow string literals as template arguments is a failure > to recognize the C++ nullptr and GCC's __null constants as pointers. > For one, I didn't realize that nullptr, being a null pointer constant, > doesn't have a pointer type, and two, I didn't think of __null (which > is a special integer constant that NULL sometimes expands to). > > The attached patch adjusts the special handling of trailing zero > initializers in reshape_init_array_1 to recognize both kinds of > constants and avoid treating them as zeros of the array integer > element type. This restores the expected diagnostics when either > constant is used in the initializer list. > > Martin > PR c++/94510 - nullptr_t implicitly cast to zero twice in std::array > > gcc/cp/ChangeLog: > > PR c++/94510 > * decl.c (reshape_init_array_1): Exclude mismatches with all kinds > of pointers. > > gcc/testsuite/ChangeLog: > > PR c++/94510 > * g++.dg/init/array57.C: New test. > * g++.dg/init/array58.C: New test. > > diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c > index a127734af69..692c8ed73f4 100644 > --- a/gcc/cp/decl.c > +++ b/gcc/cp/decl.c > @@ -6041,9 +6041,14 @@ reshape_init_array_1 (tree elt_type, tree max_index, > reshape_iter *d, > TREE_CONSTANT (new_init) = false; > >/* Pointers initialized to strings must be treated as non-zero > - even if the string is empty. */ > + even if the string is empty. Handle all kinds of pointers, > + including std::nullptr and GCC's __nullptr, neither of which > + has a pointer type. */ >tree init_type = TREE_TYPE (elt_init); > - if (POINTER_TYPE_P (elt_type) != POINTER_TYPE_P (init_type) > + bool init_is_ptr = (POINTER_TYPE_P (init_type) > + || NULLPTR_TYPE_P (init_type) > + || null_node_p (elt_init)); > + if (POINTER_TYPE_P (elt_type) != init_is_ptr > || !type_initializer_zero_p (elt_type, elt_init)) > last_nonzero = index; It looks like this still won't handle e.g. pointers to member functions, e.g. struct S { }; int arr[3] = { (void (S::*) ()) 0, 0, 0 }; would still be accepted. You could use TYPE_PTR_OR_PTRMEM_P instead of POINTER_TYPE_P to catch this case. Marek