Richard Biener <richard.guent...@gmail.com> writes: | On Fri, Mar 22, 2013 at 1:44 PM, Gabriel Dos Reis <g...@axiomatics.org> wrote: | > Richard Biener <richard.guent...@gmail.com> writes: | > | > [...] | > | > | > in the C++ front-end. identifier_p is effectively LANG_IDENTIFIER_CAST | > | > except that it returns a typed pointer instead of a boolean value. | > | | > | Hm? So you are replacing TREE_CODE (t) == IDENTIFIER_NODE | > | with kind-of dynamic_cast<identifier> (t) (in C++ terms)? | > | > It would be a mistake to name it dynamic_cast or anything like cast (I | > know it is popular in certain C++ circles to name everything xxx_cast), | > because dynamic_cast is an implementation detail. I should probably | > have called it "give_me_identifier_pointer_if_operand_points_to_a_cxx_identifier_object" | > | > | Then | > | naming it identifier_p is bad. We have is-a.h now, so please try to use | > | a single style of C++-style casting throughout GCC. | > | > I strongly agree with consistent style, On the other hand, isn't someone going | > to object that is_xxx has a predicate connotation therefore a bad naming | > because it isn't returning a bool? | > | > I think a naming that focuses too much on implementation detail is no good, | | Neither is one that is confusing ;)
You make good points below. I have one quibble -- since I was provoked into bike shedding :-) | | That said - your specific identifier case should be generalized. The cgraph | people had exactly the same issue, given a symtab * return a cgraph * | if the symbol is a function, otherwise NULL, combining the previous | if (symbol == function) fn = get-me-a-function (symbol) | | And they invented is-a.h as we settled for a template approach which | more readily mimics what the C++ language provides (in form of | static_cast<>, dynamic_cast<>, etc.). | | The tree node case is subtly different because we are not actually casting | anything (you are not returning a more specific type than tree) but still | you provide a more C++-ish form to the standard tree.h interfaces. | | That is, plain use of is-a.h is possible for your case: | | template <> | inline lang_identifier * | as_a (tree p) | { | if (TREE_CODE (p) == IDENTIFIER_NODE) | return (lang_identifier *)p; | return NULL; | } | | following existing GCC style (and yes, we've bikeshedded over that | already). I would have dropped the suffix "_a" on both "is_a" and "as_a", did I get a chance to bike shed when is-a.h was introduced. I will add the specialization and use it the appropriate places. | | Thus you'd write | | as_a<lang_identifier> (id) | | instead of your | | identifier_p (id) | | (which should have been lang_identifier_p instead). -- Gaby