refactor: use string accessors

Create context, string_view, and c_str, accessors throughout in order to
better support improvements to the underlying string representation.

src/libcmd/repl.cc

@@ -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;
 

src/libexpr/eval-cache.cc

@@ -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(), {}};

src/libexpr/eval.cc

@@ -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;

src/libexpr/flake/flake.cc

@@ -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, "

src/libexpr/get-drvs.cc

@@ -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;
 }

src/libexpr/primops.cc

@@ -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());
 

src/libexpr/primops/fetchClosure.cc

@@ -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 {};
  }

src/libexpr/tests/primops.cc

@@ -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) {

src/libexpr/value-to-json.cc

@@ -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:

src/libexpr/value-to-xml.cc

@@ -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);

src/libexpr/value.hh

@@ -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.c_str);
+ }
+
+ const char * const c_str() const
  {
  assert(internalType == tString);
- return std::string_view(string.s);
+ return string.c_str;
+ }
+
+ const char * * context() const
+ {
+ return string.context;
  }
 };
 

src/nix-env/nix-env.cc

@@ -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);
  }
  }

src/nix/eval.cc

@@ -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);