lix-project/lix
18
39
Fork 15
Code Issues 288 Code Review (Gerrit) Projects 1 Releases Wiki Activity

libexpr: abstract string members in Value

Browse source 
to allow multiple internal reps similar to how lists works

Change-Id: I94139aab28b04b73b055a3dc05d496758ee72538
This commit is contained in:
Mel Zuser 2024-05-15 20:03:19 -07:00
parent 9249c89dc6
commit 1eff2adc93
16 changed files with 72 additions and 48 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/libcmd/repl.cc
View file

@ -674,7 +674,7 @@ ProcessLineResult NixRepl::processLine(std::string line)
Value v;
evalString(arg, v);
if (v.type() == nString) {
std::cout << v.string.s;
std::cout << v.c_str();
} else {
printValue(std::cout, v);
}

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.stringContext()),
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(), {}}};
@ -580,7 +580,7 @@ std::string AttrCursor::getString()
root->state.error<TypeError>("'%s' is not a string but %s", getAttrPathStr(), v.type()).debugThrow();
}
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()
@ -622,7 +622,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(), {}};
} else {

20
src/libexpr/eval.cc
View file

@ -253,7 +253,7 @@ static Symbol getName(const AttrName & name, EvalState & state, Env & env)
Value nameValue;
name.expr->eval(state, env, nameValue);
state.forceStringNoCtx(nameValue, name.expr->getPos(), "while evaluating an attribute name");
return state.symbols.create(nameValue.string.s);
return state.symbols.create(nameValue.c_str());
}
}
@ -1330,7 +1330,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.c_str());
Bindings::iterator j = v.attrs->find(nameSym);
if (j != v.attrs->end())
state.error<EvalError>("dynamic attribute '%1%' already defined at %2%", state.symbols[nameSym], state.positions[j->pos]).atPos(i.pos).withFrame(env, *this).debugThrow();
@ -2256,7 +2256,7 @@ std::string_view EvalState::forceString(Value & v, const PosIdx pos, std::string
showType(v),
ValuePrinter(*this, v, errorPrintOptions)
).atPos(pos).debugThrow();
return v.string.s;
return v.str();
} catch (Error & e) {
e.addTrace(positions[pos], errorCtx);
throw;
@ -2266,8 +2266,8 @@ std::string_view EvalState::forceString(Value & v, const PosIdx pos, std::string
void copyContext(const Value & v, NixStringContext & context)
{
if (v.string.context)
for (const char * * p = v.string.context; *p; ++p)
if (v.stringContext())
for (const char * * p = v.stringContext(); *p; ++p)
context.insert(NixStringContextElem::parse(*p));
}
@ -2283,8 +2283,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<EvalError>("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();
if (v.stringContext()) {
error<EvalError>("the string '%1%' is not allowed to refer to a store path (such as '%2%')", v.c_str(), v.stringContext()[0]).withTrace(pos, errorCtx).debugThrow();
}
return s;
}
@ -2297,7 +2297,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 strcmp(i->value->c_str(), "derivation") == 0;
}
@ -2329,7 +2329,7 @@ BackedStringView EvalState::coerceToString(
if (v.type() == nString) {
copyContext(v, context);
return std::string_view(v.string.s);
return v.str();
}
if (v.type() == nPath) {
@ -2534,7 +2534,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 strcmp(v1.c_str(), v2.c_str()) == 0;
case nPath:
return strcmp(v1._path, v2._path) == 0;

8
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->c_str();
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, depth + 1);
} 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 });
@ -238,7 +238,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");

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 (strcmp(v->c_str(), "true") == 0) return true;
if (strcmp(v->c_str(), "false") == 0) return false;
}
return def;
}

10
src/libexpr/primops.cc
View file

@ -594,7 +594,7 @@ struct CompareValues
case nFloat:
return v1->fpoint < v2->fpoint;
case nString:
return strcmp(v1->string.s, v2->string.s) < 0;
return strcmp(v1->c_str(), v2->c_str()) < 0;
case nPath:
return strcmp(v1->_path, v2->_path) < 0;
case nList:
@ -977,7 +977,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]->c_str());
else
printError("trace: %1%", ValuePrinter(state, *args[0]));
if (evalSettings.builtinsTraceDebugger && state.debugRepl && !state.debugTraces.empty()) {
@ -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 strcmp(v1->c_str(), v2->c_str()) < 0; });
}
static RegisterPrimOp primop_attrNames({
@ -2590,7 +2590,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->c_str()), nullptr);
}
std::sort(names.begin(), names.end());
@ -3740,7 +3740,7 @@ static void prim_substring(EvalState & state, const PosIdx pos, Value * * args,
if (len == 0) {
state.forceValue(*args[2], pos);
if (args[2]->type() == nString) {
v.mkString("", args[2]->string.context);
v.mkString("", args[2]->stringContext());
return;
}
}

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->c_str() == std::string("");
if (isEmptyString) {
toPath = StorePathOrGap {};
}

2
src/libexpr/print-ambiguous.cc
View file

@ -28,7 +28,7 @@ void printAmbiguous(
printLiteralBool(str, v.boolean);
break;
case nString:
escapeString(str, v.string.s);
escapeString(str, v.c_str());
break;
case nPath:
str << v.path().to_string(); // !!! escaping?

2
src/libexpr/print.cc
View file

@ -200,7 +200,7 @@ private:
{
escapeString(
output,
v.string.s,
v.c_str(),
{
.maxLength = options.maxStringLength,
.outputAnsiColors = options.ansiColors,

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

@ -32,7 +32,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

@ -75,7 +75,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:
@ -97,14 +97,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);

30
src/libexpr/value.hh
View file

@ -437,11 +437,35 @@ public:
return SourcePath{CanonPath(_path)};
}
std::string_view str() const
// Allow selecting a subset of enum values
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wswitch-enum"
const char * c_str() const
{
assert(internalType == tString);
return std::string_view(string.s);
switch(internalType) {
case tString: return string.s;
default: abort();
}
}
std::string_view str()
{
switch(internalType) {
case tString: return std::string_view(string.s);
default: abort();
}
}
const char * * stringContext() const
{
switch(internalType) {
case tString: return string.context;
default: abort();
}
}
#pragma GCC diagnostic pop
};

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

@ -1233,7 +1233,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";
@ -1253,7 +1253,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) {
@ -1265,7 +1265,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

@ -86,7 +86,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.c_str());
else if (v.type() == nAttrs) {
if (mkdir(path.c_str(), 0777) == -1)
throw SysError("creating directory '%s'", path);

4
tests/unit/libexpr-support/tests/libexpr.hh
View file

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

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

@ -713,14 +713,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.c_str(), std::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(std::string_view(v.c_str()), "foo%bar%baz");
}
TEST_F(PrimOpTest, split1) {