1
1
Fork 0
mirror of https://github.com/NixOS/nix.git synced 2025-11-09 20:16:03 +01:00

Re-introduce mkStringNoCopy (revised)

In b70d22b `mkStringNoCopy()` was renamed to
`mkString()`, but this is a bit risky since in code like

    vStringRegular.mkString("regular");

we want to be sure that the right overload is picked. (This is
especially problematic since the overload that takes an
`std::string_view` *does* allocate.)  So let's be explicit.

(Rebased from https://github.com/NixOS/nix/pull/11551)
This commit is contained in:
Eelco Dolstra 2024-09-19 20:33:35 +02:00 committed by Sergei Zimmerman
parent a0ce514769
commit d62cfc1c97
No known key found for this signature in database
9 changed files with 26 additions and 26 deletions

View file

@ -54,7 +54,7 @@ TEST_F(JSONValueTest, IntNegative)
TEST_F(JSONValueTest, String) TEST_F(JSONValueTest, String)
{ {
Value v; Value v;
v.mkString("test"); v.mkStringNoCopy("test");
ASSERT_EQ(getJSONValue(v), "\"test\""); ASSERT_EQ(getJSONValue(v), "\"test\"");
} }
@ -62,7 +62,7 @@ TEST_F(JSONValueTest, StringQuotes)
{ {
Value v; Value v;
v.mkString("test\""); v.mkStringNoCopy("test\"");
ASSERT_EQ(getJSONValue(v), "\"test\\\"\""); ASSERT_EQ(getJSONValue(v), "\"test\\\"\"");
} }

View file

@ -35,14 +35,14 @@ TEST_F(ValuePrintingTests, tBool)
TEST_F(ValuePrintingTests, tString) TEST_F(ValuePrintingTests, tString)
{ {
Value vString; Value vString;
vString.mkString("some-string"); vString.mkStringNoCopy("some-string");
test(vString, "\"some-string\""); test(vString, "\"some-string\"");
} }
TEST_F(ValuePrintingTests, tPath) TEST_F(ValuePrintingTests, tPath)
{ {
Value vPath; Value vPath;
vPath.mkString("/foo"); vPath.mkStringNoCopy("/foo");
test(vPath, "\"/foo\""); test(vPath, "\"/foo\"");
} }
@ -290,10 +290,10 @@ TEST_F(StringPrintingTests, maxLengthTruncation)
TEST_F(ValuePrintingTests, attrsTypeFirst) TEST_F(ValuePrintingTests, attrsTypeFirst)
{ {
Value vType; Value vType;
vType.mkString("puppy"); vType.mkStringNoCopy("puppy");
Value vApple; Value vApple;
vApple.mkString("apple"); vApple.mkStringNoCopy("apple");
BindingsBuilder builder(state, state.allocBindings(10)); BindingsBuilder builder(state, state.allocBindings(10));
builder.insert(state.symbols.create("type"), &vType); builder.insert(state.symbols.create("type"), &vType);
@ -334,7 +334,7 @@ TEST_F(ValuePrintingTests, ansiColorsBool)
TEST_F(ValuePrintingTests, ansiColorsString) TEST_F(ValuePrintingTests, ansiColorsString)
{ {
Value v; Value v;
v.mkString("puppy"); v.mkStringNoCopy("puppy");
test(v, ANSI_MAGENTA "\"puppy\"" ANSI_NORMAL, PrintOptions{.ansiColors = true}); test(v, ANSI_MAGENTA "\"puppy\"" ANSI_NORMAL, PrintOptions{.ansiColors = true});
} }
@ -342,7 +342,7 @@ TEST_F(ValuePrintingTests, ansiColorsString)
TEST_F(ValuePrintingTests, ansiColorsStringElided) TEST_F(ValuePrintingTests, ansiColorsStringElided)
{ {
Value v; Value v;
v.mkString("puppy"); v.mkStringNoCopy("puppy");
test( test(
v, v,
@ -390,7 +390,7 @@ TEST_F(ValuePrintingTests, ansiColorsAttrs)
TEST_F(ValuePrintingTests, ansiColorsDerivation) TEST_F(ValuePrintingTests, ansiColorsDerivation)
{ {
Value vDerivation; Value vDerivation;
vDerivation.mkString("derivation"); vDerivation.mkStringNoCopy("derivation");
BindingsBuilder builder(state, state.allocBindings(10)); BindingsBuilder builder(state, state.allocBindings(10));
builder.insert(state.s.type, &vDerivation); builder.insert(state.s.type, &vDerivation);
@ -413,7 +413,7 @@ TEST_F(ValuePrintingTests, ansiColorsError)
{ {
Value throw_ = state.getBuiltin("throw"); Value throw_ = state.getBuiltin("throw");
Value message; Value message;
message.mkString("uh oh!"); message.mkStringNoCopy("uh oh!");
Value vError; Value vError;
vError.mkApp(&throw_, &message); vError.mkApp(&throw_, &message);
@ -430,12 +430,12 @@ TEST_F(ValuePrintingTests, ansiColorsDerivationError)
{ {
Value throw_ = state.getBuiltin("throw"); Value throw_ = state.getBuiltin("throw");
Value message; Value message;
message.mkString("uh oh!"); message.mkStringNoCopy("uh oh!");
Value vError; Value vError;
vError.mkApp(&throw_, &message); vError.mkApp(&throw_, &message);
Value vDerivation; Value vDerivation;
vDerivation.mkString("derivation"); vDerivation.mkStringNoCopy("derivation");
BindingsBuilder builder(state, state.allocBindings(10)); BindingsBuilder builder(state, state.allocBindings(10));
builder.insert(state.s.type, &vDerivation); builder.insert(state.s.type, &vDerivation);

View file

@ -292,10 +292,10 @@ EvalState::EvalState(
vNull.mkNull(); vNull.mkNull();
vTrue.mkBool(true); vTrue.mkBool(true);
vFalse.mkBool(false); vFalse.mkBool(false);
vStringRegular.mkString("regular"); vStringRegular.mkStringNoCopy("regular");
vStringDirectory.mkString("directory"); vStringDirectory.mkStringNoCopy("directory");
vStringSymlink.mkString("symlink"); vStringSymlink.mkStringNoCopy("symlink");
vStringUnknown.mkString("unknown"); vStringUnknown.mkStringNoCopy("unknown");
/* Construct the Nix expression search path. */ /* Construct the Nix expression search path. */
assert(lookupPath.elements.empty()); assert(lookupPath.elements.empty());
@ -824,7 +824,7 @@ DebugTraceStacker::DebugTraceStacker(EvalState & evalState, DebugTrace t)
void Value::mkString(std::string_view s) void Value::mkString(std::string_view s)
{ {
mkString(makeImmutableString(s)); mkStringNoCopy(makeImmutableString(s));
} }
static const char ** encodeContext(const NixStringContext & context) static const char ** encodeContext(const NixStringContext & context)
@ -843,12 +843,12 @@ static const char ** encodeContext(const NixStringContext & context)
void Value::mkString(std::string_view s, const NixStringContext & context) void Value::mkString(std::string_view s, const NixStringContext & context)
{ {
mkString(makeImmutableString(s), encodeContext(context)); mkStringNoCopy(makeImmutableString(s), encodeContext(context));
} }
void Value::mkStringMove(const char * s, const NixStringContext & context) void Value::mkStringMove(const char * s, const NixStringContext & context)
{ {
mkString(s, encodeContext(context)); mkStringNoCopy(s, encodeContext(context));
} }
void Value::mkPath(const SourcePath & path) void Value::mkPath(const SourcePath & path)

View file

@ -158,7 +158,7 @@ struct ExprString : Expr
ExprString(std::string && s) ExprString(std::string && s)
: s(std::move(s)) : s(std::move(s))
{ {
v.mkString(this->s.data()); v.mkStringNoCopy(this->s.data());
}; };
Value * maybeThunk(EvalState & state, Env & env) override; Value * maybeThunk(EvalState & state, Env & env) override;

View file

@ -113,12 +113,12 @@ public:
// for multi-threaded implementations: lock store and allocator here // for multi-threaded implementations: lock store and allocator here
const auto & [v, idx] = key.store.add(SymbolValue{}); const auto & [v, idx] = key.store.add(SymbolValue{});
if (size == 0) { if (size == 0) {
v.mkString("", nullptr); v.mkStringNoCopy("", nullptr);
} else { } else {
auto s = key.alloc.allocate(size + 1); auto s = key.alloc.allocate(size + 1);
memcpy(s, key.s.data(), size); memcpy(s, key.s.data(), size);
s[size] = '\0'; s[size] = '\0';
v.mkString(s, nullptr); v.mkStringNoCopy(s, nullptr);
} }
v.size_ = size; v.size_ = size;
v.idx = idx; v.idx = idx;

View file

@ -960,7 +960,7 @@ public:
setStorage(b); setStorage(b);
} }
inline void mkString(const char * s, const char ** context = 0) noexcept void mkStringNoCopy(const char * s, const char ** context = 0) noexcept
{ {
setStorage(StringWithContext{.c_str = s, .context = context}); setStorage(StringWithContext{.c_str = s, .context = context});
} }

View file

@ -4349,7 +4349,7 @@ static void prim_substring(EvalState & state, const PosIdx pos, Value ** args, V
if (len == 0) { if (len == 0) {
state.forceValue(*args[2], pos); state.forceValue(*args[2], pos);
if (args[2]->type() == nString) { if (args[2]->type() == nString) {
v.mkString("", args[2]->context()); v.mkStringNoCopy("", args[2]->context());
return; return;
} }
} }

View file

@ -136,7 +136,7 @@ static void prim_fromTOML(EvalState & state, const PosIdx pos, Value ** args, Va
normalizeDatetimeFormat(t); normalizeDatetimeFormat(t);
#endif #endif
auto attrs = state.buildBindings(2); auto attrs = state.buildBindings(2);
attrs.alloc("_type").mkString("timestamp"); attrs.alloc("_type").mkStringNoCopy("timestamp");
std::ostringstream s; std::ostringstream s;
s << t; s << t;
auto str = toView(s); auto str = toView(s);

View file

@ -56,7 +56,7 @@ bool createUserEnv(
auto attrs = state.buildBindings(7 + outputs.size()); auto attrs = state.buildBindings(7 + outputs.size());
attrs.alloc(state.s.type).mkString("derivation"); attrs.alloc(state.s.type).mkStringNoCopy("derivation");
attrs.alloc(state.s.name).mkString(i.queryName()); attrs.alloc(state.s.name).mkString(i.queryName());
auto system = i.querySystem(); auto system = i.querySystem();
if (!system.empty()) if (!system.empty())