diff mbox

[005/236] Introduce as_a_nullable

Message ID 1407345815-14551-6-git-send-email-dmalcolm@redhat.com
State New
Headers show

Commit Message

David Malcolm Aug. 6, 2014, 5:19 p.m. UTC
In many circumstances, is_a_helper <T>::test assumes that the pointer is
non-NULL, but sometimes you have a pointer of type T that can be NULL.

Earlier versions of this patch kit made numerous uses of the ternary
operator to handle nullable pointers e.g.:

  return insn ? as_a <rtx_insn *> (insn) : NULL;

but this was ugly.  Instead, introduce an as_a_nullable<T> variant that
adds a check for NULL, so the above can be written simply as:

  return as_a_nullable <rtx_insn *> (insn);

gcc/
	* is-a.h (template<T, U> as_a_nullable <U *p>) New function.
---
 gcc/is-a.h | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

Comments

Jeff Law Aug. 12, 2014, 9:03 p.m. UTC | #1
On 08/06/14 11:19, David Malcolm wrote:
> In many circumstances, is_a_helper <T>::test assumes that the pointer is
> non-NULL, but sometimes you have a pointer of type T that can be NULL.
>
> Earlier versions of this patch kit made numerous uses of the ternary
> operator to handle nullable pointers e.g.:
>
>    return insn ? as_a <rtx_insn *> (insn) : NULL;
>
> but this was ugly.  Instead, introduce an as_a_nullable<T> variant that
> adds a check for NULL, so the above can be written simply as:
>
>    return as_a_nullable <rtx_insn *> (insn);
>
> gcc/
> 	* is-a.h (template<T, U> as_a_nullable <U *p>) New function.
Presumably the cases where we wanted to use as_a on a NULL pointer were 
places were we can legitimately have a NULL pointer?

I guess since they'll be using as_a_nullable, they're effectively 
documented as potentially being NULL and we can go back and look at them 
at a future date.

OK.

jeff
Martin Jambor Aug. 13, 2014, 10:01 a.m. UTC | #2
Hi,

On Wed, Aug 06, 2014 at 01:19:44PM -0400, David Malcolm wrote:
> In many circumstances, is_a_helper <T>::test assumes that the pointer is
> non-NULL, but sometimes you have a pointer of type T that can be NULL.
> 
> Earlier versions of this patch kit made numerous uses of the ternary
> operator to handle nullable pointers e.g.:
> 
>   return insn ? as_a <rtx_insn *> (insn) : NULL;
> 
> but this was ugly.  Instead, introduce an as_a_nullable<T> variant that
> adds a check for NULL, so the above can be written simply as:
> 
>   return as_a_nullable <rtx_insn *> (insn);

A tiny bikeshedding, disregard it if you don't like it: however, the
"nullable" part of the name suggests the pointer can be NULLed in the
process.  I think that variants of functions that survive NULL
arguments are traditionally called "safe" in gcc, so what about
"safe_as" (or safely_as)?

Martin


> 
> gcc/
> 	* is-a.h (template<T, U> as_a_nullable <U *p>) New function.
> ---
>  gcc/is-a.h | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/gcc/is-a.h b/gcc/is-a.h
> index a14e344..f50e62c 100644
> --- a/gcc/is-a.h
> +++ b/gcc/is-a.h
> @@ -46,6 +46,14 @@ TYPE as_a <TYPE> (pointer)
>  
>        do_something_with (as_a <cgraph_node *> *ptr);
>  
> +TYPE as_a_nullable <TYPE> (pointer)
> +
> +    Like as_a <TYPE> (pointer), but where pointer could be NULL.  This
> +    adds a check against NULL where the regular is_a_helper hook for TYPE
> +    assumes non-NULL.
> +
> +      do_something_with (as_a_nullable <cgraph_node *> *ptr);
> +
>  TYPE dyn_cast <TYPE> (pointer)
>  
>      Converts pointer to TYPE if and only if "is_a <TYPE> pointer".  Otherwise,
> @@ -185,6 +193,22 @@ as_a (U *p)
>    return is_a_helper <T>::cast (p);
>  }
>  
> +/* Similar to as_a<>, but where the pointer can be NULL, even if
> +   is_a_helper<T> doesn't check for NULL.  */
> +
> +template <typename T, typename U>
> +inline T
> +as_a_nullable (U *p)
> +{
> +  if (p)
> +    {
> +      gcc_checking_assert (is_a <T> (p));
> +      return is_a_helper <T>::cast (p);
> +    }
> +  else
> +    return NULL;
> +}
> +
>  /* A generic checked conversion from a base type U to a derived type T.  See
>     the discussion above for when to use this function.  */
>  
> -- 
> 1.8.5.3
>
Richard Biener Aug. 13, 2014, 10:07 a.m. UTC | #3
On Wed, Aug 13, 2014 at 12:01 PM, Martin Jambor <mjambor@suse.cz> wrote:
> Hi,
>
> On Wed, Aug 06, 2014 at 01:19:44PM -0400, David Malcolm wrote:
>> In many circumstances, is_a_helper <T>::test assumes that the pointer is
>> non-NULL, but sometimes you have a pointer of type T that can be NULL.
>>
>> Earlier versions of this patch kit made numerous uses of the ternary
>> operator to handle nullable pointers e.g.:
>>
>>   return insn ? as_a <rtx_insn *> (insn) : NULL;
>>
>> but this was ugly.  Instead, introduce an as_a_nullable<T> variant that
>> adds a check for NULL, so the above can be written simply as:
>>
>>   return as_a_nullable <rtx_insn *> (insn);
>
> A tiny bikeshedding, disregard it if you don't like it: however, the
> "nullable" part of the name suggests the pointer can be NULLed in the
> process.  I think that variants of functions that survive NULL
> arguments are traditionally called "safe" in gcc, so what about
> "safe_as" (or safely_as)?

Ok, I resisted at first starting the bike-shedding but I agree on
the badness of "nullable" (what does that mean anyway?).
as_a_safe or safe_as_a both work for me.

Richard.

>
> Martin
>
>
>>
>> gcc/
>>       * is-a.h (template<T, U> as_a_nullable <U *p>) New function.
>> ---
>>  gcc/is-a.h | 24 ++++++++++++++++++++++++
>>  1 file changed, 24 insertions(+)
>>
>> diff --git a/gcc/is-a.h b/gcc/is-a.h
>> index a14e344..f50e62c 100644
>> --- a/gcc/is-a.h
>> +++ b/gcc/is-a.h
>> @@ -46,6 +46,14 @@ TYPE as_a <TYPE> (pointer)
>>
>>        do_something_with (as_a <cgraph_node *> *ptr);
>>
>> +TYPE as_a_nullable <TYPE> (pointer)
>> +
>> +    Like as_a <TYPE> (pointer), but where pointer could be NULL.  This
>> +    adds a check against NULL where the regular is_a_helper hook for TYPE
>> +    assumes non-NULL.
>> +
>> +      do_something_with (as_a_nullable <cgraph_node *> *ptr);
>> +
>>  TYPE dyn_cast <TYPE> (pointer)
>>
>>      Converts pointer to TYPE if and only if "is_a <TYPE> pointer".  Otherwise,
>> @@ -185,6 +193,22 @@ as_a (U *p)
>>    return is_a_helper <T>::cast (p);
>>  }
>>
>> +/* Similar to as_a<>, but where the pointer can be NULL, even if
>> +   is_a_helper<T> doesn't check for NULL.  */
>> +
>> +template <typename T, typename U>
>> +inline T
>> +as_a_nullable (U *p)
>> +{
>> +  if (p)
>> +    {
>> +      gcc_checking_assert (is_a <T> (p));
>> +      return is_a_helper <T>::cast (p);
>> +    }
>> +  else
>> +    return NULL;
>> +}
>> +
>>  /* A generic checked conversion from a base type U to a derived type T.  See
>>     the discussion above for when to use this function.  */
>>
>> --
>> 1.8.5.3
>>
Jeff Law Aug. 13, 2014, 1:48 p.m. UTC | #4
On 08/13/14 04:07, Richard Biener wrote:
> On Wed, Aug 13, 2014 at 12:01 PM, Martin Jambor <mjambor@suse.cz> wrote:
>> Hi,
>>
>> On Wed, Aug 06, 2014 at 01:19:44PM -0400, David Malcolm wrote:
>>> In many circumstances, is_a_helper <T>::test assumes that the pointer is
>>> non-NULL, but sometimes you have a pointer of type T that can be NULL.
>>>
>>> Earlier versions of this patch kit made numerous uses of the ternary
>>> operator to handle nullable pointers e.g.:
>>>
>>>    return insn ? as_a <rtx_insn *> (insn) : NULL;
>>>
>>> but this was ugly.  Instead, introduce an as_a_nullable<T> variant that
>>> adds a check for NULL, so the above can be written simply as:
>>>
>>>    return as_a_nullable <rtx_insn *> (insn);
>>
>> A tiny bikeshedding, disregard it if you don't like it: however, the
>> "nullable" part of the name suggests the pointer can be NULLed in the
>> process.  I think that variants of functions that survive NULL
>> arguments are traditionally called "safe" in gcc, so what about
>> "safe_as" (or safely_as)?
>
> Ok, I resisted at first starting the bike-shedding but I agree on
> the badness of "nullable" (what does that mean anyway?).
> as_a_safe or safe_as_a both work for me.
Then let's go with that.

David, pick one of those and adjust accordingly.  Hopefully the cases 
where you have to adjust aren't too many.

Adjusting any patch already approved with the name change is pre-approved.


jeff
David Malcolm Aug. 13, 2014, 7:54 p.m. UTC | #5
On Wed, 2014-08-13 at 12:07 +0200, Richard Biener wrote:
> On Wed, Aug 13, 2014 at 12:01 PM, Martin Jambor <mjambor@suse.cz> wrote:
> > Hi,
> >
> > On Wed, Aug 06, 2014 at 01:19:44PM -0400, David Malcolm wrote:
> >> In many circumstances, is_a_helper <T>::test assumes that the pointer is
> >> non-NULL, but sometimes you have a pointer of type T that can be NULL.
> >>
> >> Earlier versions of this patch kit made numerous uses of the ternary
> >> operator to handle nullable pointers e.g.:
> >>
> >>   return insn ? as_a <rtx_insn *> (insn) : NULL;
> >>
> >> but this was ugly.  Instead, introduce an as_a_nullable<T> variant that
> >> adds a check for NULL, so the above can be written simply as:
> >>
> >>   return as_a_nullable <rtx_insn *> (insn);
> >
> > A tiny bikeshedding, disregard it if you don't like it: however, the
> > "nullable" part of the name suggests the pointer can be NULLed in the
> > process.  I think that variants of functions that survive NULL
> > arguments are traditionally called "safe" in gcc, so what about
> > "safe_as" (or safely_as)?
> 
> Ok, I resisted at first starting the bike-shedding but I agree on
> the badness of "nullable" (what does that mean anyway?).
> as_a_safe or safe_as_a both work for me.

I think I took the terminology from:
  http://en.wikipedia.org/wiki/Nullable_type

meaning "something that can be NULL".

"safe" as a prefix seems to be the pattern in the rest of the code, so I
guess I'll use "safe_as_a".

Thanks
Dave


> Richard.
> 
> >
> > Martin
> >
> >
> >>
> >> gcc/
> >>       * is-a.h (template<T, U> as_a_nullable <U *p>) New function.
> >> ---
> >>  gcc/is-a.h | 24 ++++++++++++++++++++++++
> >>  1 file changed, 24 insertions(+)
> >>
> >> diff --git a/gcc/is-a.h b/gcc/is-a.h
> >> index a14e344..f50e62c 100644
> >> --- a/gcc/is-a.h
> >> +++ b/gcc/is-a.h
> >> @@ -46,6 +46,14 @@ TYPE as_a <TYPE> (pointer)
> >>
> >>        do_something_with (as_a <cgraph_node *> *ptr);
> >>
> >> +TYPE as_a_nullable <TYPE> (pointer)
> >> +
> >> +    Like as_a <TYPE> (pointer), but where pointer could be NULL.  This
> >> +    adds a check against NULL where the regular is_a_helper hook for TYPE
> >> +    assumes non-NULL.
> >> +
> >> +      do_something_with (as_a_nullable <cgraph_node *> *ptr);
> >> +
> >>  TYPE dyn_cast <TYPE> (pointer)
> >>
> >>      Converts pointer to TYPE if and only if "is_a <TYPE> pointer".  Otherwise,
> >> @@ -185,6 +193,22 @@ as_a (U *p)
> >>    return is_a_helper <T>::cast (p);
> >>  }
> >>
> >> +/* Similar to as_a<>, but where the pointer can be NULL, even if
> >> +   is_a_helper<T> doesn't check for NULL.  */
> >> +
> >> +template <typename T, typename U>
> >> +inline T
> >> +as_a_nullable (U *p)
> >> +{
> >> +  if (p)
> >> +    {
> >> +      gcc_checking_assert (is_a <T> (p));
> >> +      return is_a_helper <T>::cast (p);
> >> +    }
> >> +  else
> >> +    return NULL;
> >> +}
> >> +
> >>  /* A generic checked conversion from a base type U to a derived type T.  See
> >>     the discussion above for when to use this function.  */
> >>
> >> --
> >> 1.8.5.3
> >>
diff mbox

Patch

diff --git a/gcc/is-a.h b/gcc/is-a.h
index a14e344..f50e62c 100644
--- a/gcc/is-a.h
+++ b/gcc/is-a.h
@@ -46,6 +46,14 @@  TYPE as_a <TYPE> (pointer)
 
       do_something_with (as_a <cgraph_node *> *ptr);
 
+TYPE as_a_nullable <TYPE> (pointer)
+
+    Like as_a <TYPE> (pointer), but where pointer could be NULL.  This
+    adds a check against NULL where the regular is_a_helper hook for TYPE
+    assumes non-NULL.
+
+      do_something_with (as_a_nullable <cgraph_node *> *ptr);
+
 TYPE dyn_cast <TYPE> (pointer)
 
     Converts pointer to TYPE if and only if "is_a <TYPE> pointer".  Otherwise,
@@ -185,6 +193,22 @@  as_a (U *p)
   return is_a_helper <T>::cast (p);
 }
 
+/* Similar to as_a<>, but where the pointer can be NULL, even if
+   is_a_helper<T> doesn't check for NULL.  */
+
+template <typename T, typename U>
+inline T
+as_a_nullable (U *p)
+{
+  if (p)
+    {
+      gcc_checking_assert (is_a <T> (p));
+      return is_a_helper <T>::cast (p);
+    }
+  else
+    return NULL;
+}
+
 /* A generic checked conversion from a base type U to a derived type T.  See
    the discussion above for when to use this function.  */