From e55d84bb6c7a41d4ac4092246233949e42fe2d46 Mon Sep 17 00:00:00 2001 From: Pablo Marcos Date: Fri, 8 Nov 2024 10:33:01 +0100 Subject: [PATCH] Fix warnings to allow using the header on pedantic projects Some projects have strict requirements regarding warnings. Let's have an option to enable pedantic warnings and address them to allow those projects to use the header without having to disable them manually. e.g. for ClickHouse #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wall" #pragma clang diagnostic ignored "-Wextra" #pragma clang diagnostic ignored "-Weverything" #pragma clang diagnostic ignored "-Wpedantic" #define CTRACK_DISABLE_EXECUTION_POLICY 1 #include "ctrack.hpp" #pragma clang diagnostic pop --- CMakeLists.txt | 12 ++++++++++- cmake/add_warning.cmake | 42 +++++++++++++++++++++++++++++++++++++ cmake/warnings.cmake | 46 +++++++++++++++++++++++++++++++++++++++++ include/ctrack.hpp | 39 +++++++++++++++++----------------- 4 files changed, 118 insertions(+), 21 deletions(-) create mode 100644 cmake/add_warning.cmake create mode 100644 cmake/warnings.cmake diff --git a/CMakeLists.txt b/CMakeLists.txt index c7d31a3..f304386 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -10,6 +10,8 @@ option(DISABLE_PAR "Disable parallel processing" OFF) # Option to disable building examples option(DISABLE_EXAMPLES "Disable building examples" OFF) +option(ENABLE_WARNINGS "Enable warnings" OFF) + # Check for TBB if(NOT MSVC AND NOT DISABLE_PAR) find_package(TBB QUIET) @@ -25,7 +27,7 @@ endif() # Create the ctrack library add_library(ctrack INTERFACE) -target_include_directories(ctrack INTERFACE +target_include_directories(ctrack INTERFACE $ $ ) @@ -37,6 +39,14 @@ elseif(NOT MSVC AND TBB_FOUND) target_link_libraries(ctrack INTERFACE TBB::tbb) endif() +set(CMAKE_EXPORT_COMPILE_COMMANDS ON) +if(ENABLE_WARNINGS) + if (NOT MSVC) + include(cmake/add_warning.cmake) + include(cmake/warnings.cmake) + endif() +endif() + # Add the examples subdirectory if not disabled if(NOT DISABLE_EXAMPLES) add_subdirectory(examples) diff --git a/cmake/add_warning.cmake b/cmake/add_warning.cmake new file mode 100644 index 0000000..c70adb4 --- /dev/null +++ b/cmake/add_warning.cmake @@ -0,0 +1,42 @@ +# Taken from https://github.com/ClickHouse/ClickHouse/blob/master/cmake/add_warnings.cmake + +include (CheckCXXCompilerFlag) + +# Try to add -Wflag if compiler supports it +macro (add_warning flag) + string (REPLACE "-" "_" underscored_flag ${flag}) + string (REPLACE "+" "x" underscored_flag ${underscored_flag}) + + check_cxx_compiler_flag("-W${flag}" SUPPORTS_CXXFLAG_${underscored_flag}) + + if (SUPPORTS_CXXFLAG_${underscored_flag}) + set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -W${flag}") + else () + message (STATUS "Flag -W${flag} is unsupported") + endif () + +endmacro () + +# Try to add -Wno flag if compiler supports it +macro (no_warning flag) + add_warning(no-${flag}) +endmacro () + + +# The same but only for specified target. +macro (target_add_warning target flag) + string (REPLACE "-" "_" underscored_flag ${flag}) + string (REPLACE "+" "x" underscored_flag ${underscored_flag}) + + check_cxx_compiler_flag("-W${flag}" SUPPORTS_CXXFLAG_${underscored_flag}) + + if (SUPPORTS_CXXFLAG_${underscored_flag}) + target_compile_options (${target} PRIVATE "-W${flag}") + else () + message (STATUS "Flag -W${flag} is unsupported") + endif () +endmacro () + +macro (target_no_warning target flag) + target_add_warning(${target} no-${flag}) +endmacro () diff --git a/cmake/warnings.cmake b/cmake/warnings.cmake new file mode 100644 index 0000000..11c2606 --- /dev/null +++ b/cmake/warnings.cmake @@ -0,0 +1,46 @@ +# Taken from https://github.com/ClickHouse/ClickHouse/blob/master/cmake/warnings.cmake, +# slightly modified to fit into ctrack style. + +set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra") + +# Control maximum size of stack frames. It can be important if the code is run in fibers with small stack size. +# Only in release build because debug has too large stack frames. +if ((NOT CMAKE_BUILD_TYPE_UC STREQUAL "DEBUG") AND (NOT SANITIZE) AND (NOT CMAKE_CXX_COMPILER_ID MATCHES "AppleClang")) + add_warning(frame-larger-than=65536) +endif () + +# Add some warnings that are not available even with -Wall -Wextra -Wpedantic. +# We want to get everything out of the compiler for code quality. +add_warning(everything) +add_warning(pedantic) +no_warning(zero-length-array) +no_warning(c++98-compat-pedantic) +no_warning(c++98-compat) +no_warning(c++20-compat) # Use constinit in C++20 without warnings +no_warning(sign-conversion) +no_warning(implicit-int-conversion) +no_warning(implicit-int-float-conversion) +no_warning(ctad-maybe-unsupported) # clang 9+, linux-only +no_warning(disabled-macro-expansion) +no_warning(documentation-unknown-command) +no_warning(double-promotion) +no_warning(exit-time-destructors) +no_warning(float-equal) +no_warning(global-constructors) +no_warning(missing-prototypes) +no_warning(missing-variable-declarations) +no_warning(padded) +no_warning(switch-enum) +no_warning(undefined-func-template) +no_warning(unused-template) +no_warning(vla) +no_warning(weak-template-vtables) +no_warning(weak-vtables) +no_warning(thread-safety-negative) # experimental flag, too many false positives +no_warning(enum-constexpr-conversion) # breaks magic-enum library in clang-16 +no_warning(unsafe-buffer-usage) # too aggressive +no_warning(switch-default) # conflicts with "defaults in a switch covering all enum values" + +# Styling decisions for ctrack +no_warning(shadow-field-in-constructor) +no_warning(newline-eof) \ No newline at end of file diff --git a/include/ctrack.hpp b/include/ctrack.hpp index e16f6f1..05fbca9 100644 --- a/include/ctrack.hpp +++ b/include/ctrack.hpp @@ -22,7 +22,6 @@ #endif #include #include -#include #include #include #include @@ -211,7 +210,7 @@ namespace ctrack { stream << std::string(leftPadding + 1, ' ') << row[i] << std::string(rightPadding + 1, ' '); } else { - stream << " " << std::setw(columnWidths[i]) << std::right << row[i] << " "; + stream << " " << std::setw(static_cast(columnWidths[i])) << std::right << row[i] << " "; } if (useColor) stream << RESET_COLOR << colors.border_color; stream << "|"; @@ -257,7 +256,7 @@ namespace ctrack { public: BeautifulTable(const std::vector& headerColumns, bool enableColor = false, const ColorScheme& colors = default_colors, const std::vector>& top_header = {}) - : header(headerColumns), useColor(enableColor), colors(colors), top_header(top_header) { + : top_header(top_header), header(headerColumns), useColor(enableColor), colors(colors) { updateColumnWidths(header); } @@ -368,7 +367,7 @@ namespace ctrack { std::string_view function; unsigned int event_id; Event(const std::chrono::high_resolution_clock::time_point& start_time, const std::chrono::high_resolution_clock::time_point& end_time, const std::string_view filename, const int line, const std::string_view function, const int thread_id, const unsigned int event_id) - : start_time(start_time), end_time(end_time), filename(filename), line(line), function(function), thread_id(thread_id), event_id(event_id) + : start_time(start_time), end_time(end_time), line(line), thread_id(thread_id), filename(filename), function(function), event_id(event_id) {} }; @@ -377,8 +376,8 @@ namespace ctrack { std::chrono::high_resolution_clock::time_point start_time{}; int_fast64_t unique_id = 0; std::chrono::high_resolution_clock::time_point end_time{}; - Simple_Event(const std::chrono::high_resolution_clock::time_point& start_time, const std::chrono::high_resolution_clock::time_point& end_time, const uint_fast64_t duration, const int_fast64_t unique_id) :start_time(start_time), end_time(end_time), duration(duration), unique_id(unique_id) {} - Simple_Event() {}; + Simple_Event(const std::chrono::high_resolution_clock::time_point& start_time, const std::chrono::high_resolution_clock::time_point& end_time, const uint_fast64_t duration, const int_fast64_t unique_id) :duration(duration), start_time(start_time), unique_id(unique_id), end_time(end_time) {} + Simple_Event() {} }; inline bool cmp_simple_event_by_duration_asc(const Simple_Event& a, const Simple_Event& b) @@ -438,7 +437,7 @@ namespace ctrack { result.push_back(events[0]); unsigned int current_idx = 0; - for (int i = 1; i < events.size(); i++) { + for (size_t i = 1; i < events.size(); i++) { if (result[current_idx].end_time >= events[i].start_time) { result[current_idx].end_time = std::max(result[current_idx].end_time, events[i].end_time); } @@ -653,7 +652,7 @@ namespace ctrack { time_total = std::chrono::duration_cast( track_end_time - track_start_time).count(); center_intervall_str = "[" + std::to_string(settings.non_center_percent) + "-" + std::to_string(100 - settings.non_center_percent) + "]"; - }; + } template void get_summary_table(StreamType& stream, bool use_color = false) { @@ -676,7 +675,7 @@ namespace ctrack { } table.print(stream); - }; + } template void get_detail_table(StreamType& stream, bool use_color = false, bool reverse_vector = false) { @@ -749,23 +748,23 @@ namespace ctrack { void reserve_a_events(size_t size) { a_events.reserve(size); - }; + } inline void add_event(const std::string_view& filename, const std::string_view function, const int line, const Event& e) { f_res[filename][function][line].all_events.push_back(e); a_events.insert({ get_unique_event_id(e.thread_id, e.event_id), e }); } - void add_sub_events(const sub_events& s_events, const unsigned int thread_id) { + void add_sub_events(const sub_events& s_events, const unsigned int thread_id_) { for (auto const& [key, val] : s_events) { - int_fast64_t parent_id = get_unique_event_id(thread_id, key); + int_fast64_t parent_id = get_unique_event_id(thread_id_, key); for (const auto& child : val) { - child_graph[parent_id].push_back(get_unique_event_id(thread_id, child)); + child_graph[parent_id].push_back(get_unique_event_id(thread_id_, child)); } } - }; + } std::unordered_map < int_fast64_t, Event> a_events{}; filename_result f_res{}; @@ -826,7 +825,7 @@ namespace ctrack { while (store::write_events_locked) {} register_event(); - }; + } ~EventHandler() { auto end_time = std::chrono::high_resolution_clock::now(); while (store::write_events_locked) {} @@ -846,7 +845,7 @@ namespace ctrack { if ((*sub_events_ptr)[previous_event_id].capacity() - (*sub_events_ptr)[previous_event_id].size() < 1) (*sub_events_ptr)[previous_event_id].reserve((*sub_events_ptr)[previous_event_id].capacity() * 4); (*sub_events_ptr)[previous_event_id].push_back(event_id); } - }; + } private: void register_event() { t_id = fetch_event_t_id(); @@ -903,10 +902,10 @@ namespace ctrack { auto all_events_cnt = countAllEvents(store::a_events); res.reserve_a_events(all_events_cnt); - for (int thread_id = 0; thread_id <= store::thread_cnt; thread_id++) { - auto& t_events_entry = store::a_events[thread_id]; - auto& t_sub_events = store::a_sub_events[thread_id]; - res.add_sub_events(t_sub_events, thread_id); + for (int thread_id_ = 0; thread_id_ <= store::thread_cnt; thread_id_++) { + auto& t_events_entry = store::a_events[thread_id_]; + auto& t_sub_events = store::a_sub_events[thread_id_]; + res.add_sub_events(t_sub_events, thread_id_); for (const auto& c_event : t_events_entry) {