From 15afa8472e1b1bbf236d4cf8e9f399345c48d3fe Mon Sep 17 00:00:00 2001 From: Vincent Ambo Date: Sun, 19 Jul 2020 21:52:35 +0100 Subject: fix(3p/nix): Fix all remaining compiler warnings This compiles under `-Wall -Werror`. The largest chunk of this change is `final` qualifiers for the various Nix CLI command structs, which inherit from a Command class that has more virtual functions than are implemented by each command. Change-Id: I0925e6e1a39013f026773db5816e4a77d50f3b4a Reviewed-on: https://cl.tvl.fyi/c/depot/+/1294 Tested-by: BuildkiteCI Reviewed-by: isomer Reviewed-by: kanepyork --- third_party/nix/src/libexpr/eval.cc | 1 - third_party/nix/src/libexpr/parser.hh | 11 +++++++++++ third_party/nix/src/libexpr/parser.y | 5 ++++- third_party/nix/src/libexpr/primops.cc | 2 -- third_party/nix/src/libstore/local-store.cc | 8 ++++++++ third_party/nix/src/libstore/nar-info-disk-cache.cc | 7 ++++--- third_party/nix/src/libstore/nar-info-disk-cache.hh | 2 +- third_party/nix/src/libutil/util.hh | 7 +++++++ third_party/nix/src/nix/add-to-store.cc | 2 +- third_party/nix/src/nix/build.cc | 2 +- third_party/nix/src/nix/cat.cc | 4 ++-- third_party/nix/src/nix/copy.cc | 2 +- third_party/nix/src/nix/doctor.cc | 2 +- third_party/nix/src/nix/dump-path.cc | 2 +- third_party/nix/src/nix/edit.cc | 2 +- third_party/nix/src/nix/eval.cc | 2 +- third_party/nix/src/nix/hash.cc | 4 ++-- third_party/nix/src/nix/installables.cc | 6 +++--- third_party/nix/src/nix/log.cc | 2 +- third_party/nix/src/nix/ls.cc | 4 ++-- third_party/nix/src/nix/optimise-store.cc | 2 +- third_party/nix/src/nix/path-info.cc | 2 +- third_party/nix/src/nix/ping-store.cc | 2 +- third_party/nix/src/nix/repl.cc | 2 +- third_party/nix/src/nix/run.cc | 2 +- third_party/nix/src/nix/search.cc | 2 +- third_party/nix/src/nix/show-config.cc | 2 +- third_party/nix/src/nix/show-derivation.cc | 2 +- third_party/nix/src/nix/sigs.cc | 4 ++-- third_party/nix/src/nix/upgrade-nix.cc | 2 +- third_party/nix/src/nix/verify.cc | 2 +- third_party/nix/src/nix/why-depends.cc | 2 +- third_party/nix/src/tests/language-tests.cc | 2 +- 33 files changed, 66 insertions(+), 39 deletions(-) diff --git a/third_party/nix/src/libexpr/eval.cc b/third_party/nix/src/libexpr/eval.cc index 704a05b197..5f267f2053 100644 --- a/third_party/nix/src/libexpr/eval.cc +++ b/third_party/nix/src/libexpr/eval.cc @@ -1252,7 +1252,6 @@ void EvalState::concatLists(Value& v, const NixList& lists, const Pos& pos) { auto outlist = new (GC) NixList(); - size_t len = 0; for (Value* list : lists) { forceList(*list, pos); outlist->insert(outlist->end(), list->list->begin(), list->list->end()); diff --git a/third_party/nix/src/libexpr/parser.hh b/third_party/nix/src/libexpr/parser.hh index 7592e3f82c..7b8f955731 100644 --- a/third_party/nix/src/libexpr/parser.hh +++ b/third_party/nix/src/libexpr/parser.hh @@ -34,6 +34,15 @@ struct ParseData : public gc { sLetBody(symbols.Create("")){}; }; +// Clang fails to identify these functions as used, probably because +// of some interaction between the lexer/parser codegen and something +// else. +// +// To avoid warnings for that we disable -Wunused-function in this block. + +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wunused-function" + // TODO(tazjin): move dupAttr to anonymous namespace static void dupAttr(const AttrPath& attrPath, const Pos& pos, const Pos& prevPos) { @@ -85,4 +94,6 @@ static Expr* unescapeStr(SymbolTable& symbols, const char* s, size_t length) { return new ExprString(symbols.Create(t)); } +#pragma clang diagnostic pop // re-enable -Wunused-function + } // namespace nix diff --git a/third_party/nix/src/libexpr/parser.y b/third_party/nix/src/libexpr/parser.y index 56674ee7ed..9998721488 100644 --- a/third_party/nix/src/libexpr/parser.y +++ b/third_party/nix/src/libexpr/parser.y @@ -11,7 +11,10 @@ %expect 1 %expect-rr 1 -%code requires { #include "libexpr/parser.hh" } +%code requires { +#define YY_NO_INPUT 1 // disable unused yyinput features +#include "libexpr/parser.hh" +} %{ diff --git a/third_party/nix/src/libexpr/primops.cc b/third_party/nix/src/libexpr/primops.cc index fcd6914424..107cacb963 100644 --- a/third_party/nix/src/libexpr/primops.cc +++ b/third_party/nix/src/libexpr/primops.cc @@ -1680,8 +1680,6 @@ static void prim_partition(EvalState& state, const Pos& pos, Value** args, state.forceFunction(*args[0], pos); state.forceList(*args[1], pos); - auto len = args[1]->listSize(); - NixList* right = new (GC) NixList(); NixList* wrong = new (GC) NixList(); diff --git a/third_party/nix/src/libstore/local-store.cc b/third_party/nix/src/libstore/local-store.cc index e7746973f9..995bc4f998 100644 --- a/third_party/nix/src/libstore/local-store.cc +++ b/third_party/nix/src/libstore/local-store.cc @@ -490,10 +490,18 @@ static void canonicalisePathMetaData_(const Path& path, uid_t fromUid, if (inodesSeen.find(Inode(st.st_dev, st.st_ino)) == inodesSeen.end()) { throw BuildError(format("invalid ownership on file '%1%'") % path); } + +// `mode` variable is only used in debug builds +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wunused-variable" + mode_t mode = st.st_mode & ~S_IFMT; assert(S_ISLNK(st.st_mode) || (st.st_uid == geteuid() && (mode == 0444 || mode == 0555) && st.st_mtime == mtimeStore)); + +#pragma clang diagnostic pop + return; } diff --git a/third_party/nix/src/libstore/nar-info-disk-cache.cc b/third_party/nix/src/libstore/nar-info-disk-cache.cc index c98882bff3..be2ad8f2da 100644 --- a/third_party/nix/src/libstore/nar-info-disk-cache.cc +++ b/third_party/nix/src/libstore/nar-info-disk-cache.cc @@ -49,7 +49,7 @@ create table if not exists LastPurge ( )sql"; -class NarInfoDiskCacheImpl : public NarInfoDiskCache { +class NarInfoDiskCacheImpl final : public NarInfoDiskCache { public: /* How often to purge expired entries from the cache. */ const int purgeInterval = 24 * 3600; @@ -280,8 +280,9 @@ class NarInfoDiskCacheImpl : public NarInfoDiskCache { } }; -ref getNarInfoDiskCache() { - static ref cache = make_ref(); +std::shared_ptr getNarInfoDiskCache() { + static std::shared_ptr cache = + std::make_shared(); return cache; } diff --git a/third_party/nix/src/libstore/nar-info-disk-cache.hh b/third_party/nix/src/libstore/nar-info-disk-cache.hh index b4107721aa..8eeab7635a 100644 --- a/third_party/nix/src/libstore/nar-info-disk-cache.hh +++ b/third_party/nix/src/libstore/nar-info-disk-cache.hh @@ -25,6 +25,6 @@ class NarInfoDiskCache { /* Return a singleton cache object that can be used concurrently by multiple threads. */ -ref getNarInfoDiskCache(); +std::shared_ptr getNarInfoDiskCache(); } // namespace nix diff --git a/third_party/nix/src/libutil/util.hh b/third_party/nix/src/libutil/util.hh index c39a6f06a5..ede83164de 100644 --- a/third_party/nix/src/libutil/util.hh +++ b/third_party/nix/src/libutil/util.hh @@ -390,6 +390,11 @@ class Callback { } } +// The unused-variable assert is disabled in this block because the +// `prev` variables are only used in debug mode (in the asserts). +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wunused-variable" + void operator()(T&& t) noexcept { auto prev = done.test_and_set(); assert(!prev); @@ -406,6 +411,8 @@ class Callback { promise.set_exception(exc); fun(promise.get_future()); } + +#pragma clang diagnostic pop }; /* Start a thread that handles various signals. Also block those signals diff --git a/third_party/nix/src/nix/add-to-store.cc b/third_party/nix/src/nix/add-to-store.cc index f99fa30be0..34f6d03963 100644 --- a/third_party/nix/src/nix/add-to-store.cc +++ b/third_party/nix/src/nix/add-to-store.cc @@ -5,7 +5,7 @@ using namespace nix; -struct CmdAddToStore : MixDryRun, StoreCommand { +struct CmdAddToStore final : MixDryRun, StoreCommand { Path path; std::optional namePart; diff --git a/third_party/nix/src/nix/build.cc b/third_party/nix/src/nix/build.cc index d5bdb7a379..b0e8feefd3 100644 --- a/third_party/nix/src/nix/build.cc +++ b/third_party/nix/src/nix/build.cc @@ -5,7 +5,7 @@ using namespace nix; -struct CmdBuild : MixDryRun, InstallablesCommand { +struct CmdBuild final : MixDryRun, InstallablesCommand { Path outLink = "result"; CmdBuild() { diff --git a/third_party/nix/src/nix/cat.cc b/third_party/nix/src/nix/cat.cc index 49471100cf..9ead9a6ff6 100644 --- a/third_party/nix/src/nix/cat.cc +++ b/third_party/nix/src/nix/cat.cc @@ -21,7 +21,7 @@ struct MixCat : virtual Args { } }; -struct CmdCatStore : StoreCommand, MixCat { +struct CmdCatStore final : StoreCommand, MixCat { CmdCatStore() { expectArg("path", &path); } std::string name() override { return "cat-store"; } @@ -33,7 +33,7 @@ struct CmdCatStore : StoreCommand, MixCat { void run(ref store) override { cat(store->getFSAccessor()); } }; -struct CmdCatNar : StoreCommand, MixCat { +struct CmdCatNar final : StoreCommand, MixCat { Path narPath; CmdCatNar() { diff --git a/third_party/nix/src/nix/copy.cc b/third_party/nix/src/nix/copy.cc index a527d40cfa..ec25d0fc5a 100644 --- a/third_party/nix/src/nix/copy.cc +++ b/third_party/nix/src/nix/copy.cc @@ -8,7 +8,7 @@ using namespace nix; -struct CmdCopy : StorePathsCommand { +struct CmdCopy final : StorePathsCommand { std::string srcUri, dstUri; CheckSigsFlag checkSigs = CheckSigs; diff --git a/third_party/nix/src/nix/doctor.cc b/third_party/nix/src/nix/doctor.cc index 3a5cd987a9..97c31d0376 100644 --- a/third_party/nix/src/nix/doctor.cc +++ b/third_party/nix/src/nix/doctor.cc @@ -19,7 +19,7 @@ std::string formatProtocol(unsigned int proto) { return "unknown"; } -struct CmdDoctor : StoreCommand { +struct CmdDoctor final : StoreCommand { bool success = true; std::string name() override { return "doctor"; } diff --git a/third_party/nix/src/nix/dump-path.cc b/third_party/nix/src/nix/dump-path.cc index d7e8bc69fd..4ca5f1bc02 100644 --- a/third_party/nix/src/nix/dump-path.cc +++ b/third_party/nix/src/nix/dump-path.cc @@ -3,7 +3,7 @@ using namespace nix; -struct CmdDumpPath : StorePathCommand { +struct CmdDumpPath final : StorePathCommand { std::string name() override { return "dump-path"; } std::string description() override { diff --git a/third_party/nix/src/nix/edit.cc b/third_party/nix/src/nix/edit.cc index 55b193fb6b..958e8aec49 100644 --- a/third_party/nix/src/nix/edit.cc +++ b/third_party/nix/src/nix/edit.cc @@ -9,7 +9,7 @@ using namespace nix; -struct CmdEdit : InstallableCommand { +struct CmdEdit final : InstallableCommand { std::string name() override { return "edit"; } std::string description() override { diff --git a/third_party/nix/src/nix/eval.cc b/third_party/nix/src/nix/eval.cc index 5f26aa14c0..a78f5b34fa 100644 --- a/third_party/nix/src/nix/eval.cc +++ b/third_party/nix/src/nix/eval.cc @@ -9,7 +9,7 @@ using namespace nix; -struct CmdEval : MixJSON, InstallableCommand { +struct CmdEval final : MixJSON, InstallableCommand { bool raw = false; CmdEval() { mkFlag(0, "raw", "print strings unquoted", &raw); } diff --git a/third_party/nix/src/nix/hash.cc b/third_party/nix/src/nix/hash.cc index 935faec9d5..521f97d53a 100644 --- a/third_party/nix/src/nix/hash.cc +++ b/third_party/nix/src/nix/hash.cc @@ -6,7 +6,7 @@ using namespace nix; -struct CmdHash : Command { +struct CmdHash final : Command { enum Mode { mFile, mPath }; Mode mode; Base base = SRI; @@ -47,7 +47,7 @@ struct CmdHash : Command { static RegisterCommand r1(make_ref(CmdHash::mFile)); static RegisterCommand r2(make_ref(CmdHash::mPath)); -struct CmdToBase : Command { +struct CmdToBase final : Command { Base base; HashType ht = htUnknown; std::vector args; diff --git a/third_party/nix/src/nix/installables.cc b/third_party/nix/src/nix/installables.cc index c6cfc4344f..5fe155ba16 100644 --- a/third_party/nix/src/nix/installables.cc +++ b/third_party/nix/src/nix/installables.cc @@ -98,7 +98,7 @@ Buildable Installable::toBuildable() { return std::move(buildables[0]); } -struct InstallableStorePath : Installable { +struct InstallableStorePath final : Installable { Path storePath; explicit InstallableStorePath(Path storePath) @@ -158,7 +158,7 @@ struct InstallableValue : Installable { } }; -struct InstallableExpr : InstallableValue { +struct InstallableExpr final : InstallableValue { std::string text; InstallableExpr(SourceExprCommand& cmd, std::string text) @@ -173,7 +173,7 @@ struct InstallableExpr : InstallableValue { } }; -struct InstallableAttrPath : InstallableValue { +struct InstallableAttrPath final : InstallableValue { std::string attrPath; InstallableAttrPath(SourceExprCommand& cmd, std::string attrPath) diff --git a/third_party/nix/src/nix/log.cc b/third_party/nix/src/nix/log.cc index b205851ece..623d16e5d2 100644 --- a/third_party/nix/src/nix/log.cc +++ b/third_party/nix/src/nix/log.cc @@ -7,7 +7,7 @@ using namespace nix; -struct CmdLog : InstallableCommand { +struct CmdLog final : InstallableCommand { CmdLog() = default; std::string name() override { return "log"; } diff --git a/third_party/nix/src/nix/ls.cc b/third_party/nix/src/nix/ls.cc index aa461675ad..afa4db92bc 100644 --- a/third_party/nix/src/nix/ls.cc +++ b/third_party/nix/src/nix/ls.cc @@ -88,7 +88,7 @@ struct MixLs : virtual Args, MixJSON { } }; -struct CmdLsStore : StoreCommand, MixLs { +struct CmdLsStore final : StoreCommand, MixLs { CmdLsStore() { expectArg("path", &path); } Examples examples() override { @@ -108,7 +108,7 @@ struct CmdLsStore : StoreCommand, MixLs { void run(ref store) override { list(store->getFSAccessor()); } }; -struct CmdLsNar : Command, MixLs { +struct CmdLsNar final : Command, MixLs { Path narPath; CmdLsNar() { diff --git a/third_party/nix/src/nix/optimise-store.cc b/third_party/nix/src/nix/optimise-store.cc index 594cc0e637..1b33ea3ec2 100644 --- a/third_party/nix/src/nix/optimise-store.cc +++ b/third_party/nix/src/nix/optimise-store.cc @@ -6,7 +6,7 @@ using namespace nix; -struct CmdOptimiseStore : StoreCommand { +struct CmdOptimiseStore final : StoreCommand { CmdOptimiseStore() = default; std::string name() override { return "optimise-store"; } diff --git a/third_party/nix/src/nix/path-info.cc b/third_party/nix/src/nix/path-info.cc index aa43ad5262..dca08b06ad 100644 --- a/third_party/nix/src/nix/path-info.cc +++ b/third_party/nix/src/nix/path-info.cc @@ -9,7 +9,7 @@ using namespace nix; -struct CmdPathInfo : StorePathsCommand, MixJSON { +struct CmdPathInfo final : StorePathsCommand, MixJSON { bool showSize = false; bool showClosureSize = false; bool humanReadable = false; diff --git a/third_party/nix/src/nix/ping-store.cc b/third_party/nix/src/nix/ping-store.cc index 0857733c42..78a79b037b 100644 --- a/third_party/nix/src/nix/ping-store.cc +++ b/third_party/nix/src/nix/ping-store.cc @@ -4,7 +4,7 @@ using namespace nix; -struct CmdPingStore : StoreCommand { +struct CmdPingStore final : StoreCommand { std::string name() override { return "ping-store"; } std::string description() override { diff --git a/third_party/nix/src/nix/repl.cc b/third_party/nix/src/nix/repl.cc index b8edfc0e1c..7644b7b8c2 100644 --- a/third_party/nix/src/nix/repl.cc +++ b/third_party/nix/src/nix/repl.cc @@ -799,7 +799,7 @@ std::ostream& NixRepl::printValue(std::ostream& str, Value& v, return str; } -struct CmdRepl : StoreCommand, MixEvalArgs { +struct CmdRepl final : StoreCommand, MixEvalArgs { std::vector files; CmdRepl() { expectArgs("files", &files); } diff --git a/third_party/nix/src/nix/run.cc b/third_party/nix/src/nix/run.cc index b1ca56d751..241e0a2d78 100644 --- a/third_party/nix/src/nix/run.cc +++ b/third_party/nix/src/nix/run.cc @@ -17,7 +17,7 @@ using namespace nix; std::string chrootHelperName = "__run_in_chroot"; -struct CmdRun : InstallablesCommand { +struct CmdRun final : InstallablesCommand { std::vector command = {"bash"}; StringSet keep, unset; bool ignoreEnvironment = false; diff --git a/third_party/nix/src/nix/search.cc b/third_party/nix/src/nix/search.cc index 350863cf81..2bfdeac6f7 100644 --- a/third_party/nix/src/nix/search.cc +++ b/third_party/nix/src/nix/search.cc @@ -27,7 +27,7 @@ std::string hilite(const std::string& s, const std::smatch& m, postfix + std::string(m.suffix()); } -struct CmdSearch : SourceExprCommand, MixJSON { +struct CmdSearch final : SourceExprCommand, MixJSON { std::vector res; bool writeCache = true; diff --git a/third_party/nix/src/nix/show-config.cc b/third_party/nix/src/nix/show-config.cc index 8e27fd168f..56448f9d60 100644 --- a/third_party/nix/src/nix/show-config.cc +++ b/third_party/nix/src/nix/show-config.cc @@ -6,7 +6,7 @@ using namespace nix; -struct CmdShowConfig : Command, MixJSON { +struct CmdShowConfig final : Command, MixJSON { CmdShowConfig() = default; std::string name() override { return "show-config"; } diff --git a/third_party/nix/src/nix/show-derivation.cc b/third_party/nix/src/nix/show-derivation.cc index 2a6252f931..51c7ba9c29 100644 --- a/third_party/nix/src/nix/show-derivation.cc +++ b/third_party/nix/src/nix/show-derivation.cc @@ -9,7 +9,7 @@ using namespace nix; -struct CmdShowDerivation : InstallablesCommand { +struct CmdShowDerivation final : InstallablesCommand { bool recursive = false; CmdShowDerivation() { diff --git a/third_party/nix/src/nix/sigs.cc b/third_party/nix/src/nix/sigs.cc index 8df1beef2b..21d14bffac 100644 --- a/third_party/nix/src/nix/sigs.cc +++ b/third_party/nix/src/nix/sigs.cc @@ -9,7 +9,7 @@ using namespace nix; -struct CmdCopySigs : StorePathsCommand { +struct CmdCopySigs final : StorePathsCommand { Strings substituterUris; CmdCopySigs() { @@ -99,7 +99,7 @@ struct CmdCopySigs : StorePathsCommand { static RegisterCommand r1(make_ref()); -struct CmdSignPaths : StorePathsCommand { +struct CmdSignPaths final : StorePathsCommand { Path secretKeyFile; CmdSignPaths() { diff --git a/third_party/nix/src/nix/upgrade-nix.cc b/third_party/nix/src/nix/upgrade-nix.cc index 1d85af03c1..3e3b55789d 100644 --- a/third_party/nix/src/nix/upgrade-nix.cc +++ b/third_party/nix/src/nix/upgrade-nix.cc @@ -13,7 +13,7 @@ using namespace nix; -struct CmdUpgradeNix : MixDryRun, StoreCommand { +struct CmdUpgradeNix final : MixDryRun, StoreCommand { Path profileDir; std::string storePathsUrl = "https://github.com/NixOS/nixpkgs/raw/master/nixos/modules/installer/" diff --git a/third_party/nix/src/nix/verify.cc b/third_party/nix/src/nix/verify.cc index f5f222915e..a9959d50de 100644 --- a/third_party/nix/src/nix/verify.cc +++ b/third_party/nix/src/nix/verify.cc @@ -10,7 +10,7 @@ using namespace nix; -struct CmdVerify : StorePathsCommand { +struct CmdVerify final : StorePathsCommand { bool noContents = false; bool noTrust = false; Strings substituterUris; diff --git a/third_party/nix/src/nix/why-depends.cc b/third_party/nix/src/nix/why-depends.cc index 4fb91c9d91..e18314e7fd 100644 --- a/third_party/nix/src/nix/why-depends.cc +++ b/third_party/nix/src/nix/why-depends.cc @@ -23,7 +23,7 @@ static std::string filterPrintable(const std::string& s) { return res; } -struct CmdWhyDepends : SourceExprCommand { +struct CmdWhyDepends final : SourceExprCommand { std::string _package, _dependency; bool all = false; diff --git a/third_party/nix/src/tests/language-tests.cc b/third_party/nix/src/tests/language-tests.cc index cac5aee584..98f2927911 100644 --- a/third_party/nix/src/tests/language-tests.cc +++ b/third_party/nix/src/tests/language-tests.cc @@ -174,7 +174,7 @@ TEST_P(EvalFailureTest, Fails) { EvalState state({}, ref(store)); auto path = GetParam(); - Expr* expr; + Expr* expr = nullptr; EXPECT_NO_THROW(expr = state.parseExprFromFile(GetParam().string())) << path.stem().string() << ": should parse successfully"; -- cgit 1.4.1