Re: Patch to change hashtab.h hashval_t to size_t

["Kaveh R. Ghazi" <ghazi at caip dot rutgers dot edu>]

 > From: DJ Delorie <dj@redhat.com>
 > 
 > > Ok to install?
 > 
 > No.
 > 
 > > -typedef unsigned int hashval_t;
 > > +typedef size_t hashval_t;
 > 
 > Changing the size of hashval_t would break backward compatibility,
 > especially on LP64 systems.  IMHO if gcc is getting a warning because
 > it's casting a pointer to an integer, gcc is doing something wrong and
 > it's up to gcc to work around it.
 > 
 > > Using size_t instead seems to be the generally accepted way within
 > > our sources to ensure casting between pointers and integers aren't
 > > truncated.
 > 
 > The hash functions will truncate it anyway, so this argument seems
 > very weak to me.
 > 
 > Plus this header is used for more than just gcc - it's also used for
 > bfd, which is often installed as a shared object.  You can't just make
 > arbitrary changes to files in include/*.h.

Your points are certainly valid.  However I'm still concerned that the
truncation may result in poor hash values.  If we're not going to
address this in libiberty, then perhaps we should address it in gcc
itself like you said.

If the pointer bits are important, I can come up with a patch in gcc
to XOR the high and low bits of the pointer before casting it to
hashval_t.  If they're not important, then I can just cast to size_t
before casting it to hashval_t.

The places where this comes up are:

two hashval_t casts in cp/decl.c:typename_hash()
all three uses of JAVA_TREEHASHHASH_H in java/class.c
once in function.c:insns_for_mem_hash()

If you or someone else could please let me know if the truncation here
is harmless, I'd appreciate it.

		Thanks,
		--Kaveh
--
Kaveh R. Ghazi			ghazi@caip.rutgers.edu