Re: C++ PATCH: use identifier_p instead of naked TREE_CODE == IDENTIFIER_NODE

[Gabriel Dos Reis <gdr at axiomatics dot org>]

Richard Biener <richard.guenther@gmail.com> writes:

| On Fri, Mar 22, 2013 at 1:44 PM, Gabriel Dos Reis <gdr@axiomatics.org> wrote:
| > Richard Biener <richard.guenther@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