Hi Mikael,
On 2012-08-19 19:57, Mikael Morin wrote:
My main concern is this: the increased type safety by changing the (void*) -> (gfc_bbt*) change is balanced by the reduced type safety for all the types inherited from gfc_bbt as the left and right pointer have now gfc_bbt type instead of the derived type. And we better have type safety for gfc_symtree which is used all over the place.
An interesting and valid point. I don't think there's a way around this without using templates (or macros). In a way, inheriting from gfc_bbt is also not really the purpose of inheritance, which is meant to express an "any A is also a B" relationship. An object that can be stored in a tree is not the same as the tree itself. This distinction is not made by my patch. Since my interest was purely to get an equivalent alternative to the macro magic and void*'s, I wasn't too worried about this, but it's another wart.
After digging some information on the web about C++ and casts, I'm not even sure the casts are correct because of the following quote from the standard: "The order in which the base class subobjects are allocated in the most derived object (1.8) is unspecified." This tells me we shouldn't rely on the gfc_symtree, pointer_info, etc having the same address as the corresponding gfc_bbt they derive from (unless the explicit casts add/substract the necessary offset if needed though). We're probably safe with single inheritance, but still...
To my knowledge it's perfectly valid to cast from a base class to a derived class, even though 'static_cast<>' would be the preferred C++ operator for the conversion. The resulting pointer may only be dereferenced if the types of the objects in memory are correct, though. The cast can take care of the issues with different offsets, multiple inheritance and so on because the compiler knows the whole class hierarchy.
Anyway, all this is academic, as the patch brings no benefit IMO. Cheers, - Tobi
