gcc4.4 fixes: std::tr1::unordered_map

[Oleg Smolsky]

Hey Kenton, attached is a patch that clears "deprecated headers"
warnings emitted when building protobuf with g++4.4.

The fix has two parts:
a) discover and use std::tr1::unordered_map when it is available
b) ensure that string hashing is available and working

I've tested the updated code with g++4.4, g++4.1 and g++3.4.
Unfortunately the last two share the same old crusty libstdc++ due to
the way Redhad built it...

P.S. I've taken a liberty to reformat and re-order declarations in
.../stubs/hash.h while debugging the hash issue.

Oleg.

[Kenton Varda]

Thanks for the fix!  Unfortunately your re-ordering makes it hard for me to see what actually changed.  Also, your style doesn't match the Google style guide.

Is the following sufficient for hash.h, given your changes to stl_hash.m4?

Index: src/google/protobuf/stubs/hash.h

===================================================================

--- src/google/protobuf/stubs/hash.h    (revision 258)

+++ src/google/protobuf/stubs/hash.h    (working copy)

@@ -152,14 +152,14 @@

 template <typename Key, typename Data,

           typename HashFcn = hash<Key>,

           typename EqualKey = std::equal_to<Key> >

-class hash_map : public HASH_NAMESPACE::hash_map<

+class hash_map : public HASH_NAMESPACE::HASH_MAP_CLASS<

     Key, Data, HashFcn, EqualKey> {

 };

 template <typename Key,

           typename HashFcn = hash<Key>,

           typename EqualKey = std::equal_to<Key> >

-class hash_set : public HASH_NAMESPACE::hash_set<

+class hash_set : public HASH_NAMESPACE::HASH_SET_CLASS<

     Key, HashFcn, EqualKey> {

 };

[Kenton Varda]

Apparently not, as tr1::hash<char*> treats values as pointers, not strings.  :/

[Oleg Smolsky]

Hey Kenton, you also need the two updated google::protobuf::hash<>
specializations that ensure correct hashing of "const char *" and
"std::string" keys.

I've attached the complete header, so that you can read it without
having to apply the patch.

Oleg.

On Mon, Dec 21, 2009 at 10:26 AM, Kenton Varda <ken...@google.com> wrote:

[Kenton Varda]

Yes, I see now.  But your solution -- constructing string objects every time the hash function is run -- is very slow.  I've submitted r259 with this implementation instead:

template <>

struct hash<const char*> {

  inline size_t operator()(const char* str) const {

    size_t result = 0;

    for (; *str != '\0'; str++) {

      result = 5 * result + *str;

    }

    return result;

  }

};

[Oleg Smolsky]

Alright, thanks.

Oleg.

On Dec 21, 10:59 am, Kenton Varda <ken...@google.com> wrote:
> Yes, I see now.  But your solution -- constructing string objects every time
> the hash function is run -- is very slow.  I've submitted r259 with this
> implementation instead:
>
> template <>
> struct hash<const char*> {
>   inline size_t operator()(const char* str) const {
>     size_t result = 0;
>     for (; *str != '\0'; str++) {
>       result = 5 * result + *str;
>     }
>     return result;
>   }
>
> };
>

> > > On Wed, Dec 16, 2009 at 9:36 PM, Oleg Smolsky <oleg.smol...@gmail.com>

[Oleg Smolsky]

BTW, you've added this line to stubs/hash.h

#include <ext/hash_map>

but it should not be there. Includes are already handled by these:

#include HASH_MAP_H
#include HASH_SET_H

Oleg.

[Kenton Varda]

Arrghh.  I didn't mean to add that...  I just wrote it so that I could hit F3 and have eclipse show me the file, then forgot to delete it.  Fixed.  Thanks for pointing that out; I'm not normally so sloppy.

[Kenton Varda]

The implementation of tr1::hashtable on OSX 1.5 (GCC 4.0.1) is broken.  find_node() is apparently not declared const, meaning calling find() on a const hash_map does not compile.

/cry

[Kenton Varda]

Worked around with r291.  Must test on all platforms all over again...  sigh.

[Oleg Smolsky]

Oh, funky. Sorry, I don't have a Mac to test. Do you have automated
builds going?

From my experience gcc 4.0.x versions were somewhat buggy, while the
4.1.x branch is reasonable. I am surprised that std::tr1 was actually
present in that STL... Perhaps it is an Apple-brewed combo?

Oleg.

[Kenton Varda]

On Wed, Jan 6, 2010 at 9:02 PM, Oleg Smolsky <oleg.s...@gmail.com> wrote:

Oh, funky. Sorry, I don't have a Mac to test. Do you have automated
builds going?

Only on Linux, unfortunately.  Well, and the Solaris one that Monty runs.  I suppose I should see about setting up an array of automatic builds on different platforms.

 

From my experience gcc 4.0.x versions were somewhat buggy, while the
4.1.x branch is reasonable. I am surprised that std::tr1 was actually
present in that STL... Perhaps it is an Apple-brewed combo?

Dunno, but all is good now.  I just had to complain about it somewhere.  :)

[Monty Taylor]

Kenton Varda wrote:
> On Wed, Jan 6, 2010 at 9:02 PM, Oleg Smolsky <oleg.s...@gmail.com
> <mailto:oleg.s...@gmail.com>> wrote:
>
> Oh, funky. Sorry, I don't have a Mac to test. Do you have automated
> builds going?
>
>
> Only on Linux, unfortunately. Well, and the Solaris one that Monty
> runs. I suppose I should see about setting up an array of automatic
> builds on different platforms.

I'd be happy to set you up a builder on our OSX host if it would be helpful.

>
> From my experience gcc 4.0.x versions were somewhat buggy, while the
> 4.1.x branch is reasonable. I am surprised that std::tr1 was actually
> present in that STL... Perhaps it is an Apple-brewed combo?
>
>
> Dunno, but all is good now. I just had to complain about it somewhere. :)

We force gcc-4.2 on OSX in Drizzle for exactly this reason...

Monty

>
> Oleg.
>
> On Wed, Jan 6, 2010 at 8:44 PM, Kenton Varda <ken...@google.com
> <mailto:ken...@google.com>> wrote:
> > Worked around with r291. Must test on all platforms all over

> again....

> > sigh.
> > On Wed, Jan 6, 2010 at 8:29 PM, Kenton Varda <ken...@google.com
> <mailto:ken...@google.com>> wrote:
> >>
> >> The implementation of tr1::hashtable on OSX 1.5 (GCC 4.0.1) is
> broken.
> >> find_node() is apparently not declared const, meaning calling
> find() on a
> >> const hash_map does not compile.
> >> /cry
> >> On Mon, Dec 21, 2009 at 11:24 AM, Kenton Varda <ken...@google.com
> <mailto:ken...@google.com>> wrote:
> >>>
> >>> Arrghh. I didn't mean to add that... I just wrote it so that I
> could
> >>> hit F3 and have eclipse show me the file, then forgot to delete
> it. Fixed.
> >>> Thanks for pointing that out; I'm not normally so sloppy.
> >>>
> >>> On Mon, Dec 21, 2009 at 11:19 AM, Oleg Smolsky

> <oleg.s...@gmail.com <mailto:oleg.s...@gmail.com>>

> >>> wrote:
> >>>>
> >>>> BTW, you've added this line to stubs/hash.h
> >>>>
> >>>> #include <ext/hash_map>
> >>>>
> >>>> but it should not be there. Includes are already handled by these:
> >>>>
> >>>> #include HASH_MAP_H
> >>>> #include HASH_SET_H
> >>>>
> >>>> Oleg.
> >>>
> >>
> >
> >
>
>
>

> ------------------------------------------------------------------------
>
> --
> You received this message because you are subscribed to the Google
> Groups "Protocol Buffers" group.
> To post to this group, send email to prot...@googlegroups.com.
> To unsubscribe from this group, send email to
> protobuf+u...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/protobuf?hl=en.

[Kenton Varda]

On Wed, Jan 6, 2010 at 9:40 PM, Monty Taylor <mor...@inaugust.com> wrote:

Kenton Varda wrote:
> On Wed, Jan 6, 2010 at 9:02 PM, Oleg Smolsky <oleg.s...@gmail.com

> <mailto:oleg.s...@gmail.com>> wrote:
>
>     Oh, funky. Sorry, I don't have a Mac to test. Do you have automated
>     builds going?
>
>
> Only on Linux, unfortunately.  Well, and the Solaris one that Monty
> runs.  I suppose I should see about setting up an array of automatic
> builds on different platforms.

I'd be happy to set you up a builder on our OSX host if it would be helpful.

Thanks, but I'm going to find out if we have infrastructure for this here first.

 

>
>     From my experience gcc 4.0.x versions were somewhat buggy, while the
>     4.1.x branch is reasonable. I am surprised that std::tr1 was actually
>     present in that STL... Perhaps it is an Apple-brewed combo?
>
>
> Dunno, but all is good now.  I just had to complain about it somewhere.  :)

We force gcc-4.2 on OSX in Drizzle for exactly this reason...

Does Apple provide a gcc-4.2 for OSX 10.5 (Leopard)?