Re: [C++ PATCH] Do not warn about zero-as-null when NULL is used.
On Wed, May 30, 2018 at 3:00 PM, Ville Voutilainen wrote: > On 30 May 2018 at 18:32, Ville Voutilainen > wrote: >> Now it does. This passes all the Wzero-as-null-pointer-constant tests in >> warn/ >> and cpp0x/, running full suite on Linux-PPC64. Ok for trunk if the tests >> pass? > > Here we go again. :) > > + && (TYPE_PTRMEMFUNC_P (totype) || TYPE_PTRDATAMEM_P (totype) > + || TYPE_PTR_P (totype) || NULLPTR_TYPE_P (totype))) You could use TYPE_PTR_OR_PTRMEM_P here. OK with that change. Jason
Re: [C++ PATCH] Do not warn about zero-as-null when NULL is used.
On 30 May 2018 at 18:32, Ville Voutilainen wrote:
> Now it does. This passes all the Wzero-as-null-pointer-constant tests in warn/
> and cpp0x/, running full suite on Linux-PPC64. Ok for trunk if the tests
> pass?
Here we go again. :)
2018-05-30 Ville Voutilainen
gcc/cp/
Do not warn about zero-as-null when NULL is used.
* call.c (conversion_null_warnings): Check for pointer
types converted from zero constants.
(convert_like_real): Add a warning sentinel at the end.
* tree.c (maybe_warn_zero_as_null_pointer_constant): Also
check null_node_p.
testsuite/
Do not warn about zero-as-null when NULL is used.
* g++.dg/warn/Wzero-as-null-pointer-constant-7.C: New.
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 7aadd64..18e1ab9 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -6517,6 +6517,7 @@ build_temp (tree expr, tree type, int flags,
}
/* Perform warnings about peculiar, but valid, conversions from/to NULL.
+ Also handle a subset of zero as null warnings.
EXPR is implicitly converted to type TOTYPE.
FN and ARGNUM are used for diagnostics. */
@@ -6551,6 +6552,16 @@ conversion_null_warnings (tree totype, tree expr, tree fn, int argnum)
warning_at (input_location, OPT_Wconversion_null,
"converting % to pointer type %qT", totype);
}
+ /* Handle zero as null pointer warnings for cases other
+ than EQ_EXPR and NE_EXPR */
+ else if (null_ptr_cst_p (expr)
+ && (TYPE_PTRMEMFUNC_P (totype) || TYPE_PTRDATAMEM_P (totype)
+ || TYPE_PTR_P (totype) || NULLPTR_TYPE_P (totype)))
+{
+ source_location loc =
+ expansion_point_location_if_in_system_header (input_location);
+ maybe_warn_zero_as_null_pointer_constant (expr, loc);
+}
}
/* We gave a diagnostic during a conversion. If this was in the second
@@ -7101,6 +7112,7 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
&& !check_narrowing (totype, expr, complain))
return error_mark_node;
+ warning_sentinel w (warn_zero_as_null_pointer_constant);
if (issue_conversion_warnings)
expr = cp_convert_and_check (totype, expr, complain);
else
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index f21daac..332d51d 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -5423,7 +5423,7 @@ bool
maybe_warn_zero_as_null_pointer_constant (tree expr, location_t loc)
{
if (c_inhibit_evaluation_warnings == 0
- && !NULLPTR_TYPE_P (TREE_TYPE (expr)))
+ && !null_node_p (expr) && !NULLPTR_TYPE_P (TREE_TYPE (expr)))
{
warning_at (loc, OPT_Wzero_as_null_pointer_constant,
"zero as null pointer constant");
diff --git a/gcc/testsuite/g++.dg/warn/Wzero-as-null-pointer-constant-7.C b/gcc/testsuite/g++.dg/warn/Wzero-as-null-pointer-constant-7.C
new file mode 100644
index 000..0d06dbf
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wzero-as-null-pointer-constant-7.C
@@ -0,0 +1,13 @@
+// { dg-options "-Wzero-as-null-pointer-constant" }
+// { dg-do compile { target c++11 } }
+
+#include
+
+void test01()
+{
+ char* x(NULL);
+ char* x2{NULL};
+ char* x3 = NULL;
+ char* x4(0); // { dg-warning "zero as null pointer" }
+ char* x5 = 0; // { dg-warning "zero as null pointer" }
+}
Re: [C++ PATCH] Do not warn about zero-as-null when NULL is used.
On 30 May 2018 at 18:12, Jason Merrill wrote:
> Hmm, why doesn't maybe_warn_zero_as_null_pointer_constant check
> null_node_p like it does NULLPTR_TYPE_P?
Now it does. This passes all the Wzero-as-null-pointer-constant tests in warn/
and cpp0x/, running full suite on Linux-PPC64. Ok for trunk if the tests
pass?
2018-05-30 Ville Voutilainen
gcc/cp/
Do not warn about zero-as-null when NULL is used.
* typeck.c (cp_build_binary_op): Diagnose zero as null here..
* call.c (conversion_null_warnings): ..and here..
* cvt.c (cp_convert_to_pointer): ..not here.
* tree.c (maybe_warn_zero_as_null_pointer_constant): Also
check null_node_p.
testsuite/
Do not warn about zero-as-null when NULL is used.
* g++.dg/warn/Wzero-as-null-pointer-constant-7.C: New.
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 7aadd64..0c7e462 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -6517,6 +6517,7 @@ build_temp (tree expr, tree type, int flags,
}
/* Perform warnings about peculiar, but valid, conversions from/to NULL.
+ Also handle a subset of zero as null warnings.
EXPR is implicitly converted to type TOTYPE.
FN and ARGNUM are used for diagnostics. */
@@ -6551,6 +6552,16 @@ conversion_null_warnings (tree totype, tree expr, tree fn, int argnum)
warning_at (input_location, OPT_Wconversion_null,
"converting % to pointer type %qT", totype);
}
+ /* Handle zero as null pointer warnings for cases other
+ than EQ_EXPR and NE_EXPR */
+ else if (null_ptr_cst_p (expr)
+ && (TYPE_PTRMEMFUNC_P (totype) || TYPE_PTRDATAMEM_P (totype)
+ || TYPE_PTR_P (totype)))
+{
+ source_location loc =
+ expansion_point_location_if_in_system_header (input_location);
+ maybe_warn_zero_as_null_pointer_constant (expr, loc);
+}
}
/* We gave a diagnostic during a conversion. If this was in the second
diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c
index f29dacd..e2f8579 100644
--- a/gcc/cp/cvt.c
+++ b/gcc/cp/cvt.c
@@ -208,9 +208,6 @@ cp_convert_to_pointer (tree type, tree expr, bool dofold,
return build_ptrmemfunc (TYPE_PTRMEMFUNC_FN_TYPE (type), expr, 0,
/*c_cast_p=*/false, complain);
- if (complain & tf_warning)
- maybe_warn_zero_as_null_pointer_constant (expr, loc);
-
/* A NULL pointer-to-data-member is represented by -1, not by
zero. */
tree val = (TYPE_PTRDATAMEM_P (type)
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index f21daac..332d51d 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -5423,7 +5423,7 @@ bool
maybe_warn_zero_as_null_pointer_constant (tree expr, location_t loc)
{
if (c_inhibit_evaluation_warnings == 0
- && !NULLPTR_TYPE_P (TREE_TYPE (expr)))
+ && !null_node_p (expr) && !NULLPTR_TYPE_P (TREE_TYPE (expr)))
{
warning_at (loc, OPT_Wzero_as_null_pointer_constant,
"zero as null pointer constant");
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 3df043e..d65b39f 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -4353,6 +4353,21 @@ cp_build_binary_op (location_t location,
warning_at (loc, OPT_Wpointer_arith, "NULL used in arithmetic");
}
+ /* Handle zero as null pointer warnings for pointer comparisons */
+ if (code == EQ_EXPR || code == NE_EXPR)
+{
+ source_location loc =
+ expansion_point_location_if_in_system_header (input_location);
+ if ((TYPE_PTRMEMFUNC_P (type0) || TYPE_PTRDATAMEM_P (type0)
+ || TYPE_PTR_P (type0))
+ && null_ptr_cst_p (op1))
+ maybe_warn_zero_as_null_pointer_constant (op1, loc);
+ else if ((TYPE_PTRMEMFUNC_P (type1) || TYPE_PTRDATAMEM_P (type1)
+ || TYPE_PTR_P (type1))
+ && null_ptr_cst_p (op0))
+ maybe_warn_zero_as_null_pointer_constant (op0, loc);
+}
+
/* In case when one of the operands of the binary operation is
a vector and another is a scalar -- convert scalar to vector. */
if ((code0 == VECTOR_TYPE) != (code1 == VECTOR_TYPE))
@@ -4833,9 +4848,6 @@ cp_build_binary_op (location_t location,
integer_one_node,
complain);
- if (complain & tf_warning)
- maybe_warn_zero_as_null_pointer_constant (op1, input_location);
-
e2 = cp_build_binary_op (location,
EQ_EXPR, e2, integer_zero_node,
complain);
diff --git a/gcc/testsuite/g++.dg/warn/Wzero-as-null-pointer-constant-7.C b/gcc/testsuite/g++.dg/warn/Wzero-as-null-pointer-constant-7.C
new file mode 100644
index 000..0d06dbf
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wzero-as-null-pointer-constant-7.C
@@ -0,0 +1,13 @@
+// { dg-options "-Wzero-as-null-pointer-constant" }
+// { dg-do compile { target c++11 } }
+
+#include
+
+void test01()
+{
+ char* x(NULL);
+ char* x2{NULL};
+ char* x3 = NULL;
+ char* x4(0); // { dg-warning "zero as null pointer" }
+ char* x5 = 0; // { dg-warning "zero as null pointer" }
+}
Re: [C++ PATCH] Do not warn about zero-as-null when NULL is used.
On Wed, May 30, 2018 at 5:24 AM, Ville Voutilainen wrote: > On 29 May 2018 at 23:43, Ville Voutilainen > wrote: >> Another round. The other occurrence of >> maybe_warn_zero_as_null_pointer_constant >> in typeck.c seems superfluous. The one in cvt.c seems necessary for >> cpp0x/Wzero-as-null* tests. It seems like cp_build_binary_op is far more >> suited >> to check the EQ_EXPR/NE_EXPR cases than conversion_null_warnings is. >> >> Tested manually on Linux-x64, running full suite on Linux-PPC64. Ok for >> trunk? >> >> 2018-05-29 Ville Voutilainen >> >> gcc/cp/ >> >> Do not warn about zero-as-null when NULL is used. >> * typeck.c (cp_build_binary_op): Diagnose zero as null here.. >> * call.c (conversion_null_warnings): ..and here.. >> * cvt.c (cp_convert_to_pointer): ..not here. >> >> testsuite/ >> >> Do not warn about zero-as-null when NULL is used. >> * g++.dg/warn/Wzero-as-null-pointer-constant-7.C: New. > > If we want, we can fine-tune conversion_null_warnings to not bother > with the source location if we don't call maybe_warn, thus. Hmm, why doesn't maybe_warn_zero_as_null_pointer_constant check null_node_p like it does NULLPTR_TYPE_P? Jason
Re: [C++ PATCH] Do not warn about zero-as-null when NULL is used.
On 29 May 2018 at 23:43, Ville Voutilainen wrote:
> Another round. The other occurrence of
> maybe_warn_zero_as_null_pointer_constant
> in typeck.c seems superfluous. The one in cvt.c seems necessary for
> cpp0x/Wzero-as-null* tests. It seems like cp_build_binary_op is far more
> suited
> to check the EQ_EXPR/NE_EXPR cases than conversion_null_warnings is.
>
> Tested manually on Linux-x64, running full suite on Linux-PPC64. Ok for trunk?
>
> 2018-05-29 Ville Voutilainen
>
> gcc/cp/
>
> Do not warn about zero-as-null when NULL is used.
> * typeck.c (cp_build_binary_op): Diagnose zero as null here..
> * call.c (conversion_null_warnings): ..and here..
> * cvt.c (cp_convert_to_pointer): ..not here.
>
> testsuite/
>
> Do not warn about zero-as-null when NULL is used.
> * g++.dg/warn/Wzero-as-null-pointer-constant-7.C: New.
If we want, we can fine-tune conversion_null_warnings to not bother
with the source location
if we don't call maybe_warn, thus.
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 5890b73..c7c9741 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -6556,12 +6556,14 @@ conversion_null_warnings (tree totype, tree expr, tree fn, int argnum)
than EQ_EXPR and NE_EXPR */
else if (!null_node_p (expr))
{
- source_location loc =
- expansion_point_location_if_in_system_header (input_location);
if (null_ptr_cst_p (expr)
&& (TYPE_PTRMEMFUNC_P (totype) || TYPE_PTRDATAMEM_P (totype)
|| TYPE_PTR_P (totype)))
- maybe_warn_zero_as_null_pointer_constant (expr, loc);
+ {
+ source_location loc =
+ expansion_point_location_if_in_system_header (input_location);
+ maybe_warn_zero_as_null_pointer_constant (expr, loc);
+ }
}
}
Re: [C++ PATCH] Do not warn about zero-as-null when NULL is used.
Another round. The other occurrence of maybe_warn_zero_as_null_pointer_constant
in typeck.c seems superfluous. The one in cvt.c seems necessary for
cpp0x/Wzero-as-null* tests. It seems like cp_build_binary_op is far more suited
to check the EQ_EXPR/NE_EXPR cases than conversion_null_warnings is.
Tested manually on Linux-x64, running full suite on Linux-PPC64. Ok for trunk?
2018-05-29 Ville Voutilainen
gcc/cp/
Do not warn about zero-as-null when NULL is used.
* typeck.c (cp_build_binary_op): Diagnose zero as null here..
* call.c (conversion_null_warnings): ..and here..
* cvt.c (cp_convert_to_pointer): ..not here.
testsuite/
Do not warn about zero-as-null when NULL is used.
* g++.dg/warn/Wzero-as-null-pointer-constant-7.C: New.
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 7aadd64..5890b73 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -6517,6 +6517,7 @@ build_temp (tree expr, tree type, int flags,
}
/* Perform warnings about peculiar, but valid, conversions from/to NULL.
+ Also handle a subset of zero as null warnings.
EXPR is implicitly converted to type TOTYPE.
FN and ARGNUM are used for diagnostics. */
@@ -6551,6 +6552,17 @@ conversion_null_warnings (tree totype, tree expr, tree fn, int argnum)
warning_at (input_location, OPT_Wconversion_null,
"converting % to pointer type %qT", totype);
}
+ /* Handle zero as null pointer warnings for cases other
+ than EQ_EXPR and NE_EXPR */
+ else if (!null_node_p (expr))
+{
+ source_location loc =
+ expansion_point_location_if_in_system_header (input_location);
+ if (null_ptr_cst_p (expr)
+ && (TYPE_PTRMEMFUNC_P (totype) || TYPE_PTRDATAMEM_P (totype)
+ || TYPE_PTR_P (totype)))
+ maybe_warn_zero_as_null_pointer_constant (expr, loc);
+}
}
/* We gave a diagnostic during a conversion. If this was in the second
diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c
index f29dacd..e2f8579 100644
--- a/gcc/cp/cvt.c
+++ b/gcc/cp/cvt.c
@@ -208,9 +208,6 @@ cp_convert_to_pointer (tree type, tree expr, bool dofold,
return build_ptrmemfunc (TYPE_PTRMEMFUNC_FN_TYPE (type), expr, 0,
/*c_cast_p=*/false, complain);
- if (complain & tf_warning)
- maybe_warn_zero_as_null_pointer_constant (expr, loc);
-
/* A NULL pointer-to-data-member is represented by -1, not by
zero. */
tree val = (TYPE_PTRDATAMEM_P (type)
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 3df043e..2eb4cf1 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -4353,6 +4353,21 @@ cp_build_binary_op (location_t location,
warning_at (loc, OPT_Wpointer_arith, "NULL used in arithmetic");
}
+ /* Handle zero as null pointer warnings for pointer comparisons */
+ if (code == EQ_EXPR || code == NE_EXPR)
+{
+ source_location loc =
+ expansion_point_location_if_in_system_header (input_location);
+ if ((TYPE_PTRMEMFUNC_P (type0) || TYPE_PTRDATAMEM_P (type0)
+ || TYPE_PTR_P (type0))
+ && !null_node_p (op1) && null_ptr_cst_p (op1))
+ maybe_warn_zero_as_null_pointer_constant (op1, loc);
+ else if ((TYPE_PTRMEMFUNC_P (type1) || TYPE_PTRDATAMEM_P (type1)
+ || TYPE_PTR_P (type1))
+ && !null_node_p (op0) && null_ptr_cst_p (op0))
+ maybe_warn_zero_as_null_pointer_constant (op0, loc);
+}
+
/* In case when one of the operands of the binary operation is
a vector and another is a scalar -- convert scalar to vector. */
if ((code0 == VECTOR_TYPE) != (code1 == VECTOR_TYPE))
@@ -4833,9 +4848,6 @@ cp_build_binary_op (location_t location,
integer_one_node,
complain);
- if (complain & tf_warning)
- maybe_warn_zero_as_null_pointer_constant (op1, input_location);
-
e2 = cp_build_binary_op (location,
EQ_EXPR, e2, integer_zero_node,
complain);
diff --git a/gcc/testsuite/g++.dg/warn/Wzero-as-null-pointer-constant-7.C b/gcc/testsuite/g++.dg/warn/Wzero-as-null-pointer-constant-7.C
new file mode 100644
index 000..0d06dbf
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wzero-as-null-pointer-constant-7.C
@@ -0,0 +1,13 @@
+// { dg-options "-Wzero-as-null-pointer-constant" }
+// { dg-do compile { target c++11 } }
+
+#include
+
+void test01()
+{
+ char* x(NULL);
+ char* x2{NULL};
+ char* x3 = NULL;
+ char* x4(0); // { dg-warning "zero as null pointer" }
+ char* x5 = 0; // { dg-warning "zero as null pointer" }
+}
Re: [C++ PATCH] Do not warn about zero-as-null when NULL is used.
On 25 May 2018 at 20:38, Ville Voutilainen wrote: > On 25 May 2018 at 20:27, Jason Merrill wrote: >> On Thu, May 24, 2018 at 8:04 PM, Ville Voutilainen >> wrote: >>> I smacked my head against conversion_null_warnings for a while, >>> and then I realized that we could just stop convert_like_real from >>> changing the node type for null_node. >> >> Won't that sometimes mean that the result has the wrong type? If we >> convert NULL to short, we want the result to have type short. > > None of our tests revealed any regressions with the change; the change passes > the full suite. And apparently the removed code in convert_like_real doesn't > affect that; implicit and explicit conversions from NULL to short still > convert > to short, and code like auto x = NULL; still converts to unsigned long. As far as I can see, cp_convert_to_pointer does convert NULL to the "right" kind of integer constant, it's just that convert_like_real doesn't do it earlier. The earlier conversion caused the inability to diagnose the conversion properly in cp_convert_to_pointer, but that earlier conversion doesn't seem to have other effects.
Re: [C++ PATCH] Do not warn about zero-as-null when NULL is used.
On 25 May 2018 at 20:27, Jason Merrill wrote: > On Thu, May 24, 2018 at 8:04 PM, Ville Voutilainen > wrote: >> I smacked my head against conversion_null_warnings for a while, >> and then I realized that we could just stop convert_like_real from >> changing the node type for null_node. > > Won't that sometimes mean that the result has the wrong type? If we > convert NULL to short, we want the result to have type short. None of our tests revealed any regressions with the change; the change passes the full suite. And apparently the removed code in convert_like_real doesn't affect that; implicit and explicit conversions from NULL to short still convert to short, and code like auto x = NULL; still converts to unsigned long.
Re: [C++ PATCH] Do not warn about zero-as-null when NULL is used.
On Thu, May 24, 2018 at 8:04 PM, Ville Voutilainen wrote: > I smacked my head against conversion_null_warnings for a while, > and then I realized that we could just stop convert_like_real from > changing the node type for null_node. Won't that sometimes mean that the result has the wrong type? If we convert NULL to short, we want the result to have type short. Jason
[C++ PATCH] Do not warn about zero-as-null when NULL is used.
I smacked my head against conversion_null_warnings for a while,
and then I realized that we could just stop convert_like_real from
changing the node type for null_node.
Tested manually on Linux-x64, running full suite on Linux-PPC64,
ok for trunk?
2018-05-25 Ville Voutilainen
gcc/cp/
Do not warn about zero-as-null when NULL is used.
* call.c (convert_like_real): Don't turn a null_node into integer_cst.
* cvt.c (cp_convert_to_pointer): Don't warn about null_nodes.
testsuite/
Do not warn about zero-as-null when NULL is used.
* g++.dg/warn/Wzero-as-null-pointer-constant-7.C: New.
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 7aadd64..cb07bb7 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -6799,12 +6799,6 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
if (type_unknown_p (expr))
expr = instantiate_type (totype, expr, complain);
- if (expr == null_node
- && INTEGRAL_OR_UNSCOPED_ENUMERATION_TYPE_P (totype))
- /* If __null has been converted to an integer type, we do not want to
- continue to warn about uses of EXPR as an integer, rather than as a
- pointer. */
- expr = build_int_cst (totype, 0);
return expr;
case ck_ambig:
/* We leave bad_p off ck_ambig because overload resolution considers
diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c
index f29dacd..36529f9 100644
--- a/gcc/cp/cvt.c
+++ b/gcc/cp/cvt.c
@@ -208,8 +208,8 @@ cp_convert_to_pointer (tree type, tree expr, bool dofold,
return build_ptrmemfunc (TYPE_PTRMEMFUNC_FN_TYPE (type), expr, 0,
/*c_cast_p=*/false, complain);
- if (complain & tf_warning)
- maybe_warn_zero_as_null_pointer_constant (expr, loc);
+ if (!null_node_p (expr) && (complain & tf_warning))
+ maybe_warn_zero_as_null_pointer_constant (expr, loc);
/* A NULL pointer-to-data-member is represented by -1, not by
zero. */
diff --git a/gcc/testsuite/g++.dg/warn/Wzero-as-null-pointer-constant-7.C b/gcc/testsuite/g++.dg/warn/Wzero-as-null-pointer-constant-7.C
new file mode 100644
index 000..0d06dbf
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wzero-as-null-pointer-constant-7.C
@@ -0,0 +1,13 @@
+// { dg-options "-Wzero-as-null-pointer-constant" }
+// { dg-do compile { target c++11 } }
+
+#include
+
+void test01()
+{
+ char* x(NULL);
+ char* x2{NULL};
+ char* x3 = NULL;
+ char* x4(0); // { dg-warning "zero as null pointer" }
+ char* x5 = 0; // { dg-warning "zero as null pointer" }
+}
