Fix gcc 4.1 build (#1035) by matthauck · Pull Request #1913 · protocolbuffers/protobuf

matthauck · 2016-08-03T00:59:49Z

No description provided.

grpc-kokoro · 2016-08-03T00:59:50Z

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

googlebot · 2016-08-03T00:59:53Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please let us know the company's name.

matthauck · 2016-08-03T01:01:22Z

This should fix #1035

googlebot · 2016-08-03T21:26:44Z

CLAs look good, thanks!

matthauck · 2017-03-01T00:39:41Z

ping

Is there anything missing here? Any changes requested?

bazel-io · 2017-03-01T00:39:42Z

Can one of the admins verify this patch?

grpc-kokoro · 2017-03-01T00:39:43Z

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.

Fixes "error: object missing in reference to '...'" errors from protocolbuffers#1035

TeBoring · 2017-03-01T01:04:52Z

Can you rebase this pull request on the current master?

matthauck · 2017-03-01T01:06:29Z

Just finished doing that and making sure it still built. :)

TeBoring · 2017-03-01T01:08:43Z

ok to test.

TeBoring · 2017-03-01T18:45:16Z

LGTM

xfxyjwf · 2017-03-22T01:07:27Z

+#else
+      case google::protobuf::FieldDescriptor::CPPTYPE_INT64:
+      case google::protobuf::FieldDescriptor::CPPTYPE_UINT64:
+        GOOGLE_LOG(FATAL) << "Unsupported on this platform.";


This is not the right use of LOG(FATAL). Protobuf library should not crash an user's program given a valid input. If the platform can not be supported, it should be reported at compile time. Though, in this case it seems easy enough to write a custom hash function if it's not already available in the platform headers. @matthauck @TeBoring Do you plan to actually implement this hash for int64? I saw this change while working on integrating github changes to our internal repo and I'm going to leave this change out.

Oh, whoops, sorry. 😞

The thought did not cross my mind to implement this on these platforms. I'd be open to helping out if you could give me some pointers on where to start.

I guess just an empty implementation would be enough. It's better to trigger compile error if the hash for int64 is used.

https://github.com/google/protobuf/blob/master/src/google/protobuf/stubs/hash.h#L184
Here is an example. For int64, how about just trimming?

Okay, opened up two PR's to fix this missing hash: #2906 and #2907. Take your pick which fix you like better. :)

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

Rather than crashing on use (doh!) better to just implement the missing std::tr1::hash structs. Fixes regression introduced by protocolbuffers#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

Rather than crashing on use (doh!) better to just implement the missing std::tr1::hash structs. Fixes regression introduced by protocolbuffers#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 #1913

googlebot added the cla: no label Aug 3, 2016

matthauck mentioned this pull request Aug 3, 2016

Fix gcc 4.1.2 compilation of map_field_inl.h #1912

Closed

matthauck force-pushed the fix-gcc-41 branch from e9075dd to da2f4cc Compare August 3, 2016 21:26

googlebot added cla: yes and removed cla: no labels Aug 3, 2016

matthauck force-pushed the fix-gcc-41 branch from da2f4cc to 2479862 Compare August 3, 2016 21:27

matthauck force-pushed the fix-gcc-41 branch from 2630926 to e44c9d3 Compare August 22, 2016 17:41

TeBoring self-assigned this Feb 8, 2017

matthauck and others added 3 commits February 28, 2017 16:48

Fix gcc 4.1.2 compilation of map_field_inl.h

ea5f0ac

Fixes "error: object missing in reference to '...'" errors from protocolbuffers#1035

Disable 64-bit map keys on gcc <= 4.1

cbe4b27

Add missing case statements

ceebd3e

matthauck force-pushed the fix-gcc-41 branch from e44c9d3 to ceebd3e Compare March 1, 2017 01:05

TeBoring merged commit 6011d7c into protocolbuffers:master Mar 1, 2017

matthauck deleted the fix-gcc-41 branch March 1, 2017 19:17

xfxyjwf reviewed Mar 22, 2017

View reviewed changes

matthauck added a commit to matthauck/protobuf that referenced this pull request Mar 25, 2017

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

1ee8a5e

Rather than crashing on use (doh!) better to just implement the missing std::tr1::hash structs. Fixes regression introduced by protocolbuffers#1913

This was referenced Mar 25, 2017

Fix missing std::tr1::hash on GCC 4.1 (option 1) matthauck/protobuf#1

Closed

Fix missing std::tr1::hash on GCC 4.1 (option 2) matthauck/protobuf#2

Closed

matthauck added a commit to matthauck/protobuf that referenced this pull request Mar 25, 2017

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

c0b760c

Rather than crashing on use (doh!) better to just implement the missing std::tr1::hash structs. Fixes regression introduced by protocolbuffers#1913

This was referenced Mar 25, 2017

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

Closed

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

Merged

matthauck mentioned this pull request Jul 11, 2017

Fix map_field_inl.h for gcc 4.1 #3348

Merged

google-admin unassigned TeBoring Aug 21, 2018

xfxyjwf assigned TeBoring Aug 24, 2018

Conversation

matthauck commented Aug 3, 2016

Uh oh!

grpc-kokoro commented Aug 3, 2016

Uh oh!

googlebot commented Aug 3, 2016

Uh oh!

matthauck commented Aug 3, 2016

Uh oh!

googlebot commented Aug 3, 2016

Uh oh!

matthauck commented Mar 1, 2017

Uh oh!

bazel-io commented Mar 1, 2017

Uh oh!

grpc-kokoro commented Mar 1, 2017

Uh oh!

TeBoring commented Mar 1, 2017

Uh oh!

matthauck commented Mar 1, 2017

Uh oh!

TeBoring commented Mar 1, 2017

Uh oh!

TeBoring commented Mar 1, 2017

Uh oh!

xfxyjwf Mar 22, 2017

Choose a reason for hiding this comment

Uh oh!

matthauck Mar 24, 2017

Choose a reason for hiding this comment

Uh oh!

TeBoring Mar 24, 2017

Choose a reason for hiding this comment

Uh oh!

TeBoring Mar 24, 2017

Choose a reason for hiding this comment

Uh oh!

matthauck Mar 25, 2017

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants