> Hello Jan,
> 
> Just some casual notes.

Thank you! I will fix the typos shortly!
> > +static inline bool
> > +polymorphic_type_binfo_p (tree binfo)
> > +{
> > +  /* See if BINFO's type has an virtual table associtated with it.  */
> > +  return BINFO_VTABLE (TYPE_BINFO (BINFO_TYPE (binfo)));
> 
> Just for my education, why not just:
> 
>     return BINFO_VTABLE (binfo);
> ?
> 
> At the very least, I'd say there ought to be a comment here explaining
> why this "gotcha'.

I tried to explain in the toplevel comment, but I guess it should be repeated
here.  Every polymorphic type has multiple BINFOs.  One represents the type
itself and others represents the variants present as basetypes of its deriverd
types. Those do not neccesarily have BINFO_VTABLE set, so I want to look
up the basetype's BINFO.

> > +  /* Walk bases.  */
> > +  for (i = 0; BINFO_BASE_ITERATE (binfo, i, base_binfo); i++)
> > +    /* Walking bases that have no virtual method is pointless excercise.  
> > */
> > +    if (polymorphic_type_binfo_p (base_binfo))
> > +      record_binfo (nodes, base_binfo, otr_type,
> > +               BINFO_VTABLE (base_binfo) ? base_binfo : type_binfo,
> 
> ... here, as polymorphic_type_binfo_p (base_binfo) is true,
> shouldn't BINFO_VTABLE(base_info) be unconditionally true as well?

It is the same thing here.  In the case of single inheritance, the BINFOs
representing bases do not have VTABLE, since their VTABLE is shared with
the main VTABLE of the type we walk.

Honza
> 
> 
> > +               otr_token, inserted,
> > +               matched_vtables);
> > +}
> 
> Thanks.
> 
> -- 
>               Dodji

Reply via email to