puck/lix
0
0
Fork 0
forked from lix-project/lix
Code Pull requests Activity

Merge pull request #9047 from flox/tomberek.string_refactor

Browse source 
string Value refactor
This commit is contained in:
Robert Hensing 2023-09-28 02:58:57 +01:00 committed by GitHub
parent 13ed5d7106 399ef84420
commit 13a9090ffc
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 59 additions and 49 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/libcmd/repl.cc
View file

@ -922,7 +922,7 @@ std::ostream & NixRepl::printValue(std::ostream & str, Value & v, unsigned int m
case nString:
str << ANSI_WARNING;
printLiteralString(str, v.string.s);
printLiteralString(str, v.string_view());
str << ANSI_NORMAL;
break;

8
src/libexpr/eval-cache.cc
View file

@ -440,8 +440,8 @@ Value & AttrCursor::forceValue()
if (root->db && (!cachedValue || std::get_if<placeholder_t>(&cachedValue->second))) {
if (v.type() == nString)
cachedValue = {root->db->setString(getKey(), v.string.s, v.string.context),
string_t{v.string.s, {}}};
cachedValue = {root->db->setString(getKey(), v.c_str(), v.context()),
string_t{v.c_str(), {}}};
else if (v.type() == nPath) {
auto path = v.path().path;
cachedValue = {root->db->setString(getKey(), path.abs()), string_t{path.abs(), {}}};
@ -582,7 +582,7 @@ std::string AttrCursor::getString()
if (v.type() != nString && v.type() != nPath)
root->state.error("'%s' is not a string but %s", getAttrPathStr()).debugThrow<TypeError>();
return v.type() == nString ? v.string.s : v.path().to_string();
return v.type() == nString ? v.c_str() : v.path().to_string();
}
string_t AttrCursor::getStringWithContext()
@ -624,7 +624,7 @@ string_t AttrCursor::getStringWithContext()
if (v.type() == nString) {
NixStringContext context;
copyContext(v, context);
return {v.string.s, std::move(context)};
return {v.c_str(), std::move(context)};
}
else if (v.type() == nPath)
return {v.path().to_string(), {}};

18
src/libexpr/eval.cc
View file

@ -114,7 +114,7 @@ void Value::print(const SymbolTable &symbols, std::ostream &str,
printLiteralBool(str, boolean);
break;
case tString:
printLiteralString(str, string.s);
printLiteralString(str, string_view());
break;
case tPath:
str << path().to_string(); // !!! escaping?
@ -339,7 +339,7 @@ static Symbol getName(const AttrName & name, EvalState & state, Env & env)
Value nameValue;
name.expr->eval(state, env, nameValue);
state.forceStringNoCtx(nameValue, noPos, "while evaluating an attribute name");
return state.symbols.create(nameValue.string.s);
return state.symbols.create(nameValue.string_view());
}
}
@ -1343,7 +1343,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
if (nameVal.type() == nNull)
continue;
state.forceStringNoCtx(nameVal, i.pos, "while evaluating the name of a dynamic attribute");
auto nameSym = state.symbols.create(nameVal.string.s);
auto nameSym = state.symbols.create(nameVal.string_view());
Bindings::iterator j = v.attrs->find(nameSym);
if (j != v.attrs->end())
state.error("dynamic attribute '%1%' already defined at %2%", state.symbols[nameSym], state.positions[j->pos]).atPos(i.pos).withFrame(env, *this).debugThrow<EvalError>();
@ -2155,7 +2155,7 @@ std::string_view EvalState::forceString(Value & v, const PosIdx pos, std::string
forceValue(v, pos);
if (v.type() != nString)
error("value is %1% while a string was expected", showType(v)).debugThrow<TypeError>();
return v.string.s;
return v.string_view();
} catch (Error & e) {
e.addTrace(positions[pos], errorCtx);
throw;
@ -2182,8 +2182,8 @@ std::string_view EvalState::forceString(Value & v, NixStringContext & context, c
std::string_view EvalState::forceStringNoCtx(Value & v, const PosIdx pos, std::string_view errorCtx)
{
auto s = forceString(v, pos, errorCtx);
if (v.string.context) {
error("the string '%1%' is not allowed to refer to a store path (such as '%2%')", v.string.s, v.string.context[0]).withTrace(pos, errorCtx).debugThrow<EvalError>();
if (v.context()) {
error("the string '%1%' is not allowed to refer to a store path (such as '%2%')", v.string_view(), v.context()[0]).withTrace(pos, errorCtx).debugThrow<EvalError>();
}
return s;
}
@ -2196,7 +2196,7 @@ bool EvalState::isDerivation(Value & v)
if (i == v.attrs->end()) return false;
forceValue(*i->value, i->pos);
if (i->value->type() != nString) return false;
return strcmp(i->value->string.s, "derivation") == 0;
return i->value->string_view().compare("derivation") == 0;
}
@ -2228,7 +2228,7 @@ BackedStringView EvalState::coerceToString(
if (v.type() == nString) {
copyContext(v, context);
return std::string_view(v.string.s);
return v.string_view();
}
if (v.type() == nPath) {
@ -2426,7 +2426,7 @@ bool EvalState::eqValues(Value & v1, Value & v2, const PosIdx pos, std::string_v
return v1.boolean == v2.boolean;
case nString:
return strcmp(v1.string.s, v2.string.s) == 0;
return v1.string_view().compare(v2.string_view()) == 0;
case nPath:
return strcmp(v1._path, v2._path) == 0;

10
src/libexpr/flake/flake.cc
View file

@ -113,7 +113,7 @@ static FlakeInput parseFlakeInput(EvalState & state,
try {
if (attr.name == sUrl) {
expectType(state, nString, *attr.value, attr.pos);
url = attr.value->string.s;
url = attr.value->string_view();
attrs.emplace("url", *url);
} else if (attr.name == sFlake) {
expectType(state, nBool, *attr.value, attr.pos);
@ -122,7 +122,7 @@ static FlakeInput parseFlakeInput(EvalState & state,
input.overrides = parseFlakeInputs(state, attr.value, attr.pos, baseDir, lockRootPath);
} else if (attr.name == sFollows) {
expectType(state, nString, *attr.value, attr.pos);
auto follows(parseInputPath(attr.value->string.s));
auto follows(parseInputPath(attr.value->c_str()));
follows.insert(follows.begin(), lockRootPath.begin(), lockRootPath.end());
input.follows = follows;
} else {
@ -131,7 +131,7 @@ static FlakeInput parseFlakeInput(EvalState & state,
#pragma GCC diagnostic ignored "-Wswitch-enum"
switch (attr.value->type()) {
case nString:
attrs.emplace(state.symbols[attr.name], attr.value->string.s);
attrs.emplace(state.symbols[attr.name], attr.value->c_str());
break;
case nBool:
attrs.emplace(state.symbols[attr.name], Explicit<bool> { attr.value->boolean });
@ -229,7 +229,7 @@ static Flake getFlake(
if (auto description = vInfo.attrs->get(state.sDescription)) {
expectType(state, nString, *description->value, description->pos);
flake.description = description->value->string.s;
flake.description = description->value->c_str();
}
auto sInputs = state.symbols.create("inputs");
@ -850,7 +850,7 @@ static void prim_flakeRefToString(
Explicit<bool> { attr.value->boolean });
} else if (t == nString) {
attrs.emplace(state.symbols[attr.name],
std::string(attr.value->str()));
std::string(attr.value->string_view()));
} else {
state.error(
"flake reference attribute sets may only contain integers, Booleans, "

12
src/libexpr/get-drvs.cc
View file

@ -156,7 +156,7 @@ DrvInfo::Outputs DrvInfo::queryOutputs(bool withPaths, bool onlyOutputsToInstall
Outputs result;
for (auto elem : outTI->listItems()) {
if (elem->type() != nString) throw errMsg;
auto out = outputs.find(elem->string.s);
auto out = outputs.find(elem->c_str());
if (out == outputs.end()) throw errMsg;
result.insert(*out);
}
@ -230,7 +230,7 @@ std::string DrvInfo::queryMetaString(const std::string & name)
{
Value * v = queryMeta(name);
if (!v || v->type() != nString) return "";
return v->string.s;
return v->c_str();
}
@ -242,7 +242,7 @@ NixInt DrvInfo::queryMetaInt(const std::string & name, NixInt def)
if (v->type() == nString) {
/* Backwards compatibility with before we had support for
integer meta fields. */
if (auto n = string2Int<NixInt>(v->string.s))
if (auto n = string2Int<NixInt>(v->c_str()))
return *n;
}
return def;
@ -256,7 +256,7 @@ NixFloat DrvInfo::queryMetaFloat(const std::string & name, NixFloat def)
if (v->type() == nString) {
/* Backwards compatibility with before we had support for
float meta fields. */
if (auto n = string2Float<NixFloat>(v->string.s))
if (auto n = string2Float<NixFloat>(v->c_str()))
return *n;
}
return def;
@ -271,8 +271,8 @@ bool DrvInfo::queryMetaBool(const std::string & name, bool def)
if (v->type() == nString) {
/* Backwards compatibility with before we had support for
Boolean meta fields. */
if (strcmp(v->string.s, "true") == 0) return true;
if (strcmp(v->string.s, "false") == 0) return false;
if (v->string_view() == "true") return true;
if (v->string_view() == "false") return false;
}
return def;
}

10
src/libexpr/primops.cc
View file

@ -590,7 +590,7 @@ struct CompareValues
case nFloat:
return v1->fpoint < v2->fpoint;
case nString:
return strcmp(v1->string.s, v2->string.s) < 0;
return v1->string_view().compare(v2->string_view()) < 0;
case nPath:
return strcmp(v1->_path, v2->_path) < 0;
case nList:
@ -982,7 +982,7 @@ static void prim_trace(EvalState & state, const PosIdx pos, Value * * args, Valu
{
state.forceValue(*args[0], pos);
if (args[0]->type() == nString)
printError("trace: %1%", args[0]->string.s);
printError("trace: %1%", args[0]->string_view());
else
printError("trace: %1%", printValue(state, *args[0]));
state.forceValue(*args[1], pos);
@ -1528,7 +1528,7 @@ static void prim_pathExists(EvalState & state, const PosIdx pos, Value * * args,
auto path = realisePath(state, pos, arg, { .checkForPureEval = false });
/* SourcePath doesn't know about trailing slash. */
auto mustBeDir = arg.type() == nString && arg.str().ends_with("/");
auto mustBeDir = arg.type() == nString && arg.string_view().ends_with("/");
try {
auto checked = state.checkSourcePath(path);
@ -2400,7 +2400,7 @@ static void prim_attrNames(EvalState & state, const PosIdx pos, Value * * args,
(v.listElems()[n++] = state.allocValue())->mkString(state.symbols[i.name]);
std::sort(v.listElems(), v.listElems() + n,
[](Value * v1, Value * v2) { return strcmp(v1->string.s, v2->string.s) < 0; });
[](Value * v1, Value * v2) { return v1->string_view().compare(v2->string_view()) < 0; });
}
static RegisterPrimOp primop_attrNames({
@ -2541,7 +2541,7 @@ static void prim_removeAttrs(EvalState & state, const PosIdx pos, Value * * args
names.reserve(args[1]->listSize());
for (auto elem : args[1]->listItems()) {
state.forceStringNoCtx(*elem, pos, "while evaluating the values of the second argument passed to builtins.removeAttrs");
names.emplace_back(state.symbols.create(elem->string.s), nullptr);
names.emplace_back(state.symbols.create(elem->string_view()), nullptr);
}
std::sort(names.begin(), names.end());

2
src/libexpr/primops/fetchClosure.cc
View file

@ -133,7 +133,7 @@ static void prim_fetchClosure(EvalState & state, const PosIdx pos, Value * * arg
else if (attrName == "toPath") {
state.forceValue(*attr.value, attr.pos);
bool isEmptyString = attr.value->type() == nString && attr.value->string.s == std::string("");
bool isEmptyString = attr.value->type() == nString && attr.value->string_view() == "";
if (isEmptyString) {
toPath = StorePathOrGap {};
}

6
src/libexpr/tests/libexpr.hh
View file

@ -71,7 +71,7 @@ namespace nix {
if (arg.type() != nString) {
return false;
}
return std::string_view(arg.string.s) == s;
return std::string_view(arg.c_str()) == s;
}
MATCHER_P(IsIntEq, v, fmt("The string is equal to \"%1%\"", v)) {
@ -106,8 +106,8 @@ namespace nix {
if (arg.type() != nPath) {
*result_listener << "Expected a path got " << arg.type();
return false;
} else if (std::string_view(arg.string.s) != p) {
*result_listener << "Expected a path that equals \"" << p << "\" but got: " << arg.string.s;
} else if (std::string_view(arg._path) != p) {
*result_listener << "Expected a path that equals \"" << p << "\" but got: " << arg.c_str();
return false;
}
return true;

4
src/libexpr/tests/primops.cc
View file

@ -711,14 +711,14 @@ namespace nix {
// FIXME: add a test that verifies the string context is as expected
auto v = eval("builtins.replaceStrings [\"oo\" \"a\"] [\"a\" \"i\"] \"foobar\"");
ASSERT_EQ(v.type(), nString);
ASSERT_EQ(v.string.s, std::string_view("fabir"));
ASSERT_EQ(v.string_view(), "fabir");
}
TEST_F(PrimOpTest, concatStringsSep) {
// FIXME: add a test that verifies the string context is as expected
auto v = eval("builtins.concatStringsSep \"%\" [\"foo\" \"bar\" \"baz\"]");
ASSERT_EQ(v.type(), nString);
ASSERT_EQ(std::string_view(v.string.s), "foo%bar%baz");
ASSERT_EQ(v.string_view(), "foo%bar%baz");
}
TEST_F(PrimOpTest, split1) {

2
src/libexpr/value-to-json.cc
View file

@ -31,7 +31,7 @@ json printValueAsJSON(EvalState & state, bool strict,
case nString:
copyContext(v, context);
out = v.string.s;
out = v.c_str();
break;
case nPath:

6
src/libexpr/value-to-xml.cc
View file

@ -74,7 +74,7 @@ static void printValueAsXML(EvalState & state, bool strict, bool location,
case nString:
/* !!! show the context? */
copyContext(v, context);
doc.writeEmptyElement("string", singletonAttrs("value", v.string.s));
doc.writeEmptyElement("string", singletonAttrs("value", v.c_str()));
break;
case nPath:
@ -96,14 +96,14 @@ static void printValueAsXML(EvalState & state, bool strict, bool location,
if (a != v.attrs->end()) {
if (strict) state.forceValue(*a->value, a->pos);
if (a->value->type() == nString)
xmlAttrs["drvPath"] = drvPath = a->value->string.s;
xmlAttrs["drvPath"] = drvPath = a->value->c_str();
}
a = v.attrs->find(state.sOutPath);
if (a != v.attrs->end()) {
if (strict) state.forceValue(*a->value, a->pos);
if (a->value->type() == nString)
xmlAttrs["outPath"] = a->value->string.s;
xmlAttrs["outPath"] = a->value->c_str();
}
XMLOpenElement _(doc, "derivation", xmlAttrs);

20
src/libexpr/value.hh
View file

@ -186,10 +186,9 @@ public:
* For canonicity, the store paths should be in sorted order.
*/
struct {
const char * s;
const char * c_str;
const char * * context; // must be in sorted order
} string;
const char * _path;
Bindings * attrs;
struct {
@ -270,7 +269,7 @@ public:
inline void mkString(const char * s, const char * * context = 0)
{
internalType = tString;
string.s = s;
string.c_str = s;
string.context = context;
}
@ -441,10 +440,21 @@ public:
return SourcePath{CanonPath(_path)};
}
std::string_view str() const
std::string_view string_view() const
{
assert(internalType == tString);
return std::string_view(string.s);
return std::string_view(string.c_str);
}
const char * const c_str() const
{
assert(internalType == tString);
return string.c_str;
}
const char * * context() const
{
return string.context;
}
};

6
src/nix-env/nix-env.cc
View file

@ -1228,7 +1228,7 @@ static void opQuery(Globals & globals, Strings opFlags, Strings opArgs)
else {
if (v->type() == nString) {
attrs2["type"] = "string";
attrs2["value"] = v->string.s;
attrs2["value"] = v->c_str();
xml.writeEmptyElement("meta", attrs2);
} else if (v->type() == nInt) {
attrs2["type"] = "int";
@ -1248,7 +1248,7 @@ static void opQuery(Globals & globals, Strings opFlags, Strings opArgs)
for (auto elem : v->listItems()) {
if (elem->type() != nString) continue;
XMLAttrs attrs3;
attrs3["value"] = elem->string.s;
attrs3["value"] = elem->c_str();
xml.writeEmptyElement("string", attrs3);
}
} else if (v->type() == nAttrs) {
@ -1260,7 +1260,7 @@ static void opQuery(Globals & globals, Strings opFlags, Strings opArgs)
if(a.value->type() != nString) continue;
XMLAttrs attrs3;
attrs3["type"] = globals.state->symbols[i.name];
attrs3["value"] = a.value->string.s;
attrs3["value"] = a.value->c_str();
xml.writeEmptyElement("string", attrs3);
}
}

2
src/nix/eval.cc
View file

@ -85,7 +85,7 @@ struct CmdEval : MixJSON, InstallableValueCommand, MixReadOnlyOption
state->forceValue(v, pos);
if (v.type() == nString)
// FIXME: disallow strings with contexts?
writeFile(path, v.string.s);
writeFile(path, v.string_view());
else if (v.type() == nAttrs) {
if (mkdir(path.c_str(), 0777) == -1)
throw SysError("creating directory '%s'", path);