From 63e50a4b56a2e21e854636a622941fd36d2e78da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Thu, 25 Jul 2024 07:32:03 +0200 Subject: [PATCH] add werror=suggest-override Improves code readability by making overrides explicit. Inspired by lix code-base --- Makefile | 2 +- build-utils-meson/diagnostics/meson.build | 1 + src/libexpr/json-to-value.cc | 26 +++++++++---------- src/libexpr/nixexpr.hh | 4 +-- src/libstore/daemon.cc | 2 +- .../unix/build/local-derivation-goal.cc | 2 +- src/libutil/serialise.cc | 4 +-- src/libutil/serialise.hh | 6 ++--- src/perl/meson.build | 1 + 9 files changed, 25 insertions(+), 23 deletions(-) diff --git a/Makefile b/Makefile index bb64a104e..ac76b1532 100644 --- a/Makefile +++ b/Makefile @@ -92,7 +92,7 @@ ifdef HOST_WINDOWS GLOBAL_LDFLAGS += -Wl,--export-all-symbols endif -GLOBAL_CXXFLAGS += -g -Wall -Wdeprecated-copy -Wignored-qualifiers -Wimplicit-fallthrough -Werror=unused-result -include $(buildprefix)config.h -std=c++2a -I src +GLOBAL_CXXFLAGS += -g -Wall -Wdeprecated-copy -Wignored-qualifiers -Wimplicit-fallthrough -Werror=unused-result -Werror=suggest-override -include $(buildprefix)config.h -std=c++2a -I src # Include the main lib, causing rules to be defined diff --git a/build-utils-meson/diagnostics/meson.build b/build-utils-meson/diagnostics/meson.build index e81f19eff..30eedfc13 100644 --- a/build-utils-meson/diagnostics/meson.build +++ b/build-utils-meson/diagnostics/meson.build @@ -1,5 +1,6 @@ add_project_arguments( '-Wdeprecated-copy', + '-Werror=suggest-override', '-Werror=switch', '-Werror=switch-enum', '-Werror=unused-result', diff --git a/src/libexpr/json-to-value.cc b/src/libexpr/json-to-value.cc index 20bee193f..33ab55ee9 100644 --- a/src/libexpr/json-to-value.cc +++ b/src/libexpr/json-to-value.cc @@ -80,42 +80,42 @@ class JSONSax : nlohmann::json_sax { public: JSONSax(EvalState & state, Value & v) : state(state), rs(new JSONState(&v)) {}; - bool null() + bool null() override { rs->value(state).mkNull(); rs->add(); return true; } - bool boolean(bool val) + bool boolean(bool val) override { rs->value(state).mkBool(val); rs->add(); return true; } - bool number_integer(number_integer_t val) + bool number_integer(number_integer_t val) override { rs->value(state).mkInt(val); rs->add(); return true; } - bool number_unsigned(number_unsigned_t val) + bool number_unsigned(number_unsigned_t val) override { rs->value(state).mkInt(val); rs->add(); return true; } - bool number_float(number_float_t val, const string_t & s) + bool number_float(number_float_t val, const string_t & s) override { rs->value(state).mkFloat(val); rs->add(); return true; } - bool string(string_t & val) + bool string(string_t & val) override { rs->value(state).mkString(val); rs->add(); @@ -123,7 +123,7 @@ public: } #if NLOHMANN_JSON_VERSION_MAJOR >= 3 && NLOHMANN_JSON_VERSION_MINOR >= 8 - bool binary(binary_t&) + bool binary(binary_t&) override { // This function ought to be unreachable assert(false); @@ -131,35 +131,35 @@ public: } #endif - bool start_object(std::size_t len) + bool start_object(std::size_t len) override { rs = std::make_unique(std::move(rs)); return true; } - bool key(string_t & name) + bool key(string_t & name) override { dynamic_cast(rs.get())->key(name, state); return true; } - bool end_object() { + bool end_object() override { rs = rs->resolve(state); rs->add(); return true; } - bool end_array() { + bool end_array() override { return end_object(); } - bool start_array(size_t len) { + bool start_array(size_t len) override { rs = std::make_unique(std::move(rs), len != std::numeric_limits::max() ? len : 128); return true; } - bool parse_error(std::size_t, const std::string&, const nlohmann::detail::exception& ex) { + bool parse_error(std::size_t, const std::string&, const nlohmann::detail::exception& ex) override { throw JSONParseError("%s", ex.what()); } }; diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index 1bcc962c5..3279e3d48 100644 --- a/src/libexpr/nixexpr.hh +++ b/src/libexpr/nixexpr.hh @@ -186,7 +186,7 @@ struct ExprInheritFrom : ExprVar this->fromWith = nullptr; } - void bindVars(EvalState & es, const std::shared_ptr & env); + void bindVars(EvalState & es, const std::shared_ptr & env) override; }; struct ExprSelect : Expr @@ -203,7 +203,7 @@ struct ExprSelect : Expr * * @param[out] v The attribute set that should contain the last attribute name (if it exists). * @return The last attribute name in `attrPath` - * + * * @note This does *not* evaluate the final attribute, and does not fail if that's the only attribute that does not exist. */ Symbol evalExceptFinalSelect(EvalState & state, Env & env, Value & attrs); diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 6533b2f58..f28f92fce 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -167,7 +167,7 @@ struct TunnelSink : Sink { Sink & to; TunnelSink(Sink & to) : to(to) { } - void operator () (std::string_view data) + void operator () (std::string_view data) override { to << STDERR_WRITE; writeString(data, to); diff --git a/src/libstore/unix/build/local-derivation-goal.cc b/src/libstore/unix/build/local-derivation-goal.cc index 0dd102200..d30caaf51 100644 --- a/src/libstore/unix/build/local-derivation-goal.cc +++ b/src/libstore/unix/build/local-derivation-goal.cc @@ -1258,7 +1258,7 @@ bool LocalDerivationGoal::isAllowed(const DerivedPath & req) struct RestrictedStoreConfig : virtual LocalFSStoreConfig { using LocalFSStoreConfig::LocalFSStoreConfig; - const std::string name() { return "Restricted Store"; } + const std::string name() override { return "Restricted Store"; } }; /* A wrapper around LocalStore that only allows building/querying of diff --git a/src/libutil/serialise.cc b/src/libutil/serialise.cc index 36b99905a..1be7fa37a 100644 --- a/src/libutil/serialise.cc +++ b/src/libutil/serialise.cc @@ -190,11 +190,11 @@ struct VirtualStackAllocator { class DefaultStackAllocator : public StackAllocator { boost::coroutines2::default_stack stack; - boost::context::stack_context allocate() { + boost::context::stack_context allocate() override { return stack.allocate(); } - void deallocate(boost::context::stack_context sctx) { + void deallocate(boost::context::stack_context sctx) override { stack.deallocate(sctx); } }; diff --git a/src/libutil/serialise.hh b/src/libutil/serialise.hh index 8137db5f4..c7290dcef 100644 --- a/src/libutil/serialise.hh +++ b/src/libutil/serialise.hh @@ -204,7 +204,7 @@ struct TeeSink : Sink { Sink & sink1, & sink2; TeeSink(Sink & sink1, Sink & sink2) : sink1(sink1), sink2(sink2) { } - virtual void operator () (std::string_view data) + virtual void operator () (std::string_view data) override { sink1(data); sink2(data); @@ -221,7 +221,7 @@ struct TeeSource : Source Sink & sink; TeeSource(Source & orig, Sink & sink) : orig(orig), sink(sink) { } - size_t read(char * data, size_t len) + size_t read(char * data, size_t len) override { size_t n = orig.read(data, len); sink({data, n}); @@ -238,7 +238,7 @@ struct SizedSource : Source size_t remain; SizedSource(Source & orig, size_t size) : orig(orig), remain(size) { } - size_t read(char * data, size_t len) + size_t read(char * data, size_t len) override { if (this->remain <= 0) { throw EndOfFile("sized: unexpected end-of-file"); diff --git a/src/perl/meson.build b/src/perl/meson.build index 02e0e68e5..dcb6a68a4 100644 --- a/src/perl/meson.build +++ b/src/perl/meson.build @@ -25,6 +25,7 @@ nix_perl_conf.set('PACKAGE_VERSION', meson.project_version()) error_args = [ '-Wdeprecated-copy', '-Wdeprecated-declarations', + '-Werror=suggest-override', '-Werror=unused-result', '-Wignored-qualifiers', '-Wno-duplicate-decl-specifier',