mapbox / geometry.hpp

C++ geometry types
ISC License
92 stars 37 forks source link

Fix compilation errors with libc++ on QNX 7 #59

Closed nagineni closed 5 years ago

nagineni commented 5 years ago

It seems that the QNX 7 compiler (i.e qcc based GCC 5.4.0 with libc++ from LLVM) has a limited c++11 feature support and causing the compilation errors with the inheriting constructors.

compilation errors on QNX 7: workspace/qt5/qtlocation/src/3rdparty/mapbox-gl-native/deps/geometry/0.9.2/include/mapbox/geometry/line_string.hpp: In instantiation of 'struct mapbox::geometry::line_string<double>': workspace/qt5/qtlocation/src/3rdparty/mapbox-gl-native/platform/qt/src/qt_geojson.cpp:21:85: required from here workspace/qt5/qtlocation/src/3rdparty/mapbox-gl-native/deps/geometry/0.9.2/include/mapbox/geometry/line_string.hpp:19:27: error: 'template<class _InputIterator> mapbox::geometry::line_string<double>::line_string(_InputIterator, _InputIterator, const allocator_type&)' inherited from 'std::__1::vector<mapbox::geometry::point<double> >' using container_type::container_type; ^ workspace/qt5/qtlocation/src/3rdparty/mapbox-gl-native/deps/geometry/0.9.2/include/mapbox/geometry/line_string.hpp:19:27: error: conflicts with version inherited from 'std::__1::vector<mapbox::geometry::point<double> >' cc: workspace/qnx700/host/linux/x86_64/usr/lib/gcc/x86_64-pc-nto-qnx7.0.0/5.4.0/cc1plus error 1

This patch fixes the issues by providing the required constructors explicitly.

nagineni commented 5 years ago

@artemp could you please take a look?

jfirebaugh commented 5 years ago

Hi @nagineni, thanks for the contribution. I don't think we can promise ongoing support for compilers with only partial C++11 support, but in this particular case I know that the using base::base pattern has caused issues in some other contexts where we've used it, so I'm open to another solution. However, I believe that the change here will break existing code, which uses std::vector constructors that would no longer be supported after this change -- for example, constructing line_string<T> from a std::vector<point<T>>.

Would you mind trying the following "universal forwarding constructor" pattern instead?

template <class... Args>
line_string(Args&&... args) : container_type(std::forward<Args>(args)...) {}
nagineni commented 5 years ago

Hi @jfirebaugh, thanks a lot for reviewing and suggestions!

Having only 'universal forwarding constructor' was causing errors in case of brace-enclosed list-initialization, so I added both the 'universal forwarding' and 'initializer_list' constructors in the updated patch. Could you please take another look?

Error(s) without having initializer_list constructor:

mapbox-gl-native/src/mbgl/renderer/renderer.cpp: In member function 'std::__1::vector<mapbox::geometry::feature<double> > mbgl::Renderer::queryRenderedFeatures(const ScreenBox&, const mbgl::RenderedQueryOptions&) const':
mapbox-gl-native/src/mbgl/renderer/renderer.cpp:54:5: error: no matching function for call to 'mbgl::Renderer::Impl::queryRenderedFeatures(<brace-enclosed initializer list>, const mbgl::RenderedQueryOptions&)'
     );
     ^
In file included from mapbox-gl-native/src/mbgl/renderer/renderer.cpp:2:0:
mapbox-gl-native/src/mbgl/renderer/renderer_impl.hpp:52:26: note: candidate: std::__1::vector<mapbox::geometry::feature<double> > mbgl::Renderer::Impl::queryRenderedFeatures(const ScreenLineString&, const mbgl::RenderedQueryOptions&) const
     std::vector<Feature> queryRenderedFeatures(const ScreenLineString&, const RenderedQueryOptions&) const;
                          ^
mapbox-gl-native/src/mbgl/renderer/renderer_impl.hpp:52:26: note:   no known conversion for argument 1 from '<brace-enclosed initializer list>' to 'const ScreenLineString& {aka const mapbox::geometry::line_string<double>&}'
....
artemp commented 5 years ago

@jfirebaugh @nagineni - looks good to me. It shouldn't break anything but I can't be 100% cure before trying. I'm ok with merging this PR and loop back if any issues.

nagineni commented 5 years ago

@jfirebaugh yes, those errors are from the QNX 7 compiler which is based on GCC v5.4.0 with libc++ v3.7.x from LLVM.

nagineni commented 5 years ago

@jfirebaugh @artemp thanks again for reviewing. Fixed nit comment and updated the patch.