This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: C++ PATCH: throw (T *)NULL and dynamic_cast


Looks good, thanks!  A couple of bugs I see:

 You should use binfo_from_vbase, rather than TREE_VIA_VIRTUAL; the latter
 only checks the most immediate derivation.

 Declaring op== inline in tinfo.cc is invalid; the standard says an inline
 function must be declared inline and defined in all translation units where
 it's used.  Instead, you could use a static helper function in tinfo.cc, or
 just leave it alone.

And some style nits:

 The comment for upcast refers to *RESULT; I think you mean *ADJPTR.

 Please describe in your function comments what meaning (and form, if not
 obvious) the parameters have, not just whether or not they can be null.
 And please do describe the parms and return value of the do_* functions.

 s/heirarchy/hierarchy/
 s/derrived/derived/

 Please use something like

   const type_info *const nonvirtual_base_type
     = (static_cast <const type_info *> (0) + 1);

 rather than write (const void *)1 explicitly.

 Please use inlines to test access paths; something like

   return contained_nonpublic_p (access_path);

 is more readable than

   return (access_path & contained_public) == contained_mask;

 Your comment at the end of class::do_dyncast,

+         // We found SUBOBJ as a private non-virtual base, therefore all
+         // cross casts will fail. We would have already found a down cast,
+         // should there have been one.

 seems to imply that the down cast has also failed at this point, which I
 don't think is true.  I'd suggest

				    We have already found a down cast, if
          // there is one.

 instead.

Jason


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]