Skip to content

Commit 13a9090

Browse files
roberthroberth
authored
Merge pull request #9047 from flox/tomberek.string_refactor
string Value refactor
2 parents 13ed5d7 + 399ef84 commit 13a9090

File tree

14 files changed

+59
-49
lines changed

14 files changed

+59
-49
lines changed

‎src/libcmd/repl.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -922,7 +922,7 @@ std::ostream & NixRepl::printValue(std::ostream & str, Value & v, unsigned int m
922922

923923
case nString:
924924
str << ANSI_WARNING;
925-
printLiteralString(str, v.string.s);
925+
printLiteralString(str, v.string_view());
926926
str << ANSI_NORMAL;
927927
break;
928928

‎src/libexpr/eval-cache.cc‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -440,8 +440,8 @@ Value & AttrCursor::forceValue()
440440

441441
if (root->db && (!cachedValue || std::get_if<placeholder_t>(&cachedValue->second))) {
442442
if (v.type() == nString)
443-
cachedValue = {root->db->setString(getKey(), v.string.s, v.string.context),
444-
string_t{v.string.s, {}}};
443+
cachedValue = {root->db->setString(getKey(), v.c_str(), v.context()),
444+
string_t{v.c_str(), {}}};
445445
else if (v.type() == nPath) {
446446
auto path = v.path().path;
447447
cachedValue = {root->db->setString(getKey(), path.abs()), string_t{path.abs(), {}}};
@@ -582,7 +582,7 @@ std::string AttrCursor::getString()
582582
if (v.type() != nString && v.type() != nPath)
583583
root->state.error("'%s' is not a string but %s", getAttrPathStr()).debugThrow<TypeError>();
584584

585-
return v.type() == nString ? v.string.s : v.path().to_string();
585+
return v.type() == nString ? v.c_str() : v.path().to_string();
586586
}
587587

588588
string_t AttrCursor::getStringWithContext()
@@ -624,7 +624,7 @@ string_t AttrCursor::getStringWithContext()
624624
if (v.type() == nString) {
625625
NixStringContext context;
626626
copyContext(v, context);
627-
return {v.string.s, std::move(context)};
627+
return {v.c_str(), std::move(context)};
628628
}
629629
else if (v.type() == nPath)
630630
return {v.path().to_string(), {}};

‎src/libexpr/eval.cc‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ void Value::print(const SymbolTable &symbols, std::ostream &str,
114114
printLiteralBool(str, boolean);
115115
break;
116116
case tString:
117-
printLiteralString(str, string.s);
117+
printLiteralString(str, string_view());
118118
break;
119119
case tPath:
120120
str << path().to_string(); // !!! escaping?
@@ -339,7 +339,7 @@ static Symbol getName(const AttrName & name, EvalState & state, Env & env)
339339
Value nameValue;
340340
name.expr->eval(state, env, nameValue);
341341
state.forceStringNoCtx(nameValue, noPos, "while evaluating an attribute name");
342-
return state.symbols.create(nameValue.string.s);
342+
return state.symbols.create(nameValue.string_view());
343343
}
344344
}
345345

@@ -1343,7 +1343,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
13431343
if (nameVal.type() == nNull)
13441344
continue;
13451345
state.forceStringNoCtx(nameVal, i.pos, "while evaluating the name of a dynamic attribute");
1346-
auto nameSym = state.symbols.create(nameVal.string.s);
1346+
auto nameSym = state.symbols.create(nameVal.string_view());
13471347
Bindings::iterator j = v.attrs->find(nameSym);
13481348
if (j != v.attrs->end())
13491349
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
21552155
forceValue(v, pos);
21562156
if (v.type() != nString)
21572157
error("value is %1% while a string was expected", showType(v)).debugThrow<TypeError>();
2158-
return v.string.s;
2158+
return v.string_view();
21592159
} catch (Error & e) {
21602160
e.addTrace(positions[pos], errorCtx);
21612161
throw;
@@ -2182,8 +2182,8 @@ std::string_view EvalState::forceString(Value & v, NixStringContext & context, c
21822182
std::string_view EvalState::forceStringNoCtx(Value & v, const PosIdx pos, std::string_view errorCtx)
21832183
{
21842184
auto s = forceString(v, pos, errorCtx);
2185-
if (v.string.context) {
2186-
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>();
2185+
if (v.context()) {
2186+
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>();
21872187
}
21882188
return s;
21892189
}
@@ -2196,7 +2196,7 @@ bool EvalState::isDerivation(Value & v)
21962196
if (i == v.attrs->end()) return false;
21972197
forceValue(*i->value, i->pos);
21982198
if (i->value->type() != nString) return false;
2199-
return strcmp(i->value->string.s, "derivation") == 0;
2199+
return i->value->string_view().compare("derivation") == 0;
22002200
}
22012201

22022202

@@ -2228,7 +2228,7 @@ BackedStringView EvalState::coerceToString(
22282228

22292229
if (v.type() == nString) {
22302230
copyContext(v, context);
2231-
return std::string_view(v.string.s);
2231+
return v.string_view();
22322232
}
22332233

22342234
if (v.type() == nPath) {
@@ -2426,7 +2426,7 @@ bool EvalState::eqValues(Value & v1, Value & v2, const PosIdx pos, std::string_v
24262426
return v1.boolean == v2.boolean;
24272427

24282428
case nString:
2429-
return strcmp(v1.string.s, v2.string.s) == 0;
2429+
return v1.string_view().compare(v2.string_view()) == 0;
24302430

24312431
case nPath:
24322432
return strcmp(v1._path, v2._path) == 0;

‎src/libexpr/flake/flake.cc‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ static FlakeInput parseFlakeInput(EvalState & state,
113113
try {
114114
if (attr.name == sUrl) {
115115
expectType(state, nString, *attr.value, attr.pos);
116-
url = attr.value->string.s;
116+
url = attr.value->string_view();
117117
attrs.emplace("url", *url);
118118
} else if (attr.name == sFlake) {
119119
expectType(state, nBool, *attr.value, attr.pos);
@@ -122,7 +122,7 @@ static FlakeInput parseFlakeInput(EvalState & state,
122122
input.overrides = parseFlakeInputs(state, attr.value, attr.pos, baseDir, lockRootPath);
123123
} else if (attr.name == sFollows) {
124124
expectType(state, nString, *attr.value, attr.pos);
125-
auto follows(parseInputPath(attr.value->string.s));
125+
auto follows(parseInputPath(attr.value->c_str()));
126126
follows.insert(follows.begin(), lockRootPath.begin(), lockRootPath.end());
127127
input.follows = follows;
128128
} else {
@@ -131,7 +131,7 @@ static FlakeInput parseFlakeInput(EvalState & state,
131131
#pragma GCC diagnostic ignored "-Wswitch-enum"
132132
switch (attr.value->type()) {
133133
case nString:
134-
attrs.emplace(state.symbols[attr.name], attr.value->string.s);
134+
attrs.emplace(state.symbols[attr.name], attr.value->c_str());
135135
break;
136136
case nBool:
137137
attrs.emplace(state.symbols[attr.name], Explicit<bool> { attr.value->boolean });
@@ -229,7 +229,7 @@ static Flake getFlake(
229229

230230
if (auto description = vInfo.attrs->get(state.sDescription)) {
231231
expectType(state, nString, *description->value, description->pos);
232-
flake.description = description->value->string.s;
232+
flake.description = description->value->c_str();
233233
}
234234

235235
auto sInputs = state.symbols.create("inputs");
@@ -850,7 +850,7 @@ static void prim_flakeRefToString(
850850
Explicit<bool> { attr.value->boolean });
851851
} else if (t == nString) {
852852
attrs.emplace(state.symbols[attr.name],
853-
std::string(attr.value->str()));
853+
std::string(attr.value->string_view()));
854854
} else {
855855
state.error(
856856
"flake reference attribute sets may only contain integers, Booleans, "

‎src/libexpr/get-drvs.cc‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ DrvInfo::Outputs DrvInfo::queryOutputs(bool withPaths, bool onlyOutputsToInstall
156156
Outputs result;
157157
for (auto elem : outTI->listItems()) {
158158
if (elem->type() != nString) throw errMsg;
159-
auto out = outputs.find(elem->string.s);
159+
auto out = outputs.find(elem->c_str());
160160
if (out == outputs.end()) throw errMsg;
161161
result.insert(*out);
162162
}
@@ -230,7 +230,7 @@ std::string DrvInfo::queryMetaString(const std::string & name)
230230
{
231231
Value * v = queryMeta(name);
232232
if (!v || v->type() != nString) return "";
233-
return v->string.s;
233+
return v->c_str();
234234
}
235235

236236

@@ -242,7 +242,7 @@ NixInt DrvInfo::queryMetaInt(const std::string & name, NixInt def)
242242
if (v->type() == nString) {
243243
/* Backwards compatibility with before we had support for
244244
integer meta fields. */
245-
if (auto n = string2Int<NixInt>(v->string.s))
245+
if (auto n = string2Int<NixInt>(v->c_str()))
246246
return *n;
247247
}
248248
return def;
@@ -256,7 +256,7 @@ NixFloat DrvInfo::queryMetaFloat(const std::string & name, NixFloat def)
256256
if (v->type() == nString) {
257257
/* Backwards compatibility with before we had support for
258258
float meta fields. */
259-
if (auto n = string2Float<NixFloat>(v->string.s))
259+
if (auto n = string2Float<NixFloat>(v->c_str()))
260260
return *n;
261261
}
262262
return def;
@@ -271,8 +271,8 @@ bool DrvInfo::queryMetaBool(const std::string & name, bool def)
271271
if (v->type() == nString) {
272272
/* Backwards compatibility with before we had support for
273273
Boolean meta fields. */
274-
if (strcmp(v->string.s, "true") == 0) return true;
275-
if (strcmp(v->string.s, "false") == 0) return false;
274+
if (v->string_view() == "true") return true;
275+
if (v->string_view() == "false") return false;
276276
}
277277
return def;
278278
}

‎src/libexpr/primops.cc‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -590,7 +590,7 @@ struct CompareValues
590590
case nFloat:
591591
return v1->fpoint < v2->fpoint;
592592
case nString:
593-
return strcmp(v1->string.s, v2->string.s) < 0;
593+
return v1->string_view().compare(v2->string_view()) < 0;
594594
case nPath:
595595
return strcmp(v1->_path, v2->_path) < 0;
596596
case nList:
@@ -982,7 +982,7 @@ static void prim_trace(EvalState & state, const PosIdx pos, Value * * args, Valu
982982
{
983983
state.forceValue(*args[0], pos);
984984
if (args[0]->type() == nString)
985-
printError("trace: %1%", args[0]->string.s);
985+
printError("trace: %1%", args[0]->string_view());
986986
else
987987
printError("trace: %1%", printValue(state, *args[0]));
988988
state.forceValue(*args[1], pos);
@@ -1528,7 +1528,7 @@ static void prim_pathExists(EvalState & state, const PosIdx pos, Value * * args,
15281528
auto path = realisePath(state, pos, arg, { .checkForPureEval = false });
15291529

15301530
/* SourcePath doesn't know about trailing slash. */
1531-
auto mustBeDir = arg.type() == nString && arg.str().ends_with("/");
1531+
auto mustBeDir = arg.type() == nString && arg.string_view().ends_with("/");
15321532

15331533
try {
15341534
auto checked = state.checkSourcePath(path);
@@ -2400,7 +2400,7 @@ static void prim_attrNames(EvalState & state, const PosIdx pos, Value * * args,
24002400
(v.listElems()[n++] = state.allocValue())->mkString(state.symbols[i.name]);
24012401

24022402
std::sort(v.listElems(), v.listElems() + n,
2403-
[](Value * v1, Value * v2) { return strcmp(v1->string.s, v2->string.s) < 0; });
2403+
[](Value * v1, Value * v2) { return v1->string_view().compare(v2->string_view()) < 0; });
24042404
}
24052405

24062406
static RegisterPrimOp primop_attrNames({
@@ -2541,7 +2541,7 @@ static void prim_removeAttrs(EvalState & state, const PosIdx pos, Value * * args
25412541
names.reserve(args[1]->listSize());
25422542
for (auto elem : args[1]->listItems()) {
25432543
state.forceStringNoCtx(*elem, pos, "while evaluating the values of the second argument passed to builtins.removeAttrs");
2544-
names.emplace_back(state.symbols.create(elem->string.s), nullptr);
2544+
names.emplace_back(state.symbols.create(elem->string_view()), nullptr);
25452545
}
25462546
std::sort(names.begin(), names.end());
25472547

‎src/libexpr/primops/fetchClosure.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ static void prim_fetchClosure(EvalState & state, const PosIdx pos, Value * * arg
133133

134134
else if (attrName == "toPath") {
135135
state.forceValue(*attr.value, attr.pos);
136-
bool isEmptyString = attr.value->type() == nString && attr.value->string.s == std::string("");
136+
bool isEmptyString = attr.value->type() == nString && attr.value->string_view() == "";
137137
if (isEmptyString) {
138138
toPath = StorePathOrGap {};
139139
}

‎src/libexpr/tests/libexpr.hh‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ namespace nix {
7171
if (arg.type() != nString) {
7272
return false;
7373
}
74-
return std::string_view(arg.string.s) == s;
74+
return std::string_view(arg.c_str()) == s;
7575
}
7676

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

‎src/libexpr/tests/primops.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -711,14 +711,14 @@ namespace nix {
711711
// FIXME: add a test that verifies the string context is as expected
712712
auto v = eval("builtins.replaceStrings [\"oo\" \"a\"] [\"a\" \"i\"] \"foobar\"");
713713
ASSERT_EQ(v.type(), nString);
714-
ASSERT_EQ(v.string.s, std::string_view("fabir"));
714+
ASSERT_EQ(v.string_view(), "fabir");
715715
}
716716

717717
TEST_F(PrimOpTest, concatStringsSep) {
718718
// FIXME: add a test that verifies the string context is as expected
719719
auto v = eval("builtins.concatStringsSep \"%\" [\"foo\" \"bar\" \"baz\"]");
720720
ASSERT_EQ(v.type(), nString);
721-
ASSERT_EQ(std::string_view(v.string.s), "foo%bar%baz");
721+
ASSERT_EQ(v.string_view(), "foo%bar%baz");
722722
}
723723

724724
TEST_F(PrimOpTest, split1) {

‎src/libexpr/value-to-json.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ json printValueAsJSON(EvalState & state, bool strict,
3131

3232
case nString:
3333
copyContext(v, context);
34-
out = v.string.s;
34+
out = v.c_str();
3535
break;
3636

3737
case nPath:

0 commit comments

Comments
 (0)