Patchwork [C++] PR 53301

login
register
mail settings
Submitter Paolo Carlini
Date May 9, 2012, 11:12 p.m.
Message ID <4FAAF9E0.6010307@oracle.com>
Download mbox | patch
Permalink /patch/158058/
State New
Headers show

Comments

Paolo Carlini - May 9, 2012, 11:12 p.m.
Hi,

shame on me. I think the patch almost qualifies as obvious. Tested 
x86_64-linux.

Thanks,
Paolo.

//////////////////
/cp
2012-05-10  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/53301
	* decl.c (check_default_argument): Fix typo (POINTER_TYPE_P
	instead of TYPE_PTR_P) in zero-as-null-pointer-constant warning.

/testsuite
2012-05-10  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/53301
	* g++.dg/warn/Wzero-as-null-pointer-constant-6.C: New.
Jason Merrill - May 10, 2012, 2:46 p.m.
On 05/09/2012 07:12 PM, Paolo Carlini wrote:
> shame on me. I think the patch almost qualifies as obvious.

I think it does.  OK.

Jason
Paolo Carlini - May 10, 2012, 2:52 p.m.
Hi,

> On 05/09/2012 07:12 PM, Paolo Carlini wrote:
>> shame on me. I think the patch almost qualifies as obvious.
> 
> I think it does.  OK.

Good, later today I'll commit it (branch too).

Was thinking: would it make sense to have a predicate for 'any' pointer type? I see tens of such || around and I bet I would not have typoed it here... If you agree, please pick a name and I will do the work ;)

Paolo
Jason Merrill - May 10, 2012, 2:57 p.m.
On 05/10/2012 10:52 AM, Paolo Carlini wrote:
> Was thinking: would it make sense to have a predicate for 'any' pointer type?

Something like TYPE_PTR_OR_PTRMEM_P would be fine.

Hmm, I see that TYPE_PTRMEM_P only means pointer to data member, that's 
unfortunate; the name doesn't make that clear.

Jason
Paolo Carlini - May 10, 2012, 3:02 p.m.
Hi,

> On 05/10/2012 10:52 AM, Paolo Carlini wrote:
>> Was thinking: would it make sense to have a predicate for 'any' pointer type?
> 
> Something like TYPE_PTR_OR_PTRMEM_P would be fine.

Good.

> Hmm, I see that TYPE_PTRMEM_P only means pointer to data member, that's unfortunate; the name doesn't make that clear.

So, let's have a plan about the names of such predicates and I'll implement it as soon as possible. 

Paolo
Manuel López-Ibáñez - May 10, 2012, 3:37 p.m.
On 10 May 2012 17:02, Paolo Carlini <pcarlini@gmail.com> wrote:
> Hi,
>
>> On 05/10/2012 10:52 AM, Paolo Carlini wrote:
>>> Was thinking: would it make sense to have a predicate for 'any' pointer type?
>>
>> Something like TYPE_PTR_OR_PTRMEM_P would be fine.
>
> Good.
>
>> Hmm, I see that TYPE_PTRMEM_P only means pointer to data member, that's unfortunate; the name doesn't make that clear.
>
> So, let's have a plan about the names of such predicates and I'll implement it as soon as possible.

Yes, please. It feels as if the names are based more on the underlying
implementation of the macro than on anything else. Also, short names
are nice, but using MEM instead of MEMBER is a bit too short. The same
for OB for object and others.

PTR_OR_PTRMEM sounds to me like "pointer or pointer to member", which
sounds redundant since a pointer to member is a pointer already.

And there is also TYPE_PTRMEM_P and TYPE_PTR_TO_MEMBER_P. From the
names it is not clear what is the difference. This could be
TYPE_PTR_TO_DATA_MEMBER and TYPE_PTR_TO_ANY_MEMBER. The few extra
chars help a lot to clarify the meaning.

Also tree.h already has POINTER_TYPE_P, what is the difference? There
are a few other such accessors where the names seem to match with
other accessors from cp-tree.h, but the implementations are a bit
different. And both forms are used in cp/. Quite a mess...

Cheers,

Manuel.
Paolo Carlini - May 10, 2012, 9:31 p.m.
Hi,
> Yes, please. It feels as if the names are based more on the underlying
> implementation of the macro than on anything else. Also, short names
> are nice, but using MEM instead of MEMBER is a bit too short. The same
> for OB for object and others.
>
> PTR_OR_PTRMEM sounds to me like "pointer or pointer to member", which
> sounds redundant since a pointer to member is a pointer already.
>
> And there is also TYPE_PTRMEM_P and TYPE_PTR_TO_MEMBER_P. From the
> names it is not clear what is the difference. This could be
> TYPE_PTR_TO_DATA_MEMBER and TYPE_PTR_TO_ANY_MEMBER. The few extra
> chars help a lot to clarify the meaning.
Let's see if we can do something *now* ;) My concrete proposal would be:

TYPE_PTRMEM_P rename to TYPE_PTRDATAMEM_P (consistent with 
TYPE_PTRMEMFUNC_P)
TYPE_PTR_TO_MEMBER_P rename to TYPE_PTRMEM_P

and then finally

#define TYPE_PTR_OR_PTRMEM_P(NODE) \
     (TYPE_PTR_P (NODE) || TYPE_PTRMEM_P (NODE))

and use it everywhere. Sounds like an improvement?

Additionally, we could maybe rename PTRMEM_OK_P to PTRMEMFUNC_OK_P

Thanks,
Paolo.
Jason Merrill - May 11, 2012, 4:41 a.m.
On 05/10/2012 05:31 PM, Paolo Carlini wrote:
> Let's see if we can do something *now* ;) My concrete proposal would be:
>
> TYPE_PTRMEM_P rename to TYPE_PTRDATAMEM_P (consistent with
> TYPE_PTRMEMFUNC_P)
> TYPE_PTR_TO_MEMBER_P rename to TYPE_PTRMEM_P
>
> and then finally
>
> #define TYPE_PTR_OR_PTRMEM_P(NODE) \
> (TYPE_PTR_P (NODE) || TYPE_PTRMEM_P (NODE))
>
> and use it everywhere. Sounds like an improvement?

Sounds pretty good.  But I suspect a lot of places want to check 
TYPE_PTR_P || TYPE_PTRDATAMEM_P (because you can't just use TREE_TYPE to 
get the function type of a PMF), so this new macro doesn't help with 
your desire to avoid writing ||.

> Additionally, we could maybe rename PTRMEM_OK_P to PTRMEMFUNC_OK_P

No, that flag applies to both varieties of members.

Jason
Paolo Carlini - May 11, 2012, 8:18 a.m.
Hi,

On 05/11/2012 06:41 AM, Jason Merrill wrote:
> On 05/10/2012 05:31 PM, Paolo Carlini wrote:
>> Let's see if we can do something *now* ;) My concrete proposal would be:
>>
>> TYPE_PTRMEM_P rename to TYPE_PTRDATAMEM_P (consistent with
>> TYPE_PTRMEMFUNC_P)
>> TYPE_PTR_TO_MEMBER_P rename to TYPE_PTRMEM_P
>>
>> and then finally
>>
>> #define TYPE_PTR_OR_PTRMEM_P(NODE) \
>> (TYPE_PTR_P (NODE) || TYPE_PTRMEM_P (NODE))
>>
>> and use it everywhere. Sounds like an improvement?
>
> Sounds pretty good.  But I suspect a lot of places want to check 
> TYPE_PTR_P || TYPE_PTRDATAMEM_P (because you can't just use TREE_TYPE 
> to get the function type of a PMF), so this new macro doesn't help 
> with your desire to avoid writing ||.
Well, I wanted to avoid writing it somewhere *else* ;) But, if we want, 
we could add a new predicate for your case too, maybe in a following 
step. What do you think?
>> Additionally, we could maybe rename PTRMEM_OK_P to PTRMEMFUNC_OK_P
> No, that flag applies to both varieties of members.
I see. Then while we are at it we could probably tweak a bit a the 
comment, I find it a tad misleading:

/* Indicates when overload resolution may resolve to a pointer to
    member function. [expr.unary.op]/3 */

???

Thanks,
Paolo.

Patch

Index: testsuite/g++.dg/warn/Wzero-as-null-pointer-constant-6.C
===================================================================
--- testsuite/g++.dg/warn/Wzero-as-null-pointer-constant-6.C	(revision 0)
+++ testsuite/g++.dg/warn/Wzero-as-null-pointer-constant-6.C	(revision 0)
@@ -0,0 +1,6 @@ 
+// PR c++/53301
+// { dg-options "-Wzero-as-null-pointer-constant" }
+
+class x { public: x(int v) {} };
+
+void foo(const x& = 0);
Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 187335)
+++ cp/decl.c	(working copy)
@@ -10619,7 +10619,7 @@  check_default_argument (tree decl, tree arg)
 
   if (warn_zero_as_null_pointer_constant
       && c_inhibit_evaluation_warnings == 0
-      && (POINTER_TYPE_P (decl_type) || TYPE_PTR_TO_MEMBER_P (decl_type))
+      && (TYPE_PTR_P (decl_type) || TYPE_PTR_TO_MEMBER_P (decl_type))
       && null_ptr_cst_p (arg)
       && !NULLPTR_TYPE_P (TREE_TYPE (arg)))
     {