From 41d70a2fc8d243d8c83ecc1c9ba648b625957437 Mon Sep 17 00:00:00 2001 From: pennae Date: Fri, 21 Jan 2022 14:44:00 +0100 Subject: return string_views from forceString* once a string has been forced we already have dynamic storage allocated for it, so we can easily reuse that storage instead of copying. --- src/libexpr/attr-path.cc | 2 +- src/libexpr/eval.cc | 12 +++--- src/libexpr/eval.hh | 10 ++--- src/libexpr/flake/flake.cc | 6 +-- src/libexpr/get-drvs.cc | 2 +- src/libexpr/json-to-value.cc | 2 +- src/libexpr/json-to-value.hh | 2 +- src/libexpr/parser.y | 10 ++--- src/libexpr/primops.cc | 81 +++++++++++++++++++---------------- src/libexpr/primops/context.cc | 2 +- src/libexpr/primops/fetchMercurial.cc | 8 ++-- src/libexpr/primops/fromTOML.cc | 2 +- 12 files changed, 73 insertions(+), 66 deletions(-) (limited to 'src/libexpr') diff --git a/src/libexpr/attr-path.cc b/src/libexpr/attr-path.cc index c50c6d92b..edef4d9f8 100644 --- a/src/libexpr/attr-path.cc +++ b/src/libexpr/attr-path.cc @@ -121,7 +121,7 @@ Pos findPackageFilename(EvalState & state, Value & v, std::string what) std::string filename(pos, 0, colon); unsigned int lineno; try { - lineno = std::stoi(std::string(pos, colon + 1)); + lineno = std::stoi(std::string(pos, colon + 1, string::npos)); } catch (std::invalid_argument & e) { throw ParseError("cannot parse line number '%s'", pos); } diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index b884b4001..bfc5d3ebf 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -1857,7 +1857,7 @@ void EvalState::forceFunction(Value & v, const Pos & pos) } -string EvalState::forceString(Value & v, const Pos & pos) +std::string_view EvalState::forceString(Value & v, const Pos & pos) { forceValue(v, pos); if (v.type() != nString) { @@ -1866,7 +1866,7 @@ string EvalState::forceString(Value & v, const Pos & pos) else throwTypeError("value is %1% while a string was expected", v); } - return string(v.string.s); + return v.string.s; } @@ -1901,17 +1901,17 @@ std::vector> Value::getContext() } -string EvalState::forceString(Value & v, PathSet & context, const Pos & pos) +std::string_view EvalState::forceString(Value & v, PathSet & context, const Pos & pos) { - string s = forceString(v, pos); + auto s = forceString(v, pos); copyContext(v, context); return s; } -string EvalState::forceStringNoCtx(Value & v, const Pos & pos) +std::string_view EvalState::forceStringNoCtx(Value & v, const Pos & pos) { - string s = forceString(v, pos); + auto s = forceString(v, pos); if (v.string.context) { if (pos) throwEvalError(pos, "the string '%1%' is not allowed to refer to a store path (such as '%2%')", diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 67bdd4de4..5b94a96ca 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -201,8 +201,8 @@ public: void resetFileCache(); /* Look up a file in the search path. */ - Path findFile(const string & path); - Path findFile(SearchPath & searchPath, const string & path, const Pos & pos = noPos); + Path findFile(const std::string_view path); + Path findFile(SearchPath & searchPath, const std::string_view path, const Pos & pos = noPos); /* If the specified search path element is a URI, download it. */ std::pair resolveSearchPathElem(const SearchPathElem & elem); @@ -236,9 +236,9 @@ public: inline void forceList(Value & v); inline void forceList(Value & v, const Pos & pos); void forceFunction(Value & v, const Pos & pos); // either lambda or primop - string forceString(Value & v, const Pos & pos = noPos); - string forceString(Value & v, PathSet & context, const Pos & pos = noPos); - string forceStringNoCtx(Value & v, const Pos & pos = noPos); + std::string_view forceString(Value & v, const Pos & pos = noPos); + std::string_view forceString(Value & v, PathSet & context, const Pos & pos = noPos); + std::string_view forceStringNoCtx(Value & v, const Pos & pos = noPos); /* Return true iff the value `v' denotes a derivation (i.e. a set with attribute `type = "derivation"'). */ diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc index 0fbe9b960..809f54cc0 100644 --- a/src/libexpr/flake/flake.cc +++ b/src/libexpr/flake/flake.cc @@ -250,7 +250,7 @@ static Flake getFlake( for (auto & setting : *nixConfig->value->attrs) { forceTrivialValue(state, *setting.value, *setting.pos); if (setting.value->type() == nString) - flake.config.settings.insert({setting.name, state.forceStringNoCtx(*setting.value, *setting.pos)}); + flake.config.settings.insert({setting.name, string(state.forceStringNoCtx(*setting.value, *setting.pos))}); else if (setting.value->type() == nPath) { PathSet emptyContext = {}; flake.config.settings.insert({setting.name, state.coerceToString(*setting.pos, *setting.value, emptyContext, false, true, true)}); @@ -265,7 +265,7 @@ static Flake getFlake( if (elem->type() != nString) throw TypeError("list element in flake configuration setting '%s' is %s while a string is expected", setting.name, showType(*setting.value)); - ss.push_back(state.forceStringNoCtx(*elem, *setting.pos)); + ss.emplace_back(state.forceStringNoCtx(*elem, *setting.pos)); } flake.config.settings.insert({setting.name, ss}); } @@ -726,7 +726,7 @@ static void prim_getFlake(EvalState & state, const Pos & pos, Value * * args, Va { state.requireExperimentalFeatureOnEvaluation(Xp::Flakes, "builtins.getFlake", pos); - auto flakeRefS = state.forceStringNoCtx(*args[0], pos); + string flakeRefS(state.forceStringNoCtx(*args[0], pos)); auto flakeRef = parseFlakeRef(flakeRefS, {}, true); if (evalSettings.pureEval && !flakeRef.input.isImmutable()) throw Error("cannot call 'getFlake' on mutable flake reference '%s', at %s (use --impure to override)", flakeRefS, pos); diff --git a/src/libexpr/get-drvs.cc b/src/libexpr/get-drvs.cc index 25fd9b949..2651266b2 100644 --- a/src/libexpr/get-drvs.cc +++ b/src/libexpr/get-drvs.cc @@ -104,7 +104,7 @@ DrvInfo::Outputs DrvInfo::queryOutputs(bool onlyOutputsToInstall) /* For each output... */ for (auto elem : i->value->listItems()) { /* Evaluate the corresponding set. */ - string name = state->forceStringNoCtx(*elem, *i->pos); + string name(state->forceStringNoCtx(*elem, *i->pos)); Bindings::iterator out = attrs->find(state->symbols.create(name)); if (out == attrs->end()) continue; // FIXME: throw error? state->forceAttrs(*out->value); diff --git a/src/libexpr/json-to-value.cc b/src/libexpr/json-to-value.cc index 88716250c..99a475ff9 100644 --- a/src/libexpr/json-to-value.cc +++ b/src/libexpr/json-to-value.cc @@ -163,7 +163,7 @@ public: } }; -void parseJSON(EvalState & state, const string & s_, Value & v) +void parseJSON(EvalState & state, const std::string_view & s_, Value & v) { JSONSax parser(state, v); bool res = json::sax_parse(s_, &parser); diff --git a/src/libexpr/json-to-value.hh b/src/libexpr/json-to-value.hh index 3b0fdae11..84bec4eba 100644 --- a/src/libexpr/json-to-value.hh +++ b/src/libexpr/json-to-value.hh @@ -8,6 +8,6 @@ namespace nix { MakeError(JSONParseError, EvalError); -void parseJSON(EvalState & state, const string & s, Value & v); +void parseJSON(EvalState & state, const std::string_view & s, Value & v); } diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index dd76fd66f..b7910da8c 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -709,24 +709,24 @@ void EvalState::addToSearchPath(const string & s) } -Path EvalState::findFile(const string & path) +Path EvalState::findFile(const std::string_view path) { return findFile(searchPath, path); } -Path EvalState::findFile(SearchPath & searchPath, const string & path, const Pos & pos) +Path EvalState::findFile(SearchPath & searchPath, const std::string_view path, const Pos & pos) { for (auto & i : searchPath) { std::string suffix; if (i.first.empty()) - suffix = "/" + path; + suffix = concatStrings("/", path); else { auto s = i.first.size(); if (path.compare(0, s, i.first) != 0 || (path.size() > s && path[s] != '/')) continue; - suffix = path.size() == s ? "" : "/" + string(path, s); + suffix = path.size() == s ? "" : concatStrings("/", path.substr(s)); } auto r = resolveSearchPathElem(i); if (!r.first) continue; @@ -735,7 +735,7 @@ Path EvalState::findFile(SearchPath & searchPath, const string & path, const Pos } if (hasPrefix(path, "nix/")) - return corepkgsPrefix + path.substr(4); + return concatStrings(corepkgsPrefix, path.substr(4)); throw ThrownError({ .msg = hintfmt(evalSettings.pureEval diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index acee71d19..0454acc3d 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -314,7 +314,7 @@ void prim_importNative(EvalState & state, const Pos & pos, Value * * args, Value { auto path = realisePath(state, pos, *args[0]); - string sym = state.forceStringNoCtx(*args[1], pos); + string sym(state.forceStringNoCtx(*args[1], pos)); void *handle = dlopen(path.c_str(), RTLD_LAZY | RTLD_LOCAL); if (!handle) @@ -825,7 +825,7 @@ static RegisterPrimOp primop_tryEval({ /* Return an environment variable. Use with care. */ static void prim_getEnv(EvalState & state, const Pos & pos, Value * * args, Value & v) { - string name = state.forceStringNoCtx(*args[0], pos); + string name(state.forceStringNoCtx(*args[0], pos)); v.mkString(evalSettings.restrictEval || evalSettings.pureEval ? "" : getEnv(name).value_or("")); } @@ -975,7 +975,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * const string & key = i->name; vomit("processing attribute '%1%'", key); - auto handleHashMode = [&](const std::string & s) { + auto handleHashMode = [&](const std::string_view s) { if (s == "recursive") ingestionMethod = FileIngestionMethod::Recursive; else if (s == "flat") ingestionMethod = FileIngestionMethod::Flat; else @@ -1502,7 +1502,7 @@ static void prim_findFile(EvalState & state, const Pos & pos, Value * * args, Va searchPath.emplace_back(prefix, path); } - string path = state.forceStringNoCtx(*args[1], pos); + auto path = state.forceStringNoCtx(*args[1], pos); v.mkPath(state.checkSourcePath(state.findFile(searchPath, path, pos))); } @@ -1516,7 +1516,7 @@ static RegisterPrimOp primop_findFile(RegisterPrimOp::Info { /* Return the cryptographic hash of a file in base-16. */ static void prim_hashFile(EvalState & state, const Pos & pos, Value * * args, Value & v) { - string type = state.forceStringNoCtx(*args[0], pos); + auto type = state.forceStringNoCtx(*args[0], pos); std::optional ht = parseHashType(type); if (!ht) throw Error({ @@ -1723,7 +1723,7 @@ static RegisterPrimOp primop_toJSON({ /* Parse a JSON string to a value. */ static void prim_fromJSON(EvalState & state, const Pos & pos, Value * * args, Value & v) { - string s = state.forceStringNoCtx(*args[0], pos); + auto s = state.forceStringNoCtx(*args[0], pos); try { parseJSON(state, s, v); } catch (JSONParseError &e) { @@ -1752,8 +1752,8 @@ static RegisterPrimOp primop_fromJSON({ static void prim_toFile(EvalState & state, const Pos & pos, Value * * args, Value & v) { PathSet context; - string name = state.forceStringNoCtx(*args[0], pos); - string contents = state.forceString(*args[1], context, pos); + string name(state.forceStringNoCtx(*args[0], pos)); + string contents(state.forceString(*args[1], context, pos)); StorePathSet refs; @@ -2153,7 +2153,7 @@ static RegisterPrimOp primop_attrValues({ /* Dynamic version of the `.' operator. */ void prim_getAttr(EvalState & state, const Pos & pos, Value * * args, Value & v) { - string attr = state.forceStringNoCtx(*args[0], pos); + auto attr = state.forceStringNoCtx(*args[0], pos); state.forceAttrs(*args[1], pos); Bindings::iterator i = getAttr( state, @@ -2183,7 +2183,7 @@ static RegisterPrimOp primop_getAttr({ /* Return position information of the specified attribute. */ static void prim_unsafeGetAttrPos(EvalState & state, const Pos & pos, Value * * args, Value & v) { - string attr = state.forceStringNoCtx(*args[0], pos); + auto attr = state.forceStringNoCtx(*args[0], pos); state.forceAttrs(*args[1], pos); Bindings::iterator i = args[1]->attrs->find(state.symbols.create(attr)); if (i == args[1]->attrs->end()) @@ -2201,7 +2201,7 @@ static RegisterPrimOp primop_unsafeGetAttrPos(RegisterPrimOp::Info { /* Dynamic version of the `?' operator. */ static void prim_hasAttr(EvalState & state, const Pos & pos, Value * * args, Value & v) { - string attr = state.forceStringNoCtx(*args[0], pos); + auto attr = state.forceStringNoCtx(*args[0], pos); state.forceAttrs(*args[1], pos); v.mkBool(args[1]->attrs->find(state.symbols.create(attr)) != args[1]->attrs->end()); } @@ -2300,7 +2300,7 @@ static void prim_listToAttrs(EvalState & state, const Pos & pos, Value * * args, pos ); - string name = state.forceStringNoCtx(*j->value, *j->pos); + auto name = state.forceStringNoCtx(*j->value, *j->pos); Symbol sym = state.symbols.create(name); if (seen.insert(sym).second) { @@ -3032,7 +3032,7 @@ static void prim_groupBy(EvalState & state, const Pos & pos, Value * * args, Val for (auto vElem : args[1]->listItems()) { Value res; state.callFunction(*args[0], *vElem, res, pos); - string name = state.forceStringNoCtx(res, pos); + auto name = state.forceStringNoCtx(res, pos); Symbol sym = state.symbols.create(name); auto vector = attrs.try_emplace(sym, ValueVector()).first; vector->second.push_back(vElem); @@ -3376,7 +3376,7 @@ static RegisterPrimOp primop_stringLength({ /* Return the cryptographic hash of a string in base-16. */ static void prim_hashString(EvalState & state, const Pos & pos, Value * * args, Value & v) { - string type = state.forceStringNoCtx(*args[0], pos); + auto type = state.forceStringNoCtx(*args[0], pos); std::optional ht = parseHashType(type); if (!ht) throw Error({ @@ -3385,7 +3385,7 @@ static void prim_hashString(EvalState & state, const Pos & pos, Value * * args, }); PathSet context; // discarded - string s = state.forceString(*args[1], context, pos); + auto s = state.forceString(*args[1], context, pos); v.mkString(hashString(*ht, s).to_string(Base16, false)); } @@ -3403,7 +3403,18 @@ static RegisterPrimOp primop_hashString({ struct RegexCache { - std::unordered_map cache; + // TODO use C++20 transparent comparison when available + std::unordered_map cache; + std::list keys; + + std::regex get(std::string_view re) + { + auto it = cache.find(re); + if (it != cache.end()) + return it->second; + keys.emplace_back(re); + return cache.emplace(keys.back(), std::regex(keys.back(), std::regex::extended)).first->second; + } }; std::shared_ptr makeRegexCache() @@ -3417,15 +3428,13 @@ void prim_match(EvalState & state, const Pos & pos, Value * * args, Value & v) try { - auto regex = state.regexCache->cache.find(re); - if (regex == state.regexCache->cache.end()) - regex = state.regexCache->cache.emplace(re, std::regex(re, std::regex::extended)).first; + auto regex = state.regexCache->get(re); PathSet context; - const std::string str = state.forceString(*args[1], context, pos); + const auto str = state.forceString(*args[1], context, pos); - std::smatch match; - if (!std::regex_match(str, match, regex->second)) { + std::cmatch match; + if (!std::regex_match(str.begin(), str.end(), match, regex)) { v.mkNull(); return; } @@ -3500,15 +3509,13 @@ void prim_split(EvalState & state, const Pos & pos, Value * * args, Value & v) try { - auto regex = state.regexCache->cache.find(re); - if (regex == state.regexCache->cache.end()) - regex = state.regexCache->cache.emplace(re, std::regex(re, std::regex::extended)).first; + auto regex = state.regexCache->get(re); PathSet context; - const std::string str = state.forceString(*args[1], context, pos); + const auto str = state.forceString(*args[1], context, pos); - auto begin = std::sregex_iterator(str.begin(), str.end(), regex->second); - auto end = std::sregex_iterator(); + auto begin = std::cregex_iterator(str.begin(), str.end(), regex); + auto end = std::cregex_iterator(); // Any matches results are surrounded by non-matching results. const size_t len = std::distance(begin, end); @@ -3520,9 +3527,9 @@ void prim_split(EvalState & state, const Pos & pos, Value * * args, Value & v) return; } - for (std::sregex_iterator i = begin; i != end; ++i) { + for (auto i = begin; i != end; ++i) { assert(idx <= 2 * len + 1 - 3); - std::smatch match = *i; + auto match = *i; // Add a string for non-matched characters. (v.listElems()[idx++] = state.allocValue())->mkString(match.prefix().str()); @@ -3643,14 +3650,14 @@ static void prim_replaceStrings(EvalState & state, const Pos & pos, Value * * ar vector from; from.reserve(args[0]->listSize()); for (auto elem : args[0]->listItems()) - from.push_back(state.forceString(*elem, pos)); + from.emplace_back(state.forceString(*elem, pos)); vector> to; to.reserve(args[1]->listSize()); for (auto elem : args[1]->listItems()) { PathSet ctx; auto s = state.forceString(*elem, ctx, pos); - to.push_back(std::make_pair(std::move(s), std::move(ctx))); + to.emplace_back(s, std::move(ctx)); } PathSet context; @@ -3712,7 +3719,7 @@ static RegisterPrimOp primop_replaceStrings({ static void prim_parseDrvName(EvalState & state, const Pos & pos, Value * * args, Value & v) { - string name = state.forceStringNoCtx(*args[0], pos); + auto name = state.forceStringNoCtx(*args[0], pos); DrvName parsed(name); auto attrs = state.buildBindings(2); attrs.alloc(state.sName).mkString(parsed.name); @@ -3736,8 +3743,8 @@ static RegisterPrimOp primop_parseDrvName({ static void prim_compareVersions(EvalState & state, const Pos & pos, Value * * args, Value & v) { - string version1 = state.forceStringNoCtx(*args[0], pos); - string version2 = state.forceStringNoCtx(*args[1], pos); + auto version1 = state.forceStringNoCtx(*args[0], pos); + auto version2 = state.forceStringNoCtx(*args[1], pos); v.mkInt(compareVersions(version1, version2)); } @@ -3756,14 +3763,14 @@ static RegisterPrimOp primop_compareVersions({ static void prim_splitVersion(EvalState & state, const Pos & pos, Value * * args, Value & v) { - string version = state.forceStringNoCtx(*args[0], pos); + auto version = state.forceStringNoCtx(*args[0], pos); auto iter = version.cbegin(); Strings components; while (iter != version.cend()) { auto component = nextComponent(iter, version.cend()); if (component.empty()) break; - components.emplace_back(std::move(component)); + components.emplace_back(component); } state.mkList(v, components.size()); for (const auto & [n, component] : enumerate(components)) diff --git a/src/libexpr/primops/context.cc b/src/libexpr/primops/context.cc index a239c06da..cd7eeb588 100644 --- a/src/libexpr/primops/context.cc +++ b/src/libexpr/primops/context.cc @@ -180,7 +180,7 @@ static void prim_appendContext(EvalState & state, const Pos & pos, Value * * arg } for (auto elem : iter->value->listItems()) { auto name = state.forceStringNoCtx(*elem, *iter->pos); - context.insert("!" + name + "!" + string(i.name)); + context.insert(concatStrings("!", name, "!", i.name)); } } } diff --git a/src/libexpr/primops/fetchMercurial.cc b/src/libexpr/primops/fetchMercurial.cc index 42214c207..f808e2da5 100644 --- a/src/libexpr/primops/fetchMercurial.cc +++ b/src/libexpr/primops/fetchMercurial.cc @@ -12,7 +12,7 @@ static void prim_fetchMercurial(EvalState & state, const Pos & pos, Value * * ar std::string url; std::optional rev; std::optional ref; - std::string name = "source"; + std::string_view name = "source"; PathSet context; state.forceValue(*args[0], pos); @@ -22,14 +22,14 @@ static void prim_fetchMercurial(EvalState & state, const Pos & pos, Value * * ar state.forceAttrs(*args[0], pos); for (auto & attr : *args[0]->attrs) { - string n(attr.name); + std::string_view n(attr.name); if (n == "url") url = state.coerceToString(*attr.pos, *attr.value, context, false, false); else if (n == "rev") { // Ugly: unlike fetchGit, here the "rev" attribute can // be both a revision or a branch/tag name. auto value = state.forceStringNoCtx(*attr.value, *attr.pos); - if (std::regex_match(value, revRegex)) + if (std::regex_match(value.begin(), value.end(), revRegex)) rev = Hash::parseAny(value, htSHA1); else ref = value; @@ -62,7 +62,7 @@ static void prim_fetchMercurial(EvalState & state, const Pos & pos, Value * * ar fetchers::Attrs attrs; attrs.insert_or_assign("type", "hg"); attrs.insert_or_assign("url", url.find("://") != std::string::npos ? url : "file://" + url); - attrs.insert_or_assign("name", name); + attrs.insert_or_assign("name", string(name)); if (ref) attrs.insert_or_assign("ref", *ref); if (rev) attrs.insert_or_assign("rev", rev->gitRev()); auto input = fetchers::Input::fromAttrs(std::move(attrs)); diff --git a/src/libexpr/primops/fromTOML.cc b/src/libexpr/primops/fromTOML.cc index 80c7e0b82..c0e858b61 100644 --- a/src/libexpr/primops/fromTOML.cc +++ b/src/libexpr/primops/fromTOML.cc @@ -9,7 +9,7 @@ static void prim_fromTOML(EvalState & state, const Pos & pos, Value * * args, Va { auto toml = state.forceStringNoCtx(*args[0], pos); - std::istringstream tomlStream(toml); + std::istringstream tomlStream(string{toml}); std::function visit; -- cgit v1.2.3