NREL / OpenStudio

OpenStudio is a cross-platform collection of software tools to support whole building energy modeling using EnergyPlus and advanced daylight analysis using Radiance.
https://www.openstudio.net/
Other
495 stars 190 forks source link

Fix mac warnings related to visibility settings #4120

Closed jmarrec closed 2 years ago

jmarrec commented 3 years ago

Issue overview

Build logs are polluted on mac with warnings like these two:

ld: warning: direct access in function 'boost::cpp_regex_traits<char>::get_catalog_name()' from file '/Users/dev/.conan/data/boost/1.73.0/_/_/package/076aff20c42e87a34bf7b270c852b7953bc51cd2/lib/libboost_
regex.a(cpp_regex_traits.o)' to global weak symbol 'guard variable for boost::cpp_regex_traits<char>::get_catalog_name_inst()::s_name' from file 'src/utilities/CMakeFiles/openstudio_utilities.dir/bcl/BCLM
easure.cpp.o' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.                 

ld: warning: direct access in function 'boost::cpp_regex_traits<char>::get_catalog_name()' from file '/Users/dev/.conan/data/boost/1.73.0/_/_/package/076aff20c42e87a34bf7b270c852b7953bc51cd2/lib/libboost_
regex.a(cpp_regex_traits.o)' to global weak symbol 'boost::cpp_regex_traits<char>::get_catalog_name_inst()::s_name' from file 'src/utilities/CMakeFiles/openstudio_utilities.dir/bcl/BCLMeasure.cpp.o' means
 the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.

Possible Solution

Probably related to CXX_FLAGS:

-fvisibility=hidden
-fvisibility-inlines-hidden

Environment

Some additional details about your environment for this issue (if relevant):

tijcolem commented 3 years ago

I got rid of some other linker warnings errors by bumping the cmake to use -DCMAKE_OSX_DEPLOYMENT_TARGET=10.14

Still seeing these visibility linker warnings. I'll try those CXX_FLAGS that you suggest.

ld: warning: direct access in function 'boost::cpp_regex_traits::get_catalogname()' from file '/Users/jenkins/.conan/data/boost/1.73.0//_/package/96a8c1d3b76f008cb6766a82e813655646b201da/lib/libboost_regex.a(cpp_regex_traits.o)' to global weak symbol 'guard variable for boost::cpp_regex_traits::get_catalog_name_inst()::s_name' from file 'CMakeFiles/openstudio_energyplus_tests.dir/Test/ScheduleInterval_GTest.cpp.o' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.

ld: warning: direct access in function 'boost::cpp_regex_traits::get_catalogname()' from file '/Users/jenkins/.conan/data/boost/1.73.0//_/package/96a8c1d3b76f008cb6766a82e813655646b201da/lib/libboost_regex.a(cpp_regex_traits.o)' to global weak symbol 'boost::cpp_regex_traits::get_catalog_name_inst()::s_name' from file 'CMakeFiles/openstudio_energyplus_tests.dir/Test/ScheduleInterval_GTest.cpp.o' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.

ld: warning: direct access in function 'boost::cpp_regex_traits::get_catalogname()' from file '/Users/jenkins/.conan/data/boost/1.73.0//_/package/96a8c1d3b76f008cb6766a82e813655646b201da/lib/libboost_regex.a(cpp_rege

tijcolem commented 3 years ago

@jmarrec I think your on the right track by suggested we add the following flags.

-fvisibility=hidden -fvisibility-inlines-hidden

I set those flags here for Mac builds https://github.com/NREL/OpenStudio/blob/develop/CMakeLists.txt#L436

 if(APPLE)
    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-overloaded-virtual -ftemplate-depth=1024 -fvisibility=hidden -fvisibility-inlines-hidden")
  else()

After I recompiled now I see these similar types of errors but now with different conan packages and not just boost as it was before.

snippet

not be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'openstudio::System::msleep(int)' from file '../src/utilities/CMakeFiles/openstudio_utilities.dir/core/System.cpp.o' to global weak symbol 'boost::date_time::c_time::gmtime(long const*, tm*)' from file '/Users/jenkins/.conan/data/cpprestsdk/2.10.16/_/_/package/e37cb034165b963dc726e1da15465eb9cc256056/lib/libcpprest.a(http_client_asio.cpp.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'pplx::details::_ContinuationTypeTraits<openstudio::UpdateManager::UpdateManager(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)::$_0, web::http::http_response>::_TaskOfType pplx::task<web::http::http_response>::then<openstudio::UpdateManager::UpdateManager(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)::$_0>(openstudio::UpdateManager::UpdateManager(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)::$_0&&) const' from file '../src/utilities/CMakeFiles/openstudio_utilities.dir/core/UpdateManager.cpp.o' to global weak symbol 'typeinfo for pplx::invalid_operation' from file '/Users/jenkins/.conan/data/cpprestsdk/2.10.16/_/_/package/e37cb034165b963dc726e1da15465eb9cc256056/lib/libcpprest.a(http_msg.cpp.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.

I don't know if we can set these visibility settings at the conan packages level, but it seems that the conan packages for Mac may have been built with different visibility settings and we might need to go through all of these to make them consistent for these warning to go away.

tijcolem commented 3 years ago

Update. Further along the build it throws a fatal error so adding -fvisibility=hidden -fvisibility-inlines-hidden for mac builds will not work. I think we'll need to investigate settings at the package level.

gmake[2]: *** No rule to make target 'src/utilities/idd/SurfaceProperty_ExposedFoundationPerimeter_FieldEnums.hxx', needed by 'src/energyplus/CMakeFiles/openstudio_energyplus.dir/ForwardTranslator/ForwardTranslateSurfacePropertyExposedFoundationPerimeter.cpp.o'.  Stop.
gmake[2]: *** Waiting for unfinished jobs....
Building CXX object src/energyplus/CMakeFiles/openstudio_energyplus.dir/ForwardTranslator/ForwardTranslateSurfacePropertyConvectionCoefficientsMultipleSurface.cpp.o
/Users/jenkins/git/OpenStudioFull/develop/src/energyplus/ForwardTranslator/ForwardTranslateSurfaceControlMovableInsulation.cpp:39:10: fatal error: 'utilities/idd/SurfaceControl_MovableInsulation_FieldEnums.hxx' file not found
#include <utilities/idd/SurfaceControl_MovableInsulation_FieldEnums.hxx>
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
gmake[2]: *** [src/energyplus/CMakeFiles/openstudio_energyplus.dir/build.make:4589: src/energyplus/CMakeFiles/openstudio_energyplus.dir/ForwardTranslator/ForwardTranslateSurfaceControlMovableInsulation.cpp.o] Error 1
gmake[2]: *** No rule to make target 'src/utilities/idd/OS_PlantEquipmentOperation_OutdoorWetBulbDifference_FieldEnums.hxx', needed by 'src/model/CMakeFiles/openstudio_model.dir/PlantEquipmentOperationOutdoorWetBulbDifference.cpp.o'.  Stop.
gmake[2]: *** Waiting for unfinished jobs....
Building CXX object src/model/CMakeFiles/openstudio_model.dir/PlantEquipmentOperationOutdoorDryBulbDifference.cpp.o
/Users/jenkins/git/OpenStudioFull/develop/src/energyplus/ForwardTranslator/ForwardTranslateSurface.cpp:56:10: fatal error: 'utilities/idd/BuildingSurface_Detailed_FieldEnums.hxx' file not found
#include <utilities/idd/BuildingSurface_Detailed_FieldEnums.hxx>
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
tijcolem commented 3 years ago

We're going to be moving all of our conan deps to a new self-hosted bintray. #4201 It might be good to try and rebuild boost with the right visbility settings for mac while doing this.

tijcolem commented 3 years ago

@jmarrec @kbenne @joseph-robertson What are you thoughts on removing boost in favor of using the std library? My understanding is all of boost is now available in the std library.

jmarrec commented 3 years ago

std::regex is an abomination at least (it's slower than most interpreted languages...). We could replace that with another third party library like CTRE hanickadot/compile-time-regular-expressions, though that would be best with C++20 which we do not target yet. (We already have 5 files where std::regex is used instead of boost::regex). Or we could use google/re2

std::filesystem (for paths/ file operations) is not ideal since it requires some link options depending on your compiler version and forces you to target mac 10.15. It is also missing a couple of functions (like weakly_canonical but those can be reimplemented easily)

The rest should be about ok. mutexes are covered in the std lib at least. optionals are there too, though the semantics varies a bit with the boost ones (but can be fixed easily).

I'd consider moving to C++20 first. Then try to use CTRE to try and speed up our IDD/OSM parsing. Some benchmarks are provided in this C++ paper: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1433r0.pdf

image

And more here, starting on page 84 of the slides deck: https://meetingcpp.com/mcpp/slides/2018/slides.pdf