This is the mail archive of the
mailing list for the GCC project.
Re: C++ PATCH: use identifier_p instead of naked TREE_CODE == IDENTIFIER_NODE
- From: Gabriel Dos Reis <gdr at axiomatics dot org>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: Richard Biener <richard dot guenther at gmail dot com>, gcc-patches at gcc dot gnu dot org, jason at redhat dot com
- Date: Fri, 22 Mar 2013 07:35:41 -0500
- Subject: Re: C++ PATCH: use identifier_p instead of naked TREE_CODE == IDENTIFIER_NODE
- References: <87vc8kjdou dot fsf at euclid dot axiomatics dot org> <CAFiYyc2R406UDN=v93Dr76f8GkpvVG_boCi=ryU2QS07+Uz2BQ at mail dot gmail dot com> <20130322093008 dot GV12913 at tucnak dot redhat dot com>
Jakub Jelinek <firstname.lastname@example.org> writes:
| On Fri, Mar 22, 2013 at 10:21:05AM +0100, Richard Biener wrote:
| > On Fri, Mar 22, 2013 at 4:50 AM, Gabriel Dos Reis <email@example.com> wrote:
| > >
| > > This patch introduces identified_p (t) in lieu of
| > >
| > > TREE_CODE (t) == IDENTIFIER_NODE
| > Generally we have macros like IDENTIFIER_P for this kind of checks.
I know we have macros for this kind of test, and what what I can tell
looking at the source code, nobody actually bothered using it.
The point of the change isn't to do that test exactly. As I explained
earlier, the change was prompted by looking at patterns of usage inf the
existing source code: it is very frequent that we would test XXX_P (t)
and immediately do XXX_GET_YYY (t) when XXX_GET_YYY will do a XXX_CHECK (t),
which morally tests again XXX_P (t), and this is all over the places.
The reason why XXX_GET_YYY (t) does XXX_CHECK (t) is because it could be
mistakenly used on a different node since t isn't "typed".
| > > 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)? 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.
| Anyway, if you see better code generated with your change compared to the
| old style, I'd say you should file a PR with some preferrably short
| preprocessed routine where it makes a difference, because that would show
| that either VRP or other optimization pass just isn't doing good job.
We are talking about files in cp/ right? They don't know what "short"
With the next change, I'll try to find a short file, although currently
I am working on cp/name-lookup.c
| It is better code for --enable-checking=yes anyway, right?