From 21bbc112b9374817dfa03d255643e6a7e164f089 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 24 Apr 2025 09:55:33 +0200 Subject: [PATCH 1/7] Enable warnings --- CMakeLists.txt | 98 +++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 94 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 8208b2a..fb04c5f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,6 +1,9 @@ cmake_minimum_required(VERSION 3.16...3.27) -project(rerun_vrs_example LANGUAGES CXX) +set(CMAKE_COMPILE_WARNING_AS_ERROR ON) # Treat warnings as errors? + +set(TARGET_NAME rerun_vrs_example) +project(${TARGET_NAME} LANGUAGES CXX) set(CMAKE_EXPORT_COMPILE_COMMANDS ON) @@ -22,6 +25,93 @@ FetchContent_MakeAvailable(vrslib) find_package(fmt REQUIRED) -add_executable(rerun_vrs_example src/main.cpp src/frame_player.cpp src/imu_player.cpp) -target_link_libraries(rerun_vrs_example rerun_sdk vrslib vrs_utils fmt) -target_include_directories(rerun_vrs_example PRIVATE src) +add_executable(${TARGET_NAME} src/main.cpp src/frame_player.cpp src/imu_player.cpp) +target_link_libraries(${TARGET_NAME} rerun_sdk vrslib vrs_utils fmt) +target_include_directories(${TARGET_NAME} PRIVATE src) +target_include_directories(${TARGET_NAME} SYSTEM PRIVATE ${vrslib_SOURCE_DIR}) # Ignore warnings in VRS headers + + +if(MSVC) + # TODO(andreas): Try to enable /Wall + target_compile_options(${TARGET_NAME} PRIVATE /W4) + + target_compile_options(${TARGET_NAME} PRIVATE /we4996) # Using deprecated functions is an error + + if(BUILD_SHARED_LIBS) + # If we are building as shared libs, we are going to have to disable the C4251 + # warning, as it would trigger for any datatype derived from a STL class + # See also https://github.com/protocolbuffers/protobuf/blob/v26.1/cmake/README.md#notes-on-compiler-warnings + # We need also to make it public, otherwise downstream code will be flooded by c4251 warnings + target_compile_options(${TARGET_NAME} PUBLIC /wd4251) + endif() + + # CMAKE_COMPILE_WARNING_AS_ERROR is only directly supported starting in CMake `3.24` + # https://cmake.org/cmake/help/latest/prop_tgt/COMPILE_WARNING_AS_ERROR.html + if(CMAKE_COMPILE_WARNING_AS_ERROR) + target_compile_options(${TARGET_NAME} PRIVATE /WX + /w15038 # Initialization order. https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/c5038 + ) + endif() +else() + # Enabled warnings. + target_compile_options(${TARGET_NAME} PRIVATE + -Wall + -Wcast-align + -Wcast-qual + -Werror=deprecated-declarations + -Wextra + -Wformat=2 + -Wmissing-include-dirs + -Wnull-dereference + -Wold-style-cast + -Wpedantic + -Wpointer-arith + -Wshadow + -Wswitch-enum + -Wunreachable-code + -Wvla + ) + + if(CMAKE_CXX_COMPILER_ID MATCHES "Clang") # match both "Clang" and "AppleClang" + # TODO(emilk): enable some hardening flags from https://best.openssf.org/Compiler-Hardening-Guides/Compiler-Options-Hardening-Guide-for-C-and-C++.html + target_compile_options(${TARGET_NAME} PRIVATE + -Wc++17-compat-pedantic + -Wc++20-compat-pedantic + -Wc99-extensions + -Weverything + -Wgnu + -Wnon-gcc + -Wpre-c2x-compat-pedantic + -Wshadow-all + + # Turn off some warning that -Weverything turns on: + -Wno-c++98-compat + -Wno-c++98-compat-pedantic + -Wno-covered-switch-default # We often add a `default:` case out of paranoia + -Wno-ctad-maybe-unsupported + -Wno-disabled-macro-expansion + -Wno-documentation + -Wno-documentation-unknown-command + -Wno-double-promotion # float->double is nothing to whine about + -Wno-exit-time-destructors + -Wno-float-equal # comparing floats is fine + -Wno-global-constructors + -Wno-missing-prototypes + -Wno-padded + -Wno-reserved-id-macro + -Wno-reserved-identifier + -Wno-unused-macros + ) + endif() + + # CMAKE_COMPILE_WARNING_AS_ERROR is only directly supported starting in CMake `3.24` + # https://cmake.org/cmake/help/latest/prop_tgt/COMPILE_WARNING_AS_ERROR.html + if(CMAKE_COMPILE_WARNING_AS_ERROR) + target_compile_options(${TARGET_NAME} PRIVATE -Werror) + endif() + + if(CMAKE_BUILD_TYPE STREQUAL "Debug") + # Improve stack traces: + target_compile_options(${TARGET_NAME} PRIVATE -g -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -fno-optimize-sibling-calls) + endif() +endif() From 7201e1cab044c4ddbcb2eee95c0eec9ae831738a Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 24 Apr 2025 09:55:42 +0200 Subject: [PATCH 2/7] Fix some warnings --- src/frame_player.cpp | 2 +- src/frame_player.hpp | 2 +- src/imu_player.hpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/frame_player.cpp b/src/frame_player.cpp index 822b2fe..f9051c4 100644 --- a/src/frame_player.cpp +++ b/src/frame_player.cpp @@ -57,7 +57,7 @@ namespace rerun_vrs { auto& config = getExpectedLayout(layout, block_index); uint64_t frame_number; if (config.frameNumber.get(frame_number)) { - _rec->set_time_sequence("frame_number", frame_number); + _rec->set_time_sequence("frame_number", static_cast(frame_number)); } // this is meta data per record and changes over time diff --git a/src/frame_player.hpp b/src/frame_player.hpp index 9220941..6827f4e 100644 --- a/src/frame_player.hpp +++ b/src/frame_player.hpp @@ -36,8 +36,8 @@ namespace rerun_vrs { override; private: - std::shared_ptr _rec; vrs::StreamId _id; + std::shared_ptr _rec; std::string _entity_path; bool _enabled{true}; }; diff --git a/src/imu_player.hpp b/src/imu_player.hpp index 1433cc0..b6b9458 100644 --- a/src/imu_player.hpp +++ b/src/imu_player.hpp @@ -38,8 +38,8 @@ namespace rerun_vrs { void log_gyroscope(const std::array& gyroRadSec); void log_magnetometer(const std::array& magTesla); - std::shared_ptr _rec; vrs::StreamId _id; + std::shared_ptr _rec; std::string _entity_path; bool _enabled{true}; bool _has_accelerometer; From 29b0c1dabdfd8caac45256b3b7b775e074aabb80 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 24 Apr 2025 10:37:13 +0200 Subject: [PATCH 3/7] Try to enable deprecation warnings --- CMakeLists.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index fb04c5f..260d81b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -28,6 +28,7 @@ find_package(fmt REQUIRED) add_executable(${TARGET_NAME} src/main.cpp src/frame_player.cpp src/imu_player.cpp) target_link_libraries(${TARGET_NAME} rerun_sdk vrslib vrs_utils fmt) target_include_directories(${TARGET_NAME} PRIVATE src) +target_include_directories(${TARGET_NAME} SYSTEM PRIVATE ${rerun_sdk_SOURCE_DIR}) # Ignore warnings in Rerun SDK headers target_include_directories(${TARGET_NAME} SYSTEM PRIVATE ${vrslib_SOURCE_DIR}) # Ignore warnings in VRS headers @@ -58,6 +59,8 @@ else() -Wall -Wcast-align -Wcast-qual + -Wdeprecated + -Wdeprecated-declarations -Werror=deprecated-declarations -Wextra -Wformat=2 @@ -101,6 +104,8 @@ else() -Wno-reserved-id-macro -Wno-reserved-identifier -Wno-unused-macros + -Wno-unsafe-buffer-usage # Not sure why we need this, but we do. + -Wno-unknown-warning-option # Otherwise older clang will complain about `-Wno-unsafe-buffer-usage` ) endif() From 9d8ae7b708843e64ab3a2ca1bf19bf8a3943b46c Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 24 Apr 2025 10:41:13 +0200 Subject: [PATCH 4/7] Ignore warnings in VRS source code --- CMakeLists.txt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 260d81b..226fd28 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -22,6 +22,13 @@ set(BUILD_TOOLS OFF) # VRS tools set(BUILD_SAMPLES OFF) # VRS sample code and sample apps FetchContent_Declare(vrslib URL https://github.com/facebookresearch/vrs/archive/refs/tags/v1.1.0.zip) FetchContent_MakeAvailable(vrslib) +if(NOT MSVC) +# Disable warnings for VRS source files: + get_target_property(VRS_TYPE vrslib TYPE) + if(VRS_TYPE STREQUAL "STATIC_LIBRARY") + target_compile_options(vrslib PRIVATE -w) # Disable all warnings for VRS + endif() +endif() find_package(fmt REQUIRED) From d2d889f3b03ab6ddf3bc13b99374f731dcc76603 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 24 Apr 2025 10:45:04 +0200 Subject: [PATCH 5/7] Try that again --- CMakeLists.txt | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 226fd28..1fd3d40 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -23,11 +23,7 @@ set(BUILD_SAMPLES OFF) # VRS sample code and sample apps FetchContent_Declare(vrslib URL https://github.com/facebookresearch/vrs/archive/refs/tags/v1.1.0.zip) FetchContent_MakeAvailable(vrslib) if(NOT MSVC) -# Disable warnings for VRS source files: - get_target_property(VRS_TYPE vrslib TYPE) - if(VRS_TYPE STREQUAL "STATIC_LIBRARY") - target_compile_options(vrslib PRIVATE -w) # Disable all warnings for VRS - endif() + target_compile_options(vrslib PRIVATE -w) # Disable all warnings for VRS source code endif() find_package(fmt REQUIRED) From 06fac5eeef669e9870d995c940ba3393a05eaefe Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 24 Apr 2025 10:47:45 +0200 Subject: [PATCH 6/7] Try another fix --- CMakeLists.txt | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 1fd3d40..6b5f66d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -22,9 +22,6 @@ set(BUILD_TOOLS OFF) # VRS tools set(BUILD_SAMPLES OFF) # VRS sample code and sample apps FetchContent_Declare(vrslib URL https://github.com/facebookresearch/vrs/archive/refs/tags/v1.1.0.zip) FetchContent_MakeAvailable(vrslib) -if(NOT MSVC) - target_compile_options(vrslib PRIVATE -w) # Disable all warnings for VRS source code -endif() find_package(fmt REQUIRED) @@ -112,6 +109,10 @@ else() ) endif() + target_compile_options(${TARGET_NAME} PRIVATE + -Wno-rerun-type # VRS source code breaks this + ) + # CMAKE_COMPILE_WARNING_AS_ERROR is only directly supported starting in CMake `3.24` # https://cmake.org/cmake/help/latest/prop_tgt/COMPILE_WARNING_AS_ERROR.html if(CMAKE_COMPILE_WARNING_AS_ERROR) From d78aca60370a45c68358bf8fae8fff536978ee2d Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 24 Apr 2025 10:51:28 +0200 Subject: [PATCH 7/7] I hate CMAKE --- CMakeLists.txt | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 6b5f66d..d58fbb8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,6 +1,6 @@ cmake_minimum_required(VERSION 3.16...3.27) -set(CMAKE_COMPILE_WARNING_AS_ERROR ON) # Treat warnings as errors? +set(CMAKE_COMPILE_WARNING_AS_ERROR OFF) # TODO(emilk): If we turn this ON, VRS fails to compile set(TARGET_NAME rerun_vrs_example) project(${TARGET_NAME} LANGUAGES CXX) @@ -109,10 +109,6 @@ else() ) endif() - target_compile_options(${TARGET_NAME} PRIVATE - -Wno-rerun-type # VRS source code breaks this - ) - # CMAKE_COMPILE_WARNING_AS_ERROR is only directly supported starting in CMake `3.24` # https://cmake.org/cmake/help/latest/prop_tgt/COMPILE_WARNING_AS_ERROR.html if(CMAKE_COMPILE_WARNING_AS_ERROR)