Re: [pph] Stream TREE_TYPE for identifier node (issue4550121)

[Steven Bosscher <stevenb dot gcc at gmail dot com>]

On Wed, Jun 8, 2011 at 4:12 PM, Diego Novillo wrote:
>>>> That seems unlikely, as identifiers do not have a type. There is some
>>>> TREE_TYPE abuse in cp-tree.h, perhaps you should find out what you're
>>>> streaming.
>>>
>>> It's used by the C++ parser, so it needs to be streamed in pph.
>>
>> Yes, but you should not stream TREE_TYPE but use the accessor macro
>> that uses TREE_TYPE. Otherwise, if someone gets around to making
>> IDENTIFIER nodes non-typed trees (and update cp-tree.h accordingly)
>> the streaming will break.
>
> It does, actually:
>
> cp/rtti.c:
> bool
> emit_tinfo_decl (tree decl)
> {
> ?tree type = TREE_TYPE (DECL_NAME (decl));

Well, I wasn't saying that TREE_TYPE isn't used, just that it is not
actually TREE_TYPE.

Not very thorough, but consider this:

stevenb@gcc17:~/devel/trunk/gcc$ egrep "TREE_TYPE.*DECL_NAME" *.[ch]
{fortran,java,ada/gcc-interface,cp,objc}/*.[ch]
fortran/f95-lang.c:     TYPE_NAME (TREE_TYPE (decl)) = DECL_NAME (decl);
cp/cp-tree.h:  (DECL_CONV_FN_P (FN) ? TREE_TYPE (DECL_NAME (FN)) : NULL_TREE)
cp/decl2.c:       tree underlying_type = TREE_TYPE (DECL_NAME (decl));
cp/decl2.c:         (decl, type_visibility (TREE_TYPE (DECL_NAME (decl))));
cp/decl2.c:           && CLASS_TYPE_P (TREE_TYPE (DECL_NAME (decl)))
cp/decl2.c:           && !CLASSTYPE_VISIBILITY_SPECIFIED (TREE_TYPE
(DECL_NAME (decl))))
cp/decl2.c:      tree type = TREE_TYPE (DECL_NAME (decl));
cp/decl.c:        if (TREE_TYPE (DECL_NAME (decl)) && TREE_TYPE (decl) != type)
cp/repo.c:      type = TREE_TYPE (DECL_NAME (decl));
cp/rtti.c:  tree type = TREE_TYPE (DECL_NAME (decl));

The one in fortran looks like a mistake (this is in pushdecl which was
copy-and-pasted long ago from treelang or something). The
documentation in doc/generic.text is pretty clear about it: TYPE_NAME
of a TYPE_DECL is not an IDENTIFIER node. There should probably be a
checker for that, some kind of negative tree code check perhaps...

The rest are all in cp/.  It looks like g++ uses TREE_TYPE as a cache
for name lookups. Perhaps Jason can comment. Obviously not a front end
I know very well, but let's look at them one at a time:

cp/cp-tree.h:  (DECL_CONV_FN_P (FN) ? TREE_TYPE (DECL_NAME (FN)) : NULL_TREE)

Apparently g++ puts the type of an operator in TREE_TYPE of an
IDENTIFIER_NODE. This should probably be using
REAL_IDENTIFIER_TYPE_VALUE() instead of TREE_TYPE().


cp/decl.c:        if (TREE_TYPE (DECL_NAME (decl)) && TREE_TYPE (decl) != type)

This is in a warning for a type declaration shadowing a local or class
scope.  Should use identifier_type_value (or
REAL_IDENTIFIER_TYPE_VALUE but I think that's supposed to be used
directly only in name-lookup.c??)


cp/decl2.c:       tree underlying_type = TREE_TYPE (DECL_NAME (decl));
cp/decl2.c:         (decl, type_visibility (TREE_TYPE (DECL_NAME (decl))));
cp/decl2.c:           && CLASS_TYPE_P (TREE_TYPE (DECL_NAME (decl)))
cp/decl2.c:           && !CLASSTYPE_VISIBILITY_SPECIFIED (TREE_TYPE
(DECL_NAME (decl))))
cp/decl2.c:      tree type = TREE_TYPE (DECL_NAME (decl));
cp/repo.c:      type = TREE_TYPE (DECL_NAME (decl));
cp/rtti.c:  tree type = TREE_TYPE (DECL_NAME (decl));

All of these are covered by a check on DECL_TINFO_P. I am not sure
what that means but probably these should also be using
identifier_type_value or REAL_IDENTIFIER_TYPE_VALUE instead of
TREE_TYPE. Jason?


Anyway, it seems that there are already a lot of places where
TREE_TYPE is used instead of a separate accessor macro for this
overloaded meaning of TREE_TYPE on IDENTIFIER_NODEs. That is no reason
to further confuse things with pph. It seems to me that in the long
run tree_identifier could (should) be made a non-tree_typed tree...

Would you be willing to try if it is sufficient to only stream
REAL_IDENTIFIER_TYPE_VALUE?

Ciao!
Steven