C++ PATCH: use identifier_p instead of naked TREE_CODE == IDENTIFIER_NODE
Gabriel Dos Reis
Fri Mar 22 14:52:00 GMT 2013
Richard Biener <firstname.lastname@example.org> writes:
| On Fri, Mar 22, 2013 at 1:44 PM, Gabriel Dos Reis <email@example.com> wrote:
| > Richard Biener <firstname.lastname@example.org> 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
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).
More information about the Gcc-patches