Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[Qt] Use -fvisibility=hidden for gcc > 6.3 #10499

Closed
wants to merge 1 commit into from
Closed

[Qt] Use -fvisibility=hidden for gcc > 6.3 #10499

wants to merge 1 commit into from

Conversation

oxidase
Copy link

@oxidase oxidase commented Nov 18, 2017 •
edited by kkaefer
Loading

For gcc 6.3 building with default settings errors with

[ 29%] Building CXX object CMakeFiles/mbgl-core.dir/src/mbgl/style/parser.cpp.o
/home/miha/mapbox/mapbox-gl-native/build/launch-cxx /usr/bin/c++   -DQT_CORE_LIB -DQT_GUI_LIB -DQT_NO_DEBUG -DQT_OPENGL_LIB -DQT_WIDGETS_LIB -DRAPIDJSON_HAS_STDSTRING=1 -DUCHAR_TYPE=char16_t -D_GLIBCXX_USE_CXX11_ABI=0 -I/home/miha/mapbox/mapbox-gl-native/include -I/home/miha/mapbox/mapbox-gl-native/src -I/home/miha/mapbox/mapbox-gl-native/mason_packages/headers/geometry/0.9.2/include -I/home/miha/mapbox/mapbox-gl-native/mason_packages/headers/variant/1.1.4/include -I/home/miha/mapbox/mapbox-gl-native/mason_packages/headers/any/8fef1e9/include -I/home/miha/mapbox/mapbox-gl-native/mason_packages/headers/unique_resource/cba309e/include -I/home/miha/mapbox/mapbox-gl-native/mason_packages/headers/rapidjson/1.1.0/include -I/home/miha/mapbox/mapbox-gl-native/mason_packages/headers/boost/1.62.0/include -I/home/miha/mapbox/mapbox-gl-native/mason_packages/headers/geojson/0.4.2/include -I/home/miha/mapbox/mapbox-gl-native/mason_packages/headers/geojsonvt/6.3.0/include -I/home/miha/mapbox/mapbox-gl-native/mason_packages/headers/supercluster/0.2.2/include -I/home/miha/mapbox/mapbox-gl-native/mason_packages/headers/kdbush/0.1.1-1/include -I/home/miha/mapbox/mapbox-gl-native/mason_packages/headers/earcut/0.12.4/include -I/home/miha/mapbox/mapbox-gl-native/mason_packages/headers/protozero/1.5.2/include -I/home/miha/mapbox/mapbox-gl-native/mason_packages/headers/polylabel/1.0.3/include -I/home/miha/mapbox/mapbox-gl-native/mason_packages/headers/wagyu/0.4.3/include -I/home/miha/mapbox/mapbox-gl-native/mason_packages/headers/shelf-pack/2.1.1/include -I/home/miha/mapbox/mapbox-gl-native/mason_packages/headers/vector-tile/1.0.0-rc7/include -I/home/miha/mapbox/mapbox-gl-native/platform/default -I/home/miha/mapbox/mapbox-gl-native/platform/qt -I/home/miha/mapbox/mapbox-gl-native/platform/qt/include -I/home/miha/mapbox/mapbox-gl-native/mason_packages/linux-x86_64/libjpeg-turbo/1.5.0/include -I/home/miha/mapbox/mapbox-gl-native/mason_packages/linux-x86_64/libpng/1.6.25/include/libpng16 -I/home/miha/mapbox/mapbox-gl-native/mason_packages/linux-x86_64/webp/0.5.1/include -I/home/miha/mapbox/mapbox-gl-native/mason_packages/linux-x86_64/icu/58.1-min-size/include -isystem /usr/include/x86_64-linux-gnu/qt5 -isystem /usr/include/x86_64-linux-gnu/qt5/QtCore -isystem /usr/lib/x86_64-linux-gnu/qt5/mkspecs/linux-g++-64 -isystem /usr/include/x86_64-linux-gnu/qt5/QtGui -isystem /usr/include/x86_64-linux-gnu/qt5/QtOpenGL -isystem /usr/include/x86_64-linux-gnu/qt5/QtWidgets  -std=c++14 -ftemplate-depth=1024 -Wall -Wextra -Wshadow -Wnon-virtual-dtor -Werror -Wno-variadic-macros -Wno-unknown-pragmas -fext-numeric-literals -fvisibility=hidden -D__QT__ -DMBGL_USE_GLES2   -fPIC -fvisibility-inlines-hidden -std=gnu++14 -o CMakeFiles/mbgl-core.dir/src/mbgl/style/parser.cpp.o -c /home/miha/mapbox/mapbox-gl-native/src/mbgl/style/parser.cpp
In file included from /home/miha/mapbox/mapbox-gl-native/include/mbgl/style/expression/parsing_context.hpp:6:0,
                 from /home/miha/mapbox/mapbox-gl-native/include/mbgl/style/expression/expression.hpp:11,
                 from /home/miha/mapbox/mapbox-gl-native/include/mbgl/style/function/camera_function.hpp:3,
                 from /home/miha/mapbox/mapbox-gl-native/include/mbgl/style/property_value.hpp:5,
                 from /home/miha/mapbox/mapbox-gl-native/include/mbgl/style/light.hpp:5,
                 from /home/miha/mapbox/mapbox-gl-native/src/mbgl/style/parser.hpp:5,
                 from /home/miha/mapbox/mapbox-gl-native/src/mbgl/style/parser.cpp:1:
/home/miha/mapbox/mapbox-gl-native/include/mbgl/style/conversion.hpp: In instantiation of ‘struct mbgl::style::conversion::Convertible::vtableForType()::<lambda(const Storage&, const std::function<std::experimental::fundamentals_v1::optional<mbgl::style::conversion::Error>(const std::basic_string<char>&, const mbgl::style::conversion::Convertible&)>&)> [with T = const rapidjson::GenericValue<rapidjson::UTF8<>, rapidjson::CrtAllocator>*; mbgl::style::conversion::Convertible::Storage = std::aligned_storage<32ul, 8ul>::type]::<lambda(const string&, const class rapidjson::GenericValue<rapidjson::UTF8<>, rapidjson::CrtAllocator>*&&)>’:
/home/miha/mapbox/mapbox-gl-native/include/mbgl/style/conversion.hpp:261:42:   required from ‘mbgl::style::conversion::Convertible::vtableForType()::<lambda(const Storage&, const std::function<std::experimental::fundamentals_v1::optional<mbgl::style::conversion::Error>(const std::basic_string<char>&, const mbgl::style::conversion::Convertible&)>&)> [with T = const rapidjson::GenericValue<rapidjson::UTF8<>, rapidjson::CrtAllocator>*; mbgl::style::conversion::Convertible::Storage = std::aligned_storage<32ul, 8ul>::type]’
/home/miha/mapbox/mapbox-gl-native/include/mbgl/style/conversion.hpp:260:14:   required from ‘struct mbgl::style::conversion::Convertible::vtableForType() [with T = const rapidjson::GenericValue<rapidjson::UTF8<>, rapidjson::CrtAllocator>*]::<lambda(const Storage&, const class std::function<std::experimental::fundamentals_v1::optional<mbgl::style::conversion::Error>(const std::basic_string<char>&, const mbgl::style::conversion::Convertible&)>&)>’
/home/miha/mapbox/mapbox-gl-native/include/mbgl/style/conversion.hpp:283:9:   required from ‘static mbgl::style::conversion::Convertible::VTable* mbgl::style::conversion::Convertible::vtableForType() [with T = const rapidjson::GenericValue<rapidjson::UTF8<>, rapidjson::CrtAllocator>*]’
/home/miha/mapbox/mapbox-gl-native/include/mbgl/style/conversion.hpp:88:67:   required from ‘mbgl::style::conversion::Convertible::Convertible(T&&) [with T = const rapidjson::GenericValue<rapidjson::UTF8<>, rapidjson::CrtAllocator>*]’
/home/miha/mapbox/mapbox-gl-native/src/mbgl/style/parser.cpp:261:78:   required from here
/home/miha/mapbox/mapbox-gl-native/include/mbgl/style/conversion.hpp:261:76: error: ‘mbgl::style::conversion::Convertible::vtableForType()::<lambda(const Storage&, const std::function<std::experimental::fundamentals_v1::optional<mbgl::style::conversion::Error>(const std::basic_string<char>&, const mbgl::style::conversion::Convertible&)>&)> [with T = const rapidjson::GenericValue<rapidjson::UTF8<>, rapidjson::CrtAllocator>*; mbgl::style::conversion::Convertible::Storage = std::aligned_storage<32ul, 8ul>::type]::<lambda(const string&, const rapidjson::GenericValue<rapidjson::UTF8<>, rapidjson::CrtAllocator>*&&)>’ declared with greater visibility than the type of its field ‘mbgl::style::conversion::Convertible::vtableForType()::<lambda(const Storage&, const std::function<std::experimental::fundamentals_v1::optional<mbgl::style::conversion::Error>(const std::basic_string<char>&, const mbgl::style::conversion::Convertible&)>&)> [with T = const rapidjson::GenericValue<rapidjson::UTF8<>, rapidjson::CrtAllocator>*; mbgl::style::conversion::Convertible::Storage = std::aligned_storage<32ul, 8ul>::type]::<lambda(const string&, const rapidjson::GenericValue<rapidjson::UTF8<>, rapidjson::CrtAllocator>*&&)>::<fn capture>’ [-Werror=attributes]
                 return Traits::eachMember(reinterpret_cast<const T&>(s), [&](const std::string& k, T&& v) {
                                                                            ^
cc1plus: all warnings being treated as errors

The patch hides gcc bug

@oxidase oxidase requested a review from tmpsantos November 18, 2017 09:19
tmpsantos
tmpsantos suggested changes Nov 23, 2017
View reviewed changes
platform/qt/qt.cmake Outdated
@@ -5,8 +5,13 @@ option(WITH_QT_DECODERS "Use builtin Qt image decoders" OFF)
option(WITH_QT_I18N "Use builtin Qt i18n support" OFF)
option(WITH_QT_4 "Use Qt4 instead of Qt5" OFF)

set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fvisibility=hidden -D__QT__")
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fvisibility=hidden -D__QT__")
if (CMAKE_COMPILER_IS_GNUCC AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 6.3)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will disable the visibility for GCC < 6.3 and Clang. I build with GCC 5.4 and 4.9 just fine. Can we make sure that only affected GCC versions won't have -fvisibility=hidden

@oxidase oxidase force-pushed the dont-use-hidden-visibility-for-gcc-63 branch from 2c0a2c3 to f737285 Compare November 23, 2017 17:29
@oxidase
Copy link
Author

oxidase commented Nov 23, 2017

@tmpsantos i have updated the condition to 6.1-6.4 and 7.2 as stated in the gcc bug report

@kkaefer kkaefer added Qt Mapbox Maps SDK for Qt, aka Qt Location Mapbox GL build labels Nov 24, 2017
tmpsantos
tmpsantos reviewed Nov 27, 2017
View reviewed changes
platform/qt/qt.cmake
@@ -5,8 +5,15 @@ option(WITH_QT_DECODERS "Use builtin Qt image decoders" OFF)
option(WITH_QT_I18N "Use builtin Qt i18n support" OFF)
option(WITH_QT_4 "Use Qt4 instead of Qt5" OFF)

set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fvisibility=hidden -D__QT__")
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fvisibility=hidden -D__QT__")
if (CMAKE_COMPILER_IS_GNUCC AND NOT
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will still disable -fvisibility=hidden for clang because it only sets it when CMAKE_COMPILER_IS_GNUCC.

I think you could always do set(USE_DEFAULT_VISIBILITY "-fvisibility=hidden") and override with set(USE_DEFAULT_VISIBILITY "") when the broken compiler is detected.

@kkaefer kkaefer mentioned this pull request Nov 29, 2017
Merged
@kkaefer
Copy link
Member

kkaefer commented Nov 29, 2017

See #10592 for an alternative fix that doesn't require changing visibility settings.

@kkaefer
Copy link
Member

kkaefer commented Nov 29, 2017

Fixed in #10592. Thanks for reporting this build issue and for finding the GCC bug report, it was very helpful for finding a workaround!

@kkaefer kkaefer closed this Nov 29, 2017
@kkaefer kkaefer deleted the dont-use-hidden-visibility-for-gcc-63 branch November 29, 2017 16:56
@tobrun tobrun mentioned this pull request Sep 19, 2018
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@tmpsantos tmpsantos tmpsantos requested changes

Assignees
No one assigned
Labels
build Qt Mapbox Maps SDK for Qt, aka Qt Location Mapbox GL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@oxidase @kkaefer @tmpsantos