Hi all,
at the moment I am trying to fix an unnecessary iterator dereferencing
in Casting.h and am uncovering several ugly things.
For example, look at this code:
// dyn_cast_or_null<X> - Functionally identical to dyn_cast, except that a null
// value is accepted.
//
template <class X, class Y>
inline typename cast_retty<X, Y>::ret_type dyn_cast_or_null(const Y &Val) {
return (Val && isa<X>(Val)) ? cast<X, Y>(Val) : 0;
}
When Y is a pointer type all works perfectly, but when it is a (structured)
iterator we encounter several problems:
1) Val is cast to bool on the LHS of the &&-operator. This is semantically
wrong, as we want to dereference (local jargon: simplify) Val to a
pointer first and compare that. Also Y may have a conversion to bool,
which may or may not coincide with comparison of the dereferenced pointer
against NULL. In this case no compile error will occur, but we may get
some strange behaviour.
2) When Val is an iterator, it will be simplified at least two times, namely
in isa<> and in cast<>.
My suggestion is to simplify Val to a pointer first and then do the
comparison against NULL, the instance check and the cast.
I tried this (using getSimplifiedValue), but got further problems and had to
add a specialization
+template<typename From> struct simplify_type<From*const> {
+ typedef From* SimpleType;
+ static SimpleType &getSimplifiedValue(From*const &Val) {
+ return const_cast<From*&>(Val);
+ }
+};
Is there a simpler way to accomplish this?
Cheers,
Gabor
Is this a recommended approach/good style/good idea to use
dyn_cast_or_null<X>(I) instead of dyn_cast_or_null<X>(*I)? (And other
is and cast functions).
Eugene
Is this a recommended approach/good style/good idea to use
dyn_cast_or_null<X>(I) instead of dyn_cast_or_null<X>(*I)? (And other
is and cast functions).
We generally don't want auto-dereference. There is some special magic for unwrapping Uses etc though. Is this what you're in contact with Gabor?
-Chris
I agree with Chris, that auto-dereferencing is a bad thing (esp. because of the gothcas I detailed).
I checked in a bunch of fixes to remove auto-dereferencing.
Maybe we should get rid of the type simplifier for (const_)use_iterator? Sabre what do you think?
There is one thing that is still irritating me:
dyn_cast_or_null and cast_or_null have different interfaces. Should these be made consistent?
Cheers,
Gabor
Is this a recommended approach/good style/good idea to use
dyn_cast_or_null(I) instead of dyn_cast_or_null(*I)? (And other
is and cast functions).
We generally don’t want auto-dereference. There is some special magic for unwrapping Uses etc though. Is this what you’re in contact with Gabor?
-Chris
I agree with Chris, that auto-dereferencing is a bad thing (esp. because of the gothcas I detailed).
I checked in a bunch of fixes to remove auto-dereferencing.
Maybe we should get rid of the type simplifier for (const_)use_iterator? Sabre what do you think?
Sounds great to me!
Okay, soon 
There is one thing that is still irritating me:
dyn_cast_or_null and cast_or_null have different interfaces. Should these be made consistent?
What do you mean?
Observe:
… cast_or_null(Y *Val)
… dyn_cast_or_null(const Y &Val)
I propose:
… dyn_cast_or_null(Y *Val)
Cheers,
Gabor
Sure, works for me. This shouldn't affect the interface they vend, right?
-Chris