Message ID | 1407345815-14551-6-git-send-email-dmalcolm@redhat.com |
---|---|
State | New |
Headers | show |
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
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 >
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 >>
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
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 --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. */