Closed Bug 1186561 Opened 5 years ago Closed 4 years ago

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


(Toolkit :: General, defect)

Not set



Tracking Status
firefox42 --- fixed


(Reporter: froydnj, Assigned: froydnj)




(1 file)

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
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)
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+
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.