Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix missing std::tr1::hash on GCC 4.1 #2907

Merged
merged 2 commits into from
Jul 10, 2017

Conversation

matthauck
Copy link
Contributor

Rather than crashing on use (doh!) better to just declare this platform
is missing a proper hash_map/hash_set implementation and use
the std::map/std::set emulation.

Fixes regression introduced by #1913

Rather than crashing on use (doh!) better to just declare this platform
is missing a proper hash_map/hash_set implementation and use
the std::map/std::set emulation.

Fixes regression introduced by protocolbuffers#1913
@grpc-kokoro
Copy link

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

1 similar comment
@grpc-kokoro
Copy link

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

@bazel-io
Copy link

Can one of the admins verify this patch?

@xfxyjwf
Copy link
Contributor

xfxyjwf commented Apr 7, 2017

Closing as this seems a duplicate of #2906

@xfxyjwf xfxyjwf closed this Apr 7, 2017
@matthauck
Copy link
Contributor Author

Yes, this was an intentional duplicate (see comment on #1913). I wasn't sure which implementation would be preferred.

@xfxyjwf
Copy link
Contributor

xfxyjwf commented Apr 7, 2017

I see. I actually prefer the fallback to approach in this PR. The other PR casts int64 to size_t for hash and that is too week a hash function that invites hash flooding for 32bit systems where size_t is only 32bit.

@xfxyjwf xfxyjwf reopened this Apr 7, 2017
@grpc-kokoro
Copy link

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

1 similar comment
@grpc-kokoro
Copy link

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

@bazel-io
Copy link

bazel-io commented Apr 7, 2017

Can one of the admins verify this patch?

@xfxyjwf
Copy link
Contributor

xfxyjwf commented Apr 7, 2017

ok to test

@matthauck
Copy link
Contributor Author

looks like there are conflicts here now. 😢

@bazel-io
Copy link

Can one of the admins verify this patch?

@matthauck
Copy link
Contributor Author

ok. updated with master. Is there anything left pending on this PR to pull this in?

@matthauck
Copy link
Contributor Author

ping.

@xfxyjwf xfxyjwf merged commit 9ab7c73 into protocolbuffers:master Jul 10, 2017
@matthauck matthauck deleted the fix-missing-hash-1 branch July 11, 2017 00:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants