Re: Use conditional casting with symtab_node

[Lawrence Crowl <crowl at googlers dot com>]

On 10/12/12, Richard Biener <richard.guenther@gmail.com> wrote:
> On Oct 11, 2012 Xinliang David Li <davidxl@google.com> wrote:
>> On Oct 11, 2012 Lawrence Crowl <crowl@googlers.com> wrote:
>>> On 10/10/12, Xinliang David Li <davidxl@google.com> wrote:
>>>> In a different thread, I proposed the following alternative to
>>>> 'try_xxx':
>>>>
>>>> template<typename T> T* symbol::cast_to(symbol* p) {
>>>>    if (p->is<T>())
>>>>       return static_cast<T*>(p);
>>>>    return 0;
>>>>  }
>>>>
>>>> cast:
>>>>
>>>> template<typename T> T& symbol:as(symbol* p) {
>>>>    assert(p->is<T>())
>>>>    return static_cast<T&>(*p);
>>>>
>>>>  }
>>>
>>> My concern here was never the technical feasibility, nor the
>>> desirability of having the facility for generic code, but the amount
>>> of the amount of typing in non-generic contexts.  That is
>>>
>>>   if (cgraph_node *q = p->try_function ())
>>>
>>> versus
>>>
>>>   if (cgraph_node *q = p->cast_to <cgraph_node *> ())
>>>
>>> I thought the latter would generate objections.  Does anyone object
>>> to the extra typing?
>>>
>>> If not, I can make that change.
>>
>> Think about a more complex class hierarchy and interface consistency.
>> I believe you don't want this:
>>
>> tree::try_decl () { .. }
>> tree::try_ssa_name () { ..}
>> tree::try_type() {...}
>> tree::try_int_const () {..}
>> tree::try_exp () { ...}
>>  ..
>>
>> Rather one (or two with the const_cast version).
>>
>> template <T> T *tree::cast_to ()
>> {
>>    if (kind_ == kind_traits<T>::value)
>>     return static_cast<T*> (this);
>>
>>  return 0;
>> }
>
> I also think that instead of
>
>>>   if (cgraph_node *q = p->cast_to <cgraph_node *> ())
>
> we want
>
>   if ((q = cast_to <cgraph_node *> (p))
>
> I see absolutely no good reason to make cast_to a member, given
> that the language has static_cast, const_cast and stuff.  cast_to
> would simply be our equivalent to dynamic_cast within our OO model.

Sorry, that was a think-o.  Agreed there is no point in it being a
member.

> Then I'd call it *_cast instead of cast_*, so, why not gcc_cast < >?
> Or dyn_cast <> ().  That way
>
>   if ((q = dyn_cast <function *> (p))
>
> would be how to use it.

Which function?  The point with my original proposal is that the
kind of function was derivable from context, and thus can be shorter.

> Small enough, compared to
>
>   if ((q = p->try_function ()))
>
> and a lot more familiar to folks knowing C++ (and probably doesn't
> make a difference to others).
>
> Thus, please re-use or follow existing concepts.

Both are existing concepts.  What I proposed is a common technique
for avoiding the cost of dynamic_cast when down casting in a class
hierarchy.  Both concepts will work.  I proposed what I thought
would be most convenient to programmers.  However, I will change
to the other form unless someone objects.

> As for the example with tree we'd then have
>
>   if ((q = dyn_cast <INTEGER_CST> (p)))
>
> if we can overload on the template parameter kind (can we?
> typename vs.  enum?)

There are two template parameters, one for the argument type and
one for the return type.  We can overload on the argument type but
not on the return type.  We can, however, (indirectly) partially
specialize on the return type.  We need to do that anyway to
represent the fact that not all type conversions are legal.

However, I recommend against specializing on the enum, as it would
become somewhat obscure when the hierarchy is discriminated on more
than one enum.

> or use tree_cast <> (no I don't want dyn_cast <tree_common>
> all around the code).

Once we have proper class hierarchies, derived to base conversions
are implicit.  In the meantime, we need something else.

-- 
Lawrence Crowl