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
This commit is contained in:
Patrick Ziegler 2021-01-04 10:38:43 +01:00 committed by GitHub
parent 26be83f893
commit d5be8cad97
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
28 changed files with 145 additions and 128 deletions

View File

@ -111,3 +111,12 @@ jobs:
make check make check
cd $POLYBAR_DIR cd $POLYBAR_DIR
bash <(curl -s https://codecov.io/bash) -F unittests -a "-ap" -Z 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

View File

@ -22,9 +22,15 @@ set(CMAKE_CXX_EXTENSIONS OFF)
set(THREADS_PREFER_PTHREAD_FLAG ON) set(THREADS_PREFER_PTHREAD_FLAG ON)
# Compiler flags # Compiler flags
include(CheckCXXCompilerFlag)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wextra") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wextra")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wpedantic") 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") if (CMAKE_SYSTEM_NAME STREQUAL "FreeBSD")
# Need dprintf() for FreeBSD 11.1 and older # Need dprintf() for FreeBSD 11.1 and older

View File

@ -86,23 +86,23 @@ class bar : public xpp::event::sink<evt::button_press, evt::expose, evt::propert
void reconfigure_wm_hints(); void reconfigure_wm_hints();
void broadcast_visibility(); void broadcast_visibility();
void handle(const evt::client_message& evt); void handle(const evt::client_message& evt) override;
void handle(const evt::destroy_notify& evt); void handle(const evt::destroy_notify& evt) override;
void handle(const evt::enter_notify& evt); void handle(const evt::enter_notify& evt) override;
void handle(const evt::leave_notify& evt); void handle(const evt::leave_notify& evt) override;
void handle(const evt::motion_notify& evt); void handle(const evt::motion_notify& evt) override;
void handle(const evt::button_press& evt); void handle(const evt::button_press& evt) override;
void handle(const evt::expose& evt); void handle(const evt::expose& evt) override;
void handle(const evt::property_notify& evt); void handle(const evt::property_notify& evt) override;
void handle(const evt::configure_notify& evt); void handle(const evt::configure_notify& evt) override;
bool on(const signals::eventqueue::start&); bool on(const signals::eventqueue::start&) override;
bool on(const signals::ui::unshade_window&); bool on(const signals::ui::unshade_window&) override;
bool on(const signals::ui::shade_window&); bool on(const signals::ui::shade_window&) override;
bool on(const signals::ui::tick&); bool on(const signals::ui::tick&) override;
bool on(const signals::ui::dim_window&); bool on(const signals::ui::dim_window&) override;
#if WITH_XCURSOR #if WITH_XCURSOR
bool on(const signals::ui::cursor_change&); bool on(const signals::ui::cursor_change&) override;
#endif #endif
private: private:

View File

@ -58,17 +58,17 @@ class controller
void process_inputdata(); void process_inputdata();
bool process_update(bool force); bool process_update(bool force);
bool on(const signals::eventqueue::notify_change& evt); bool on(const signals::eventqueue::notify_change& evt) override;
bool on(const signals::eventqueue::notify_forcechange& evt); bool on(const signals::eventqueue::notify_forcechange& evt) override;
bool on(const signals::eventqueue::exit_terminate& evt); bool on(const signals::eventqueue::exit_terminate& evt) override;
bool on(const signals::eventqueue::exit_reload& evt); bool on(const signals::eventqueue::exit_reload& evt) override;
bool on(const signals::eventqueue::check_state& evt); bool on(const signals::eventqueue::check_state& evt) override;
bool on(const signals::ui::ready& evt); bool on(const signals::ui::ready& evt) override;
bool on(const signals::ui::button_press& evt); bool on(const signals::ui::button_press& evt) override;
bool on(const signals::ipc::action& evt); bool on(const signals::ipc::action& evt) override;
bool on(const signals::ipc::command& evt); bool on(const signals::ipc::command& evt) override;
bool on(const signals::ipc::hook& evt); bool on(const signals::ipc::hook& evt) override;
bool on(const signals::ui::update_background& evt); bool on(const signals::ui::update_background& evt) override;
private: private:
size_t setup_modules(alignment align); size_t setup_modules(alignment align);

View File

@ -71,22 +71,22 @@ class renderer
void flush(alignment a); void flush(alignment a);
void highlight_clickable_areas(); void highlight_clickable_areas();
bool on(const signals::ui::request_snapshot& evt); bool on(const signals::ui::request_snapshot& evt) override;
bool on(const signals::parser::change_background& evt); bool on(const signals::parser::change_background& evt) override;
bool on(const signals::parser::change_foreground& evt); bool on(const signals::parser::change_foreground& evt) override;
bool on(const signals::parser::change_underline& evt); bool on(const signals::parser::change_underline& evt) override;
bool on(const signals::parser::change_overline& evt); bool on(const signals::parser::change_overline& evt) override;
bool on(const signals::parser::change_font& evt); bool on(const signals::parser::change_font& evt) override;
bool on(const signals::parser::change_alignment& evt); bool on(const signals::parser::change_alignment& evt) override;
bool on(const signals::parser::reverse_colors&); bool on(const signals::parser::reverse_colors&) override;
bool on(const signals::parser::offset_pixel& evt); bool on(const signals::parser::offset_pixel& evt) override;
bool on(const signals::parser::attribute_set& evt); bool on(const signals::parser::attribute_set& evt) override;
bool on(const signals::parser::attribute_unset& evt); bool on(const signals::parser::attribute_unset& evt) override;
bool on(const signals::parser::attribute_toggle& evt); bool on(const signals::parser::attribute_toggle& evt) override;
bool on(const signals::parser::action_begin& evt); bool on(const signals::parser::action_begin& evt) override;
bool on(const signals::parser::action_end& evt); bool on(const signals::parser::action_end& evt) override;
bool on(const signals::parser::text& evt); bool on(const signals::parser::text& evt) override;
bool on(const signals::parser::control& evt); bool on(const signals::parser::control& evt) override;
protected: protected:
struct reserve_area { struct reserve_area {

View File

@ -33,7 +33,7 @@ class screen : public xpp::event::sink<evt::randr_screen_change_notify> {
} }
protected: protected:
void handle(const evt::randr_screen_change_notify& evt); void handle(const evt::randr_screen_change_notify& evt) override;
private: private:
connection& m_connection; connection& m_connection;
@ -45,7 +45,9 @@ class screen : public xpp::event::sink<evt::randr_screen_change_notify> {
xcb_window_t m_proxy{XCB_NONE}; xcb_window_t m_proxy{XCB_NONE};
vector<monitor_t> m_monitors; vector<monitor_t> m_monitors;
struct size m_size {0U, 0U}; struct size m_size {
0U, 0U
};
bool m_sigraised{false}; bool m_sigraised{false};
bool have_monitors_changed() const; bool have_monitors_changed() const;

View File

@ -1,9 +1,9 @@
#pragma once #pragma once
#include <map> #include <map>
#include <unordered_map>
#include <typeindex> #include <typeindex>
#include <typeinfo> #include <typeinfo>
#include <unordered_map>
#include "common.hpp" #include "common.hpp"
@ -42,7 +42,7 @@ class signal_receiver : public signal_receiver_interface,
public signal_receiver_impl<Signal>, public signal_receiver_impl<Signal>,
public signal_receiver_impl<Signals>... { public signal_receiver_impl<Signals>... {
public: public:
prio priority() const { prio priority() const override {
return Priority; return Priority;
} }
}; };

View File

@ -48,7 +48,7 @@ namespace modules {
public: public:
explicit battery_module(const bar_settings&, string); explicit battery_module(const bar_settings&, string);
void start(); void start() override;
void teardown(); void teardown();
void idle(); void idle();
bool on_event(inotify_event* event); bool on_event(inotify_event* event);

View File

@ -41,7 +41,7 @@ namespace modules {
public: public:
explicit bspwm_module(const bar_settings&, string); explicit bspwm_module(const bar_settings&, string);
void stop(); void stop() override;
bool has_event(); bool has_event();
bool update(); bool update();
string get_output(); string get_output();

View File

@ -46,7 +46,7 @@ namespace modules {
public: public:
explicit i3_module(const bar_settings&, string); explicit i3_module(const bar_settings&, string);
void stop(); void stop() override;
bool has_event(); bool has_event();
bool update(); bool update();
bool build(builder* builder, const string& tag) const; bool build(builder* builder, const string& tag) const;

View File

@ -26,7 +26,7 @@ namespace modules {
public: public:
explicit ipc_module(const bar_settings&, string); explicit ipc_module(const bar_settings&, string);
void start(); void start() override;
void update() {} void update() {}
string get_output(); string get_output();
bool build(builder* builder, const string& tag) const; bool build(builder* builder, const string& tag) const;

View File

@ -145,21 +145,21 @@ namespace modules {
module(const bar_settings bar, string name); module(const bar_settings bar, string name);
~module() noexcept; ~module() noexcept;
string type() const; string type() const override;
string name_raw() const; string name_raw() const override;
string name() const; string name() const override;
bool running() const; bool running() const override;
bool visible() const; bool visible() const override;
void set_visible(bool value); void set_visible(bool value) override;
void stop(); void stop() override;
void halt(string error_message); void halt(string error_message) override;
void teardown(); 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: protected:
void broadcast(); void broadcast();

View File

@ -10,7 +10,7 @@ namespace modules {
public: public:
using module<Impl>::module; using module<Impl>::module;
void start() { void start() override {
this->m_mainthread = thread(&event_module::runner, this); this->m_mainthread = thread(&event_module::runner, this);
} }
@ -40,6 +40,6 @@ namespace modules {
} }
} }
}; };
} } // namespace modules
POLYBAR_NS_END POLYBAR_NS_END

View File

@ -11,7 +11,7 @@ namespace modules {
public: public:
using module<Impl>::module; using module<Impl>::module;
void start() { void start() override {
this->m_mainthread = thread(&inotify_module::runner, this); this->m_mainthread = thread(&inotify_module::runner, this);
} }
@ -88,6 +88,6 @@ namespace modules {
private: private:
map<string, int> m_watchlist; map<string, int> m_watchlist;
}; };
} } // namespace modules
POLYBAR_NS_END POLYBAR_NS_END

View File

@ -10,7 +10,7 @@ namespace modules {
public: public:
using module<Impl>::module; using module<Impl>::module;
void start() { void start() override {
this->m_mainthread = thread([&] { this->m_mainthread = thread([&] {
this->m_log.trace("%s: Thread id = %i", this->name(), concurrency_util::thread_id(this_thread::get_id())); this->m_log.trace("%s: Thread id = %i", this->name(), concurrency_util::thread_id(this_thread::get_id()));
CAST_MOD(Impl)->update(); CAST_MOD(Impl)->update();
@ -22,6 +22,6 @@ namespace modules {
return true; return true;
} }
}; };
} } // namespace modules
POLYBAR_NS_END POLYBAR_NS_END

View File

@ -12,7 +12,7 @@ namespace modules {
public: public:
using module<Impl>::module; using module<Impl>::module;
void start() { void start() override {
this->m_mainthread = thread(&timer_module::runner, this); this->m_mainthread = thread(&timer_module::runner, this);
} }

View File

@ -12,8 +12,8 @@ namespace modules {
explicit script_module(const bar_settings&, string); explicit script_module(const bar_settings&, string);
~script_module() {} ~script_module() {}
void start(); void start() override;
void stop(); void stop() override;
string get_output(); string get_output();
bool build(builder* builder, const string& tag) const; bool build(builder* builder, const string& tag) const;

View File

@ -15,29 +15,29 @@ namespace modules {
throw application_error("No built-in support for '" + string{MODULE_TYPE} + "'"); \ throw application_error("No built-in support for '" + string{MODULE_TYPE} + "'"); \
} \ } \
static constexpr auto TYPE = MODULE_TYPE; \ static constexpr auto TYPE = MODULE_TYPE; \
string type() const { \ string type() const override { \
return ""; \ return ""; \
} \ } \
string name_raw() const { \ string name_raw() const override { \
return ""; \ return ""; \
} \ } \
string name() const { \ string name() const override { \
return ""; \ return ""; \
} \ } \
bool running() const { \ bool running() const override { \
return false; \ return false; \
} \ } \
bool visible() const { \ bool visible() const override { \
return false; \ return false; \
} \ } \
void set_visible(bool) {} \ void set_visible(bool) override {} \
void start() {} \ void start() override {} \
void stop() {} \ void stop() override {} \
void halt(string) {} \ void halt(string) override {} \
string contents() { \ string contents() override { \
return ""; \ return ""; \
} \ } \
bool input(const string&, const string&) { \ bool input(const string&, const string&) override { \
return false; \ return false; \
} \ } \
} }

View File

@ -36,7 +36,7 @@ namespace modules {
static constexpr const char* EVENT_DEC = "dec"; static constexpr const char* EVENT_DEC = "dec";
protected: protected:
void handle(const evt::randr_notify& evt); void handle(const evt::randr_notify& evt) override;
void action_inc(); void action_inc();
void action_dec(); void action_dec();

View File

@ -34,9 +34,9 @@ namespace modules {
bool query_keyboard(); bool query_keyboard();
bool blacklisted(const string& indicator_name); bool blacklisted(const string& indicator_name);
void handle(const evt::xkb_new_keyboard_notify& evt); void handle(const evt::xkb_new_keyboard_notify& evt) override;
void handle(const evt::xkb_state_notify& evt); void handle(const evt::xkb_state_notify& evt) override;
void handle(const evt::xkb_indicator_state_notify& evt); void handle(const evt::xkb_indicator_state_notify& evt) override;
void action_switch(); void action_switch();

View File

@ -39,7 +39,7 @@ namespace modules {
static constexpr auto TYPE = "internal/xwindow"; static constexpr auto TYPE = "internal/xwindow";
protected: protected:
void handle(const evt::property_notify& evt); void handle(const evt::property_notify& evt) override;
private: private:
static constexpr const char* TAG_LABEL{"<label>"}; static constexpr const char* TAG_LABEL{"<label>"};

View File

@ -65,7 +65,7 @@ namespace modules {
static constexpr auto EVENT_PREV = "prev"; static constexpr auto EVENT_PREV = "prev";
protected: protected:
void handle(const evt::property_notify& evt); void handle(const evt::property_notify& evt) override;
void rebuild_clientlist(); void rebuild_clientlist();
void rebuild_urgent_hints(); void rebuild_urgent_hints();

View File

@ -24,7 +24,7 @@ namespace tags {
msg.append(" (Context: '" + ctxt + "')"); msg.append(" (Context: '" + ctxt + "')");
} }
virtual const char* what() const noexcept { virtual const char* what() const noexcept override {
return msg.c_str(); return msg.c_str();
} }

View File

@ -67,9 +67,9 @@ class fd_streambuf : public std::streambuf {
void close(); void close();
protected: protected:
int sync(); int sync() override;
int overflow(int c); int overflow(int c) override;
int underflow(); int underflow() override;
private: private:
file_descriptor m_fd; file_descriptor m_fd;

View File

@ -17,7 +17,7 @@ class logger;
namespace cairo { namespace cairo {
class surface; class surface;
class xcb_surface; class xcb_surface;
} } // namespace cairo
class bg_slice { class bg_slice {
public: public:
@ -65,8 +65,7 @@ class bg_slice {
* so this class takes a rectangle that limits what part of the background is stored. * so this class takes a rectangle that limits what part of the background is stored.
*/ */
class background_manager : public signal_receiver<SIGN_PRIORITY_SCREEN, signals::ui::update_geometry>, class background_manager : public signal_receiver<SIGN_PRIORITY_SCREEN, signals::ui::update_geometry>,
public xpp::event::sink<evt::property_notify> public xpp::event::sink<evt::property_notify> {
{
public: public:
using make_type = background_manager&; using make_type = background_manager&;
static make_type make(); static make_type make();
@ -96,8 +95,9 @@ class background_manager : public signal_receiver<SIGN_PRIORITY_SCREEN, signals:
*/ */
std::shared_ptr<bg_slice> observe(xcb_rectangle_t rect, xcb_window_t window); std::shared_ptr<bg_slice> observe(xcb_rectangle_t rect, xcb_window_t window);
void handle(const evt::property_notify& evt); void handle(const evt::property_notify& evt) override;
bool on(const signals::ui::update_geometry&); bool on(const signals::ui::update_geometry&) override;
private: private:
void activate(); void activate();
void deactivate(); void deactivate();
@ -119,7 +119,6 @@ class background_manager : public signal_receiver<SIGN_PRIORITY_SCREEN, signals:
void allocate_resources(); void allocate_resources();
void free_resources(); void free_resources();
void fetch_root_pixmap(); void fetch_root_pixmap();
}; };
POLYBAR_NS_END POLYBAR_NS_END

View File

@ -1,6 +1,7 @@
#pragma once #pragma once
#include <xcb/xcb.h> #include <xcb/xcb.h>
#include <cstdlib> #include <cstdlib>
#include <xpp/core.hpp> #include <xpp/core.hpp>
#include <xpp/generic/factory.hpp> #include <xpp/generic/factory.hpp>
@ -44,7 +45,7 @@ namespace detail {
virtual ~connection_base() {} virtual ~connection_base() {}
void operator()(const shared_ptr<xcb_generic_error_t>& error) const { void operator()(const shared_ptr<xcb_generic_error_t>& error) const override {
check<xpp::x::extension, Extensions...>(error); check<xpp::x::extension, Extensions...>(error);
} }
@ -59,7 +60,7 @@ namespace detail {
return make()(*this, m_root_window); return make()(*this, m_root_window);
} }
shared_ptr<xcb_generic_event_t> wait_for_event() const { shared_ptr<xcb_generic_event_t> wait_for_event() const override {
try { try {
return core::wait_for_event(); return core::wait_for_event();
} catch (const shared_ptr<xcb_generic_error_t>& error) { } catch (const shared_ptr<xcb_generic_error_t>& error) {
@ -68,7 +69,7 @@ namespace detail {
throw; // re-throw exception throw; // re-throw exception
} }
shared_ptr<xcb_generic_event_t> wait_for_special_event(xcb_special_event_t* se) const { shared_ptr<xcb_generic_event_t> wait_for_special_event(xcb_special_event_t* se) const override {
try { try {
return core::wait_for_special_event(se); return core::wait_for_special_event(se);
} catch (const shared_ptr<xcb_generic_error_t>& error) { } catch (const shared_ptr<xcb_generic_error_t>& error) {
@ -93,7 +94,7 @@ namespace detail {
dispatcher(error); dispatcher(error);
} }
}; };
} } // namespace detail
class connection : public detail::connection_base<connection&, XPP_EXTENSION_LIST> { class connection : public detail::connection_base<connection&, XPP_EXTENSION_LIST> {
public: public:

View File

@ -120,21 +120,21 @@ class tray_manager : public xpp::event::sink<evt::expose, evt::visibility_notify
void remove_client(xcb_window_t win, bool reconfigure = true); void remove_client(xcb_window_t win, bool reconfigure = true);
unsigned int mapped_clients() const; unsigned int mapped_clients() const;
void handle(const evt::expose& evt); void handle(const evt::expose& evt) override;
void handle(const evt::visibility_notify& evt); void handle(const evt::visibility_notify& evt) override;
void handle(const evt::client_message& evt); void handle(const evt::client_message& evt) override;
void handle(const evt::configure_request& evt); void handle(const evt::configure_request& evt) override;
void handle(const evt::resize_request& evt); void handle(const evt::resize_request& evt) override;
void handle(const evt::selection_clear& evt); void handle(const evt::selection_clear& evt) override;
void handle(const evt::property_notify& evt); void handle(const evt::property_notify& evt) override;
void handle(const evt::reparent_notify& evt); void handle(const evt::reparent_notify& evt) override;
void handle(const evt::destroy_notify& evt); void handle(const evt::destroy_notify& evt) override;
void handle(const evt::map_notify& evt); void handle(const evt::map_notify& evt) override;
void handle(const evt::unmap_notify& evt); void handle(const evt::unmap_notify& evt) override;
bool on(const signals::ui::visibility_change& evt); bool on(const signals::ui::visibility_change& evt) override;
bool on(const signals::ui::dim_window& evt); bool on(const signals::ui::dim_window& evt) override;
bool on(const signals::ui::update_background& evt); bool on(const signals::ui::update_background& evt) override;
private: private:
connection& m_connection; connection& m_connection;

View File

@ -1,32 +1,32 @@
#include "common/test.hpp"
#include "components/command_line.hpp" #include "components/command_line.hpp"
#include "common/test.hpp"
#include "utils/string.hpp" #include "utils/string.hpp"
using namespace polybar; using namespace polybar;
class CommandLine : public ::testing::Test { class CommandLine : public ::testing::Test {
protected: protected:
virtual void SetUp() { virtual void SetUp() override {
set_cli(); set_cli();
} }
virtual void set_cli() { virtual void set_cli() {
cli = command_line::parser::make("cmd", get_opts()); cli = command_line::parser::make("cmd", get_opts());
} }
command_line::options get_opts() { command_line::options get_opts() {
// clang-format off // clang-format off
return command_line::options { return command_line::options {
command_line::option{"-f", "--flag", "Flag description"}, command_line::option{"-f", "--flag", "Flag description"},
command_line::option{"-o", "--option", "Option description", "OPTION", {"foo", "bar", "baz"}}, command_line::option{"-o", "--option", "Option description", "OPTION", {"foo", "bar", "baz"}},
}; };
// clang-format on // clang-format on
}; };
command_line::parser::make_type cli; command_line::parser::make_type cli;
}; };
TEST_F(CommandLine, hasShort) { TEST_F(CommandLine, hasShort) {
cli->process_input(string_util::split("-f", ' ')); cli->process_input(string_util::split("-f", ' '));
EXPECT_TRUE(cli->has("flag")); EXPECT_TRUE(cli->has("flag"));