add #include <vector> to a protobuf header to workaround problems on Android

RESOLVED FIXED in Firefox 42

Status

()

Toolkit
General
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: froydnj, Assigned: froydnj)

Tracking

unspecified
mozilla42
Points:
---

Firefox Tracking Flags

(firefox42 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
Compiling Gecko with libc++ and GCC 4.9 on Android runs into a problem.
The protobuf #includes and libc++ result in preprocessed code that looks
something like:

/* via <iterator> */

namespace std {
namespace __1 {
}
using namespace __1 __attribute__((__strong__));
}

namespace std { namespace __1 {
template <class _Iter>
class __wrap_iter
{
  ...
  template <class _Tp, class _Alloc> friend class vector;
  ...
};

} // namespace __1
} // namespace std

/* via <vector> */

namespace std { namespace __1 {

template <class _Tp, class _Alloc>
class _LIBCPP_TYPE_VIS_ONLY vector : ...
{ ... };

} // namespace __1
} // namespace std

and the problem is that GCC doesn't understand that the forward
declaration of vector inside __wrap_iter is forward-declaring the actual
vector class; it thinks it's declaring something else.

Hacking <iterator> to include _LIBCPP_TYPE_VIS_ONLY for the forward
declaration doesn't help.  What does help is including <vector> earlier
than <iterator>, so the __wrap_iter forward declaration picks up the
correct definition of std::vector, and makes everything happy.  It's
possible that there are other places that could get this same treatment,
but this one place was the only one I needed to modify to make things
go.
(Assignee)

Comment 1

2 years ago
Created attachment 8637383 [details] [diff] [review]
add #include <vector> to a protobuf header to workaround problems on Android

fitzgen was the last person to touch protobuf stuff, so he gets to review this!

I'll update the Bug XXXXXX bits in the actual patch to reflect what bug # this
is before committing.  I won't pretend this is a pretty solution, but it is
minimally invasive, and the m-c-changes.patch appears to have prior art in its
modifications to wire_format_lite.h.
Attachment #8637383 - Flags: review?(nfitzgerald)
(Assignee)

Updated

2 years ago
Blocks: 1164921
Comment on attachment 8637383 [details] [diff] [review]
add #include <vector> to a protobuf header to workaround problems on Android

Review of attachment 8637383 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the detailed comments! With that provided context, this seems rather straightforward.
Attachment #8637383 - Flags: review?(nfitzgerald) → review+

Comment 3

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/3caca87b85d7
https://hg.mozilla.org/mozilla-central/rev/3caca87b85d7
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox42: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.