From d5be8cad97e8582feefc7b8445c02e796bcd98c1 Mon Sep 17 00:00:00 2001 From: Patrick Ziegler Date: Mon, 4 Jan 2021 10:38:43 +0100 Subject: [PATCH] Add compiler warning for missing override specifier (#2341) * build: Add -Wsuggest-override We should always use the override specifier when overriding virtual functions. This helps prevent errors when a subclass tries to create a function with the same name as a virtual function in a super-class but with a different purpose. * clang-format * Upload logs on failure * Add override to unsupported.hpp * cmake: Make -Wsuggest-override flag conditional --- .github/workflows/ci.yml | 9 ++++++ cmake/cxx.cmake | 6 ++++ include/components/bar.hpp | 30 +++++++++--------- include/components/controller.hpp | 22 +++++++------- include/components/renderer.hpp | 32 ++++++++++---------- include/components/screen.hpp | 6 ++-- include/events/signal_receiver.hpp | 4 +-- include/modules/battery.hpp | 2 +- include/modules/bspwm.hpp | 2 +- include/modules/i3.hpp | 2 +- include/modules/ipc.hpp | 2 +- include/modules/meta/base.hpp | 20 ++++++------ include/modules/meta/event_module.hpp | 4 +-- include/modules/meta/inotify_module.hpp | 4 +-- include/modules/meta/static_module.hpp | 4 +-- include/modules/meta/timer_module.hpp | 2 +- include/modules/script.hpp | 4 +-- include/modules/unsupported.hpp | 22 +++++++------- include/modules/xbacklight.hpp | 2 +- include/modules/xkeyboard.hpp | 6 ++-- include/modules/xwindow.hpp | 2 +- include/modules/xworkspaces.hpp | 2 +- include/tags/parser.hpp | 2 +- include/utils/file.hpp | 6 ++-- include/x11/background_manager.hpp | 11 +++---- include/x11/connection.hpp | 9 +++--- include/x11/tray_manager.hpp | 28 ++++++++--------- tests/unit_tests/components/command_line.cpp | 28 ++++++++--------- 28 files changed, 145 insertions(+), 128 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index db2d2162..bdf02c2a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -111,3 +111,12 @@ jobs: make check cd $POLYBAR_DIR bash <(curl -s https://codecov.io/bash) -F unittests -a "-ap" -Z + - name: Upload Logs + if: failure() + uses: actions/upload-artifact@v2 + with: + name: cmake + path: | + build/CMakeFiles/CMakeError.log + build/CMakeFiles/CMakeOutput.log + retention-days: 5 diff --git a/cmake/cxx.cmake b/cmake/cxx.cmake index 751e1a60..b271e4b8 100644 --- a/cmake/cxx.cmake +++ b/cmake/cxx.cmake @@ -22,9 +22,15 @@ set(CMAKE_CXX_EXTENSIONS OFF) set(THREADS_PREFER_PTHREAD_FLAG ON) # Compiler flags +include(CheckCXXCompilerFlag) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wextra") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wpedantic") +check_cxx_compiler_flag("-Wsuggest-override" HAS_SUGGEST_OVERRIDE) +if (HAS_SUGGEST_OVERRIDE) + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wsuggest-override") +endif() +unset(HAS_SUGGEST_OVERRIDE) if (CMAKE_SYSTEM_NAME STREQUAL "FreeBSD") # Need dprintf() for FreeBSD 11.1 and older diff --git a/include/components/bar.hpp b/include/components/bar.hpp index 60416d1d..0c554678 100644 --- a/include/components/bar.hpp +++ b/include/components/bar.hpp @@ -86,23 +86,23 @@ class bar : public xpp::event::sink { } protected: - void handle(const evt::randr_screen_change_notify& evt); + void handle(const evt::randr_screen_change_notify& evt) override; private: connection& m_connection; @@ -45,7 +45,9 @@ class screen : public xpp::event::sink { xcb_window_t m_proxy{XCB_NONE}; vector m_monitors; - struct size m_size {0U, 0U}; + struct size m_size { + 0U, 0U + }; bool m_sigraised{false}; bool have_monitors_changed() const; diff --git a/include/events/signal_receiver.hpp b/include/events/signal_receiver.hpp index 9f2a8055..71cd6013 100644 --- a/include/events/signal_receiver.hpp +++ b/include/events/signal_receiver.hpp @@ -1,9 +1,9 @@ #pragma once #include -#include #include #include +#include #include "common.hpp" @@ -42,7 +42,7 @@ class signal_receiver : public signal_receiver_interface, public signal_receiver_impl, public signal_receiver_impl... { public: - prio priority() const { + prio priority() const override { return Priority; } }; diff --git a/include/modules/battery.hpp b/include/modules/battery.hpp index 583a6de1..83c7450f 100644 --- a/include/modules/battery.hpp +++ b/include/modules/battery.hpp @@ -48,7 +48,7 @@ namespace modules { public: explicit battery_module(const bar_settings&, string); - void start(); + void start() override; void teardown(); void idle(); bool on_event(inotify_event* event); diff --git a/include/modules/bspwm.hpp b/include/modules/bspwm.hpp index b8f45f32..056d36d9 100644 --- a/include/modules/bspwm.hpp +++ b/include/modules/bspwm.hpp @@ -41,7 +41,7 @@ namespace modules { public: explicit bspwm_module(const bar_settings&, string); - void stop(); + void stop() override; bool has_event(); bool update(); string get_output(); diff --git a/include/modules/i3.hpp b/include/modules/i3.hpp index bed2c4cf..6775169b 100644 --- a/include/modules/i3.hpp +++ b/include/modules/i3.hpp @@ -46,7 +46,7 @@ namespace modules { public: explicit i3_module(const bar_settings&, string); - void stop(); + void stop() override; bool has_event(); bool update(); bool build(builder* builder, const string& tag) const; diff --git a/include/modules/ipc.hpp b/include/modules/ipc.hpp index 63e4fe70..d0097cc1 100644 --- a/include/modules/ipc.hpp +++ b/include/modules/ipc.hpp @@ -26,7 +26,7 @@ namespace modules { public: explicit ipc_module(const bar_settings&, string); - void start(); + void start() override; void update() {} string get_output(); bool build(builder* builder, const string& tag) const; diff --git a/include/modules/meta/base.hpp b/include/modules/meta/base.hpp index 2a6855af..984e1610 100644 --- a/include/modules/meta/base.hpp +++ b/include/modules/meta/base.hpp @@ -145,21 +145,21 @@ namespace modules { module(const bar_settings bar, string name); ~module() noexcept; - string type() const; + string type() const override; - string name_raw() const; - string name() const; - bool running() const; + string name_raw() const override; + string name() const override; + bool running() const override; - bool visible() const; - void set_visible(bool value); + bool visible() const override; + void set_visible(bool value) override; - void stop(); - void halt(string error_message); + void stop() override; + void halt(string error_message) override; void teardown(); - string contents(); + string contents() override; - bool input(const string& action, const string& data) final; + bool input(const string& action, const string& data) final override; protected: void broadcast(); diff --git a/include/modules/meta/event_module.hpp b/include/modules/meta/event_module.hpp index 417ca3a4..025fdccd 100644 --- a/include/modules/meta/event_module.hpp +++ b/include/modules/meta/event_module.hpp @@ -10,7 +10,7 @@ namespace modules { public: using module::module; - void start() { + void start() override { this->m_mainthread = thread(&event_module::runner, this); } @@ -40,6 +40,6 @@ namespace modules { } } }; -} +} // namespace modules POLYBAR_NS_END diff --git a/include/modules/meta/inotify_module.hpp b/include/modules/meta/inotify_module.hpp index 0b174d3c..2c69e948 100644 --- a/include/modules/meta/inotify_module.hpp +++ b/include/modules/meta/inotify_module.hpp @@ -11,7 +11,7 @@ namespace modules { public: using module::module; - void start() { + void start() override { this->m_mainthread = thread(&inotify_module::runner, this); } @@ -88,6 +88,6 @@ namespace modules { private: map m_watchlist; }; -} +} // namespace modules POLYBAR_NS_END diff --git a/include/modules/meta/static_module.hpp b/include/modules/meta/static_module.hpp index cc30e8fa..8e53938c 100644 --- a/include/modules/meta/static_module.hpp +++ b/include/modules/meta/static_module.hpp @@ -10,7 +10,7 @@ namespace modules { public: using module::module; - void start() { + void start() override { this->m_mainthread = thread([&] { this->m_log.trace("%s: Thread id = %i", this->name(), concurrency_util::thread_id(this_thread::get_id())); CAST_MOD(Impl)->update(); @@ -22,6 +22,6 @@ namespace modules { return true; } }; -} +} // namespace modules POLYBAR_NS_END diff --git a/include/modules/meta/timer_module.hpp b/include/modules/meta/timer_module.hpp index 2cd5f3bf..e2ca716a 100644 --- a/include/modules/meta/timer_module.hpp +++ b/include/modules/meta/timer_module.hpp @@ -12,7 +12,7 @@ namespace modules { public: using module::module; - void start() { + void start() override { this->m_mainthread = thread(&timer_module::runner, this); } diff --git a/include/modules/script.hpp b/include/modules/script.hpp index 2a62d3a0..1ad83222 100644 --- a/include/modules/script.hpp +++ b/include/modules/script.hpp @@ -12,8 +12,8 @@ namespace modules { explicit script_module(const bar_settings&, string); ~script_module() {} - void start(); - void stop(); + void start() override; + void stop() override; string get_output(); bool build(builder* builder, const string& tag) const; diff --git a/include/modules/unsupported.hpp b/include/modules/unsupported.hpp index 28e53155..c54b57a4 100644 --- a/include/modules/unsupported.hpp +++ b/include/modules/unsupported.hpp @@ -15,29 +15,29 @@ namespace modules { throw application_error("No built-in support for '" + string{MODULE_TYPE} + "'"); \ } \ static constexpr auto TYPE = MODULE_TYPE; \ - string type() const { \ + string type() const override { \ return ""; \ } \ - string name_raw() const { \ + string name_raw() const override { \ return ""; \ } \ - string name() const { \ + string name() const override { \ return ""; \ } \ - bool running() const { \ + bool running() const override { \ return false; \ } \ - bool visible() const { \ + bool visible() const override { \ return false; \ } \ - void set_visible(bool) {} \ - void start() {} \ - void stop() {} \ - void halt(string) {} \ - string contents() { \ + void set_visible(bool) override {} \ + void start() override {} \ + void stop() override {} \ + void halt(string) override {} \ + string contents() override { \ return ""; \ } \ - bool input(const string&, const string&) { \ + bool input(const string&, const string&) override { \ return false; \ } \ } diff --git a/include/modules/xbacklight.hpp b/include/modules/xbacklight.hpp index 99cf63b9..269b4244 100644 --- a/include/modules/xbacklight.hpp +++ b/include/modules/xbacklight.hpp @@ -36,7 +36,7 @@ namespace modules { static constexpr const char* EVENT_DEC = "dec"; protected: - void handle(const evt::randr_notify& evt); + void handle(const evt::randr_notify& evt) override; void action_inc(); void action_dec(); diff --git a/include/modules/xkeyboard.hpp b/include/modules/xkeyboard.hpp index 44892d77..d8d11926 100644 --- a/include/modules/xkeyboard.hpp +++ b/include/modules/xkeyboard.hpp @@ -34,9 +34,9 @@ namespace modules { bool query_keyboard(); bool blacklisted(const string& indicator_name); - void handle(const evt::xkb_new_keyboard_notify& evt); - void handle(const evt::xkb_state_notify& evt); - void handle(const evt::xkb_indicator_state_notify& evt); + void handle(const evt::xkb_new_keyboard_notify& evt) override; + void handle(const evt::xkb_state_notify& evt) override; + void handle(const evt::xkb_indicator_state_notify& evt) override; void action_switch(); diff --git a/include/modules/xwindow.hpp b/include/modules/xwindow.hpp index 506504ed..80c45e44 100644 --- a/include/modules/xwindow.hpp +++ b/include/modules/xwindow.hpp @@ -39,7 +39,7 @@ namespace modules { static constexpr auto TYPE = "internal/xwindow"; protected: - void handle(const evt::property_notify& evt); + void handle(const evt::property_notify& evt) override; private: static constexpr const char* TAG_LABEL{"