From 6163dea01c060a4c2a509e0c241b95ec863f7e8a Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Thu, 5 Dec 2024 17:03:31 -0800 Subject: [PATCH] tree-wide: fix a pile of lints Mostly these are bugprone-unused-local-non-trivial-variable. Also fix instances of: - bugprone-optional-value-conversion - bugprone-inc-dec-in-conditions (please check this loop is correct, it is the only non trivial code change in here) - bugprone-unused-return-value (well, by fixing the lint config) There are three notable changes relating to undefined vars: - openLogFile ignoring the result. This is because openLogFile does a whole bunch of mutation of member variables - hiliteMatches: i am guessing this is because showing the derivation name was unhelpful and it just got changed - canonPath in NarAccessor: canonPath inside of a thing that is supposed to be vfs based cannot possibly be correct, so let's delete it given it is unused. Fixes: https://git.lix.systems/lix-project/lix/issues/584 Change-Id: I887adc9ff28b61f726dcfed197e6796b414c2fcf --- .clang-tidy | 3 +++ src/libexpr/attr-path.cc | 1 - src/libfetchers/fetchers.cc | 12 +++--------- src/libfetchers/git.cc | 2 +- src/libstore/build/derivation-goal.cc | 2 +- src/libstore/build/local-derivation-goal.cc | 2 +- src/libstore/common-protocol.cc | 2 +- src/libstore/export-import.cc | 5 ----- src/libstore/nar-accessor.cc | 1 - src/libstore/nar-info.cc | 2 +- src/libstore/store-api.cc | 3 +-- src/libutil/hilite.cc | 2 +- src/libutil/serialise.cc | 3 +-- src/libutil/terminal.cc | 2 +- src/nix-env/nix-env.cc | 2 +- src/nix/bundle.cc | 2 -- src/nix/path-info.cc | 2 +- src/nix/search.cc | 1 - src/nix/sigs.cc | 6 ------ 19 files changed, 17 insertions(+), 38 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 87f6d0404..77ee39cb8 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -16,6 +16,8 @@ Checks: - -bugprone-unchecked-optional-access # many warnings, seems like a questionable lint - -bugprone-branch-clone + # extremely noisy before clang 19: https://github.com/llvm/llvm-project/issues/93959 + - -bugprone-multi-level-implicit-pointer-conversion # all thrown exceptions must derive from std::exception - hicpp-exception-baseclass # capturing async lambdas are dangerous @@ -33,3 +35,4 @@ Checks: CheckOptions: bugprone-reserved-identifier.AllowedIdentifiers: '__asan_default_options' + bugprone-unused-return-value.AllowCastToVoid: true diff --git a/src/libexpr/attr-path.cc b/src/libexpr/attr-path.cc index 3e60f295d..d20fbf8cf 100644 --- a/src/libexpr/attr-path.cc +++ b/src/libexpr/attr-path.cc @@ -129,7 +129,6 @@ std::pair findPackageFilename(EvalState & state, Value & v try { auto colon = fn.rfind(':'); if (colon == std::string::npos) fail(); - std::string filename(fn, 0, colon); auto lineno = std::stoi(std::string(fn, colon + 1, std::string::npos)); return {CanonPath(fn.substr(0, colon)), lineno}; } catch (std::invalid_argument & e) { diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc index c441ffb12..6f5026e20 100644 --- a/src/libfetchers/fetchers.cc +++ b/src/libfetchers/fetchers.cc @@ -250,9 +250,7 @@ std::optional Input::getNarHash() const std::optional Input::getRef() const { - if (auto s = maybeGetStrAttr(attrs, "ref")) - return *s; - return {}; + return maybeGetStrAttr(attrs, "ref"); } std::optional Input::getRev() const @@ -273,16 +271,12 @@ std::optional Input::getRev() const std::optional Input::getRevCount() const { - if (auto n = maybeGetIntAttr(attrs, "revCount")) - return *n; - return {}; + return maybeGetIntAttr(attrs, "revCount"); } std::optional Input::getLastModified() const { - if (auto n = maybeGetIntAttr(attrs, "lastModified")) - return *n; - return {}; + return maybeGetIntAttr(attrs, "lastModified"); } ParsedURL InputScheme::toURL(const Input & input) const diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 74275d8e7..c972145b6 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -130,7 +130,7 @@ std::optional readHeadCached(const std::string & actualUrl) // This function must behave the same way, so we return the expired // cached ref here. warn("could not get HEAD ref for repository '%s'; using expired cached ref '%s'", actualUrl, *cachedRef); - return *cachedRef; + return cachedRef; } return std::nullopt; diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 50f39ddbc..69d54bd5e 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -1212,7 +1212,7 @@ HookReply DerivationGoal::tryBuildHook() hook->toHook.writeSide.reset(); /* Create the log file and pipe. */ - Path logFile = openLogFile(); + openLogFile(); std::set fds; fds.insert(hook->fromHook.readSide.get()); diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 8dae6e7aa..6488693de 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -778,7 +778,7 @@ void LocalDerivationGoal::startBuilder() printMsg(lvlVomit, "setting builder env variable '%1%'='%2%'", i.first, i.second); /* Create the log file. */ - Path logFile = openLogFile(); + openLogFile(); /* Create a pseudoterminal to get the output of the builder. */ builderOut = AutoCloseFD{posix_openpt(O_RDWR | O_NOCTTY)}; diff --git a/src/libstore/common-protocol.cc b/src/libstore/common-protocol.cc index 4e2b2df31..0387034fc 100644 --- a/src/libstore/common-protocol.cc +++ b/src/libstore/common-protocol.cc @@ -93,7 +93,7 @@ WireFormatGenerator CommonProto::Serialise>::write { return [](std::string s) -> WireFormatGenerator { co_yield s; - }(caOpt ? renderContentAddress(*caOpt) : ""); + }(caOpt ? renderContentAddress(caOpt) : ""); } } diff --git a/src/libstore/export-import.cc b/src/libstore/export-import.cc index 2ccb7f213..78b774725 100644 --- a/src/libstore/export-import.cc +++ b/src/libstore/export-import.cc @@ -13,14 +13,9 @@ void Store::exportPaths(const StorePathSet & paths, Sink & sink) auto sorted = topoSortPaths(paths); std::reverse(sorted.begin(), sorted.end()); - std::string doneLabel("paths exported"); - //logger->incExpected(doneLabel, sorted.size()); - for (auto & path : sorted) { - //Activity act(*logger, lvlInfo, "exporting path '%s'", path); sink << 1; exportPath(path, sink); - //logger->incProgress(doneLabel); } sink << 0; diff --git a/src/libstore/nar-accessor.cc b/src/libstore/nar-accessor.cc index f0dfcb19b..cf28add87 100644 --- a/src/libstore/nar-accessor.cc +++ b/src/libstore/nar-accessor.cc @@ -152,7 +152,6 @@ struct NarAccessor : public FSAccessor NarMember * find(const Path & path) { - Path canon = path == "" ? "" : canonPath(path); NarMember * current = &root; auto end = path.end(); for (auto it = path.begin(); it != end; ) { diff --git a/src/libstore/nar-info.cc b/src/libstore/nar-info.cc index e557b4677..512dfce11 100644 --- a/src/libstore/nar-info.cc +++ b/src/libstore/nar-info.cc @@ -120,7 +120,7 @@ std::string NarInfo::to_string(const Store & store) const res += "Sig: " + sig + "\n"; if (ca) - res += "CA: " + renderContentAddress(*ca) + "\n"; + res += "CA: " + renderContentAddress(ca) + "\n"; return res; } diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 6d9fec41b..9be89f9af 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -1322,8 +1322,7 @@ std::optional Store::getBuildDerivationPath(const StorePath & path) if (!path.isDerivation()) { try { auto info = queryPathInfo(path); - if (!info->deriver) return std::nullopt; - return *info->deriver; + return info->deriver; } catch (InvalidPath &) { return std::nullopt; } diff --git a/src/libutil/hilite.cc b/src/libutil/hilite.cc index e5088230d..7050246f0 100644 --- a/src/libutil/hilite.cc +++ b/src/libutil/hilite.cc @@ -25,7 +25,7 @@ std::string hiliteMatches( out.append(s.substr(last_end, m.position() - last_end)); // Merge continous matches ssize_t end = start + m.length(); - while (++it != matches.end() && (*it).position() <= end) { + for (++it; it != matches.end() && (*it).position() <= end; ++it) { auto n = *it; ssize_t nend = start + (n.position() - start + n.length()); if (nend > end) diff --git a/src/libutil/serialise.cc b/src/libutil/serialise.cc index f1db05b0b..79bdb6d5c 100644 --- a/src/libutil/serialise.cc +++ b/src/libutil/serialise.cc @@ -75,7 +75,6 @@ void Source::operator () (char * data, size_t len) void Source::drainInto(Sink & sink) { - std::string s; std::array buf; while (true) { size_t n; @@ -251,7 +250,7 @@ Error readError(Source & source) auto type = readString(source); assert(type == "Error"); auto level = (Verbosity) readInt(source); - auto name = readString(source); // removed + readString(source); // removed (name) auto msg = readString(source); ErrorInfo info { .level = level, diff --git a/src/libutil/terminal.cc b/src/libutil/terminal.cc index 25e97e599..e2f063218 100644 --- a/src/libutil/terminal.cc +++ b/src/libutil/terminal.cc @@ -34,7 +34,7 @@ bool shouldANSI(StandardOutputStream fileno) // FIXME(jade): replace with TerminalCodeEater. wowie this is evil code. std::string filterANSIEscapes(std::string_view s, bool filterAll, unsigned int width, bool eatTabs) { - std::string t, e; + std::string t; size_t w = 0; auto i = s.begin(); diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc index 13fadb1d8..914627ffa 100644 --- a/src/nix-env/nix-env.cc +++ b/src/nix-env/nix-env.cc @@ -1313,7 +1313,7 @@ static void opSwitchGeneration(Globals & globals, Strings opFlags, Strings opArg throw UsageError("exactly one argument expected"); if (auto dstGen = string2Int(opArgs.front())) - switchGeneration(globals.profile, *dstGen, globals.dryRun); + switchGeneration(globals.profile, dstGen, globals.dryRun); else throw UsageError("expected a generation number"); } diff --git a/src/nix/bundle.cc b/src/nix/bundle.cc index 8bf158d61..67519ddd7 100644 --- a/src/nix/bundle.cc +++ b/src/nix/bundle.cc @@ -116,8 +116,6 @@ struct CmdBundle : InstallableCommand }, }); - auto outPathS = store->printStorePath(outPath); - if (!outLink) { auto * attr = vRes->attrs->get(evalState->sName); if (!attr) diff --git a/src/nix/path-info.cc b/src/nix/path-info.cc index b14eef467..75dda528c 100644 --- a/src/nix/path-info.cc +++ b/src/nix/path-info.cc @@ -114,7 +114,7 @@ struct CmdPathInfo : StorePathsCommand, MixJSON std::cout << '\t'; Strings ss; if (info->ultimate) ss.push_back("ultimate"); - if (info->ca) ss.push_back("ca:" + renderContentAddress(*info->ca)); + if (info->ca) ss.push_back("ca:" + renderContentAddress(info->ca)); for (auto & sig : info->sigs) ss.push_back(sig); std::cout << concatStringsSep(" ", ss); } diff --git a/src/nix/search.cc b/src/nix/search.cc index 9f6b7a24e..6a392ddda 100644 --- a/src/nix/search.cc +++ b/src/nix/search.cc @@ -160,7 +160,6 @@ struct CmdSearch : InstallableCommand, MixJSON {"description", description}, }; } else { - auto name2 = hiliteMatches(name.name, nameMatches, ANSI_GREEN, "\e[0;2m"); if (results > 1) logger->cout(""); logger->cout( "* %s%s", diff --git a/src/nix/sigs.cc b/src/nix/sigs.cc index 948844e22..0d15aedd2 100644 --- a/src/nix/sigs.cc +++ b/src/nix/sigs.cc @@ -40,13 +40,9 @@ struct CmdCopySigs : StorePathsCommand ThreadPool pool; - std::string doneLabel = "done"; std::atomic added{0}; - //logger->setExpected(doneLabel, storePaths.size()); - auto doPath = [&](const Path & storePathS) { - //Activity act(*logger, lvlInfo, "getting signatures for '%s'", storePath); checkInterrupt(); @@ -78,8 +74,6 @@ struct CmdCopySigs : StorePathsCommand store->addSignatures(storePath, newSigs); added += newSigs.size(); } - - //logger->incProgress(doneLabel); }; for (auto & storePath : storePaths)