[C++ Patch] PR 65323

[C++ Patch] PR 65323

[Paolo Carlini]

[-- Attachment #1: Type: text/plain, Size: 639 bytes --]

Hi,

this is a regression about duplicate warnings with 
-Wzero-as-null-pointer-constant. The regression is rather old, affects 
4_8-branch too, and started when check_default_argument got a 
perform_implicit_conversion_flags call which warns a first time, then 
maybe_warn_zero_as_null_pointer_constant as called by 
check_default_argument itself warns a second time. The latter call is 
even older, dates back to c++/52718, I think we can now safely remove it 
and keep on returning nullptr_node to avoid warning later still at the 
call sites (that was the point of c++/52718). Tested x86_64-linux.

Thanks,
Paolo.

///////////////////

[-- Attachment #2: patch_65323 --]
[-- Type: text/plain, Size: 633 bytes --]

Index: decl.c
===================================================================
--- decl.c	(revision 221230)
+++ decl.c	(working copy)
@@ -11227,11 +11227,9 @@ check_default_argument (tree decl, tree arg, tsubs
 				     LOOKUP_IMPLICIT);
   --cp_unevaluated_operand;
 
-  if (warn_zero_as_null_pointer_constant
-      && TYPE_PTR_OR_PTRMEM_P (decl_type)
+  if (TYPE_PTR_OR_PTRMEM_P (decl_type)
       && null_ptr_cst_p (arg)
-      && (complain & tf_warning)
-      && maybe_warn_zero_as_null_pointer_constant (arg, input_location))
+      && !NULLPTR_TYPE_P (TREE_TYPE (arg)))
     return nullptr_node;
 
   /* [dcl.fct.default]

Re: [C++ Patch] PR 65323

[Paolo Carlini]

[-- Attachment #1: Type: text/plain, Size: 116 bytes --]

... in case, I think we can as well apply the below, a tad simpler. Also 
passes testing.

Paolo.

////////////////

[-- Attachment #2: patch_65323_2 --]
[-- Type: text/plain, Size: 620 bytes --]

Index: decl.c
===================================================================
--- decl.c	(revision 221230)
+++ decl.c	(working copy)
@@ -11227,11 +11227,8 @@ check_default_argument (tree decl, tree arg, tsubs
 				     LOOKUP_IMPLICIT);
   --cp_unevaluated_operand;
 
-  if (warn_zero_as_null_pointer_constant
-      && TYPE_PTR_OR_PTRMEM_P (decl_type)
-      && null_ptr_cst_p (arg)
-      && (complain & tf_warning)
-      && maybe_warn_zero_as_null_pointer_constant (arg, input_location))
+  if (TYPE_PTR_OR_PTRMEM_P (decl_type)
+      && null_ptr_cst_p (arg))
     return nullptr_node;
 
   /* [dcl.fct.default]

Re: [C++ Patch] PR 65323

[Jason Merrill]

On 03/06/2015 03:36 AM, Paolo Carlini wrote:
> this is a regression about duplicate warnings with
> -Wzero-as-null-pointer-constant. The regression is rather old, affects
> 4_8-branch too, and started when check_default_argument got a
> perform_implicit_conversion_flags call which warns a first time, then
> maybe_warn_zero_as_null_pointer_constant as called by
> check_default_argument itself warns a second time. The latter call is
> even older, dates back to c++/52718, I think we can now safely remove it
> and keep on returning nullptr_node to avoid warning later still at the
> call sites (that was the point of c++/52718). Tested x86_64-linux.

Do we need this special handling at all?  When I remove that whole 'if' 
block I still only get one warning from the 52718 testcase.

Jason

Re: [C++ Patch] PR 65323

[Paolo Carlini]

Hi,

On 03/11/2015 09:26 PM, Jason Merrill wrote:
> On 03/06/2015 03:36 AM, Paolo Carlini wrote:
>> this is a regression about duplicate warnings with
>> -Wzero-as-null-pointer-constant. The regression is rather old, affects
>> 4_8-branch too, and started when check_default_argument got a
>> perform_implicit_conversion_flags call which warns a first time, then
>> maybe_warn_zero_as_null_pointer_constant as called by
>> check_default_argument itself warns a second time. The latter call is
>> even older, dates back to c++/52718, I think we can now safely remove it
>> and keep on returning nullptr_node to avoid warning later still at the
>> call sites (that was the point of c++/52718). Tested x86_64-linux.
>
> Do we need this special handling at all?  When I remove that whole 
> 'if' block I still only get one warning from the 52718 testcase.
I just tried again for this reduced version of 52718 (I added a 0 && to 
the 'if'):

void* fun(void* a = 0);
void* f2 = fun();

and I got (removed the irrelevant carets):

52718_red.C:1:22: warning: zero as null pointer constant 
[-Wzero-as-null-pointer-constant]
  void* fun(void* a = 0);

52718_red.C:2:16: warning: zero as null pointer constant 
[-Wzero-as-null-pointer-constant]
  void* f2 = fun();

That is, as far as I can see, the rationale that led to an early 
*return* for 52718 still stand: no matter what we do at the beginning of 
check_default_argument, whether we warn via 
perform_implicit_conversion_flags or immediately, we still want to early 
return to avoid warning again at the call site.

Paolo.

Re: [C++ Patch] PR 65323

[Jason Merrill]

On 03/12/2015 06:13 AM, Paolo Carlini wrote:
> 52718_red.C:1:22: warning: zero as null pointer constant
> [-Wzero-as-null-pointer-constant]
>   void* fun(void* a = 0);
>
> 52718_red.C:2:16: warning: zero as null pointer constant
> [-Wzero-as-null-pointer-constant]
>   void* f2 = fun();

OK, then your second patch is OK.  But please add a comment that the 
code is there to avoid a redundant warning at the call site.

Jason