This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Fwd: [PATCH] Fix may_alias canonical types regression


Hi Mark,

On Sun, Sep 28, 2008 at 2:00 PM, Mark Mitchell <mark@codesourcery.com> wrote:
> Doug Gregor wrote:
>
>> That doesn't matter here. type_hash_eq isn't about type equality in
>> the language or the middle-end, it's about type equality from the
>> perspective of the hash table. It's used by type_hash_canon to ask
>> "did we already build a node like this?" It actually makes perfect
>> sense to check TYPE_NAME all the time
>
> I think that's a reasonable argument -- but you are using the hash table
> for type compatibility.  Or, rather, incompatibility.
>
> If names matter for the hash table, then they should matter for all
> types; not just for INTEGER_TYPEs.

Agreed.

> Otherwise, we should find some other
> way to ensure that incompatible types are considered different in the
> hash table.  My suggestion is that we take this second approach, but the
> first approach (names always matter) is also plausible.  However,
> TYPE_NAME is sometimes a TYPE_DECL (not just an TREE_IDENTIFIER), so
> that introduces some complexity.

It's actually good that TYPE_NAME is a TYPE_DECL, e.g., in the case of
typedefs, because we wouldn't want N1::foo and N2::foo to be
considered to have the same name when the former is a typedef for
'int' and the latter is a typedef for 'wchar_t'.

Anyway, I tried moving the language-specific type_hash_eq checks
earlier, but there are two problems. First, the language-specific
versions are only designed to handle the leftovers from the main
type_hash_eq. The C++ version, for example, only handle function
types:

bool
cxx_type_hash_eq (const_tree typea, const_tree typeb)
{
  gcc_assert (TREE_CODE (typea) == FUNCTION_TYPE);

  return comp_except_specs (TYPE_RAISES_EXCEPTIONS (typea),
			    TYPE_RAISES_EXCEPTIONS (typeb), 1);
}

The second, larger problem is that I've already concluded once that
one can't properly tell the difference between two INTEGER_TYPEs using
what's in the tree, without canonical types, which means that
type_hash_eq just can't do the right thing regarding type equality. I
while ago, I wrote this comment in cp/typeck.c (structural_comptypes):

    case INTEGER_TYPE:
    case FIXED_POINT_TYPE:
    case REAL_TYPE:
      /* With these nodes, we can't determine type equivalence by
	 looking at what is stored in the nodes themselves, because
	 two nodes might have different TYPE_MAIN_VARIANTs but still
	 represent the same type.  For example, wchar_t and int could
	 have the same properties (TYPE_PRECISION, TYPE_MIN_VALUE,
	 TYPE_MAX_VALUE, etc.), but have different TYPE_MAIN_VARIANTs
	 and are distinct types. On the other hand, int and the
	 following typedef

           typedef int INT __attribute((may_alias));

	 have identical properties, different TYPE_MAIN_VARIANTs, but
	 represent the same type.  The canonical type system keeps
	 track of equivalence in this case, so we fall back on it.  */

So, I'm attaching a slightly tweaked version of the original patch,
which does the TYPE_NAME check for all types except COMPLEX_TYPE
(which sets TYPE_NAME too late for the check to work). I really don't
think we can do any better than this without introducing some extra
information into INTEGER_TYPE nodes to tell 'int' and 'wchar_t' apart.
The patch passes bootstrap+test for C, C++, Objective C, Objective
C++, Java, and Ada.

  - Doug

2008-10-01  Douglas Gregor  <doug.gregor@gmail.com>

	PR c++/37553
	* tree.c (build_type_attribute_qual_variant): Hash on the
	unqualified type, and don't overwrite an existing
	(type_hash_eq): Make the TYPE_NAME of the types significant, to
	allow distinguishing between wchar_t and its underlying type. This
	also means that we'll retain a little more typedef information.

2008-10-01  Douglas Gregor  <doug.gregor@gmail.com>

	PR c++/37553
	* g++.dg/ext/alias-canon2.C: New.

Attachment: canon-may-alias.patch
Description: Binary data


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]