Jakub Jelinek <ja...@redhat.com> 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 <g...@axiomatics.org> 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" means:-) 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? Yes. -- Gaby
