#10345 - Sush warnings in third_party directory on modern compilers/OSes #10346

jmarrec · 2023-12-22T14:38:47Z

Pull request overview

Fixes Shush warnings from third_party code? #10345
Allow building on LLVM clang 17: fix a SCC cmake issue

Pull Request Author

Add to this list or remove from it as applicable. This is a simple templated set of guidelines.

Reviewer

This will not be exhaustively relevant to every PR.

* The arch shouldn't be passed, I'm on M1 not x86_64. This was removed upstream already: NREL/ssc@4e0ab97 * Shouldn't link to stdc++ directly. I'm actively trying to build against's LLVM's libc++ and getting a multiple flag warning (I use `export LDFLAGS="-L/opt/homebrew/opt/llvm/lib/c++ -Wl,-rpath,/opt/homebrew/opt/llvm/lib/c++"`)

``` /Users/julien/Software/Others/EnergyPlus/third_party/kiva/vendor/boost-1.77.0/boost/type_traits/has_nothrow_constructor.hpp:27:84: warning: builtin __has_nothrow_constructor is deprecated; use __is_nothrow_constructible instead [-Wdeprecated-builtins] 27 | template <class T> struct has_nothrow_constructor : public integral_constant<bool, BOOST_HAS_NOTHROW_CONSTRUCTOR(T)>{}; ` `` I pulled the latest from develop for the move and type_traits boost projects, keeping only the macro changes I needed.

…to ssc because it tries to export/install)

…nings for now

``` In file included from /Users/julien/Software/Others/EnergyPlus/src/EnergyPlus/FileSystem.hh:56: /Users/julien/Software/Others/EnergyPlus/third_party/nlohmann/json.hpp:5350:32: warning: 'char_traits<unsigned char>' is deprecated: char_traits<T> for T not equal to char, wchar_t, char8_t, char16_t or char32_t is non-standard and is provided for a temporary period. It will be removed in LLVM 18, so please migrate off of it. [-Wdeprecated-declarations] ```

jmarrec · 2023-12-22T14:39:49Z

CMakeLists.txt

@@ -59,6 +59,8 @@ add_library(project_fp_options INTERFACE)

 add_library(project_warnings INTERFACE)

+add_library(turn_off_warnings INTERFACE)


A new interface library, which does -w on gcc/clang and /W0 on msvc to remove all warnings

I agree, we shouldn't be trying to handle warnings emitted from third_party, so this is fine with me.

jmarrec · 2023-12-22T14:40:38Z

third_party/CMakeLists.txt

 add_subdirectory(FMI)
 set_target_properties(epfmiimport PROPERTIES FOLDER ThirdParty/FMI)
+target_link_libraries(epfmiimport turn_off_warnings)


Turn off warnings for some of the third party libs that were issuing some:

FMI

zlib

DElight

jmarrec · 2023-12-22T14:40:57Z

third_party/kiva/vendor/boost-1.77.0/boost/geometry/policies/relate/intersection_points.hpp

+        unsigned int index = 0;
+        [[maybe_unused]] unsigned int count_a = 0;
+        [[maybe_unused]] unsigned int count_b = 0;


easy warning avoidance.

jmarrec · 2023-12-22T14:41:04Z

third_party/eigen/Eigen/src/SparseCore/TriangularSolver.h

@@ -270,7 +270,7 @@ struct sparse_solve_triangular_sparse_selector<Lhs,Rhs,Mode,UpLo,ColMajor>
      }


-      Index count = 0;
+      [[maybe_unused]] Index count = 0;


easy warning avoidance.

jmarrec · 2023-12-22T14:42:02Z

third_party/kiva/vendor/boost-1.77.0/boost/type_traits/intrinsics.hpp

+#   if __has_builtin(__is_nothrow_constructible)
+#     define BOOST_HAS_NOTHROW_CONSTRUCTOR(T) (__is_nothrow_constructible(T) && is_default_constructible<T>::value)
+#   elif __has_feature(has_nothrow_constructor)
 #     define BOOST_HAS_NOTHROW_CONSTRUCTOR(T) (__has_nothrow_constructor(T) && is_default_constructible<T>::value)
 #   endif


/Users/julien/Software/Others/EnergyPlus/third_party/kiva/vendor/boost-1.77.0/boost/type_traits/has_nothrow_constructor.hpp:27:84: warning: builtin __has_nothrow_constructor is deprecated; use __is_nothrow_constructible instead [-Wdeprecated-builtins] 27 | template <class T> struct has_nothrow_constructor : public integral_constant<bool, BOOST_HAS_NOTHROW_CONSTRUCTOR(T)>{};

I pulled the latest from develop for the move and type_traits boost projects, keeping only the macro changes I needed.

jmarrec · 2023-12-22T14:43:14Z

third_party/CMakeLists.txt

 target_include_directories(
        nlohmann_json
-        INTERFACE
+        SYSTEM INTERFACE
        $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/nlohmann>
 )
+set_source_files_properties(
+  $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/nlohmann>/json.hpp
+  TARGET_DIRECTORY nlohmann_json
+  PROPERTIES COMPILE_FLAGS -Wno-deprecated-declarations
+)


Somehow I can't figure out how to suppress this warning?

In file included from /Users/julien/Software/Others/EnergyPlus/src/EnergyPlus/FileSystem.hh:56: /Users/julien/Software/Others/EnergyPlus/third_party/nlohmann/json.hpp:5350:32: warning: 'char_traits<unsigned char>' is deprecated: char_traits<T> for T not equal to char, wchar_t, char8_t, char16_t or char32_t is non-standard and is provided for a temporary period. It will be removed in LLVM 18, so please migrate off of it. [-Wdeprecated-declarations] 5350 | auto result = std::char_traits<char_type>::to_int_type(*current); | ^ /Users/julien/Software/Others/EnergyPlus/third_party/nlohmann/json.hpp:10489:29: note: in instantiation of member function 'nlohmann::detail::iterator_input_adapter<const unsigned char *>::get_character' requested here 10489 | return current = ia.get_character(); | ^ /Users/julien/Software/Others/EnergyPlus/third_party/nlohmann/json.hpp:8308:17: note: in instantiation of member function 'nlohmann::detail::binary_reader<nlohmann::basic_json<>, nlohmann::detail::iterator_input_adapter<const unsigned char *>>::get' requested here 8308 | get();

Myoldmopar

This looks great @jmarrec, I'm happy to carefully mute warnings from our third party folder. I am also getting a few cmake warnings from third party that I may also mute when I pull latest develop into this branch locally.

Myoldmopar · 2023-12-22T15:14:53Z

CMakeLists.txt

@@ -59,6 +59,8 @@ add_library(project_fp_options INTERFACE)

 add_library(project_warnings INTERFACE)

+add_library(turn_off_warnings INTERFACE)


I agree, we shouldn't be trying to handle warnings emitted from third_party, so this is fine with me.

Myoldmopar · 2023-12-22T15:19:15Z

third_party/kiva/vendor/boost-1.77.0/boost/move/detail/type_traits.hpp

 #     define BOOST_MOVE_HAS_NOTHROW_MOVE_ASSIGN(T) (__is_assignable(T, T&&) && __is_nothrow_assignable(T, T&&))
 #   elif BOOST_MOVE_HAS_TRAIT(has_nothrow_move_assign)
 #     define BOOST_MOVE_HAS_NOTHROW_MOVE_ASSIGN(T) __has_nothrow_move_assign(T)
 #   endif

+#   endif   //BOOST_NO_CXX11_RVALUE_REFERENCES


@jmarrec could you confirm the intent here? It looks like you just pulled one of the IF conditions to an outer IF block. Anything functionally different?

Same as above, it's mostly just using the right builtins.

The outer condition is just the way it's on develop at https://github.com/boostorg/move/blob/develop/include/boost/move/detail/type_traits.hpp

OK, that should be fine.

Myoldmopar · 2023-12-22T15:19:55Z

third_party/ssc/CMakeLists.txt

@@ -91,7 +91,7 @@ function(set_default_compile_options target)
    else(MSVC)
        SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11")
        if (APPLE)
-            set(MAIN_CFLAGS "${MAIN_CFLAGS} -arch x86_64 -fno-common -DWX_PRECOMP -D__MACOSX__")
+            set(MAIN_CFLAGS "${MAIN_CFLAGS} -fno-common -DWX_PRECOMP -D__MACOSX__")


Myoldmopar · 2023-12-22T15:20:18Z

third_party/ssc/ssc/CMakeLists.txt

+  find_library(MATH_LIBRARY m)
+  if(MATH_LIBRARY)
+    target_link_libraries(ssc ${MATH_LIBRARY})
+  endif()


Myoldmopar · 2023-12-22T15:37:07Z

I was getting lots of CMake version warnings:

CMake Deprecation Warning at third_party/gtest/CMakeLists.txt:4 (cmake_minimum_required):
   Compatibility with CMake < 3.5 will be removed from a future version of
   CMake.

   Update the VERSION argument <min> value or use a ...<max> suffix to tell
   CMake that the project does not need compatibility with older versions.

 CMake Deprecation Warning at third_party/gtest/googlemock/CMakeLists.txt:45 (cmake_minimum_required):
   Compatibility with CMake < 3.5 will be removed from a future version of
   CMake.

   Update the VERSION argument <min> value or use a ...<max> suffix to tell
   CMake that the project does not need compatibility with older versions.

 CMake Deprecation Warning at third_party/gtest/googletest/CMakeLists.txt:56 (cmake_minimum_required):
   Compatibility with CMake < 3.5 will be removed from a future version of
   CMake.

   Update the VERSION argument <min> value or use a ...<max> suffix to tell
   CMake that the project does not need compatibility with older versions.

 CMake Deprecation Warning at third_party/DElight/CMakeLists.txt:1 (CMAKE_MINIMUM_REQUIRED):
   Compatibility with CMake < 3.5 will be removed from a future version of
   CMake.

   Update the VERSION argument <min> value or use a ...<max> suffix to tell
   CMake that the project does not need compatibility with older versions.

CMake Deprecation Warning at third_party/Windows-CalcEngine/CMakeLists.txt:1 (cmake_minimum_required):
   Compatibility with CMake < 3.5 will be removed from a future version of
   CMake.

   Update the VERSION argument <min> value or use a ...<max> suffix to tell
   CMake that the project does not need compatibility with older versions.

We use these packages in much more modern versions of CMake, like 3.17+, so there's no reason we can't allow them to run with a more modern CMake version specifier. In the longer term, we should simply update the third party libraries, and hopefully they have made the udpates there. Anyway, this hushes things up in a pretty innocent way. If we end up updating any of these libraries and it overrides the change, no big deal.

Myoldmopar · 2023-12-22T15:40:24Z

This was totally happy on CI with the previous commit, and it's still happy locally. I don't think I'll wait on CI, let's just get this merged in. Thanks @jmarrec !

jmarrec added 8 commits December 22, 2023 12:56

Shush some Wunused-but-set-variable

cc20b31

#10345 - suppress all warnings from major third_party offenders

0e3b9f4

Redo with an interface library turn_off_warnings (note: can't link …

604ea98

…to ssc because it tries to export/install)

Remove kiva from lists of third_party project were I suppress all war…

a32540d

…nings for now

Treat nlohmann includes as SYSTEM

1c80e2b

jmarrec added Defect Includes code to repair a defect in EnergyPlus Developer Issue Related to cmake, packaging, installers, or developer tooling (CI, etc) labels Dec 22, 2023

jmarrec self-assigned this Dec 22, 2023

jmarrec commented Dec 22, 2023

View reviewed changes

Myoldmopar reviewed Dec 22, 2023

View reviewed changes

Myoldmopar added 2 commits December 22, 2023 09:23

Merge remote-tracking branch 'origin/develop' into 10345-SushWarnings

c0ccd20

Add updated Cmake supported version to hush cmake warnings

cc6b54f

Myoldmopar merged commit 8f96c94 into develop Dec 22, 2023
10 checks passed

Myoldmopar deleted the 10345-SushWarnings branch December 22, 2023 15:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#10345 - Sush warnings in third_party directory on modern compilers/OSes #10346

#10345 - Sush warnings in third_party directory on modern compilers/OSes #10346

jmarrec commented Dec 22, 2023

jmarrec Dec 22, 2023

Myoldmopar Dec 22, 2023

jmarrec Dec 22, 2023 •

edited

Loading

jmarrec Dec 22, 2023

jmarrec Dec 22, 2023

jmarrec Dec 22, 2023

jmarrec Dec 22, 2023

Myoldmopar left a comment

Myoldmopar Dec 22, 2023

Myoldmopar Dec 22, 2023

jmarrec Dec 22, 2023

Myoldmopar Dec 22, 2023

Myoldmopar Dec 22, 2023

Myoldmopar Dec 22, 2023

Myoldmopar commented Dec 22, 2023

Myoldmopar commented Dec 22, 2023

		@@ -59,6 +59,8 @@ add_library(project_fp_options INTERFACE)

		add_library(project_warnings INTERFACE)

		add_library(turn_off_warnings INTERFACE)

#10345 - Sush warnings in third_party directory on modern compilers/OSes #10346

#10345 - Sush warnings in third_party directory on modern compilers/OSes #10346

Conversation

jmarrec commented Dec 22, 2023

Pull request overview

Pull Request Author

Reviewer

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmarrec Dec 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Myoldmopar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Myoldmopar commented Dec 22, 2023

Myoldmopar commented Dec 22, 2023

jmarrec Dec 22, 2023 •

edited

Loading