diff --git a/src/libexpr/eval-cache.cc b/src/libexpr/eval-cache.cc index de74d2143..0372d6cc1 100644 --- a/src/libexpr/eval-cache.cc +++ b/src/libexpr/eval-cache.cc @@ -136,17 +136,19 @@ struct AttrDb }); } - AttrId setString(AttrKey key, std::string_view s, const char ** context = nullptr) + AttrId setString(AttrKey key, std::string_view s, const Value::StringWithContext::Context * context = nullptr) { return doSQLite([&]() { auto state(_state->lock()); if (context) { std::string ctx; - for (const char ** p = context; *p; ++p) { - if (p != context) + bool first = true; + for (auto * elem : *context) { + if (!first) ctx.push_back(' '); - ctx.append(*p); + ctx.append(elem); + first = false; } state->insertAttributeWithContext.use()(key.first)(symbols[key.second])(AttrType::String) (s) (ctx) .exec(); diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 7036df957..940c6a937 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -36,6 +36,7 @@ #include #include #include +#include #include #include @@ -821,28 +822,25 @@ void Value::mkString(std::string_view s) mkStringNoCopy(makeImmutableString(s)); } -static const char ** encodeContext(const NixStringContext & context) +Value::StringWithContext::Context * Value::StringWithContext::Context::fromBuilder(const NixStringContext & context) { - if (!context.empty()) { - size_t n = 0; - auto ctx = (const char **) allocBytes((context.size() + 1) * sizeof(char *)); - for (auto & i : context) { - ctx[n++] = makeImmutableString({i.to_string()}); - } - ctx[n] = nullptr; - return ctx; - } else + if (context.empty()) return nullptr; + + auto ctx = new (allocBytes(sizeof(Context) + context.size() * sizeof(value_type))) Context(context.size()); + std::ranges::transform( + context, ctx->elems, [](const NixStringContextElem & elt) { return makeImmutableString(elt.to_string()); }); + return ctx; } void Value::mkString(std::string_view s, const NixStringContext & context) { - mkStringNoCopy(makeImmutableString(s), encodeContext(context)); + mkStringNoCopy(makeImmutableString(s), Value::StringWithContext::Context::fromBuilder(context)); } void Value::mkStringMove(const char * s, const NixStringContext & context) { - mkStringNoCopy(s, encodeContext(context)); + mkStringNoCopy(s, Value::StringWithContext::Context::fromBuilder(context)); } void Value::mkPath(const SourcePath & path) @@ -2287,9 +2285,9 @@ std::string_view EvalState::forceString(Value & v, const PosIdx pos, std::string void copyContext(const Value & v, NixStringContext & context, const ExperimentalFeatureSettings & xpSettings) { - if (v.context()) - for (const char ** p = v.context(); *p; ++p) - context.insert(NixStringContextElem::parse(*p, xpSettings)); + if (auto * ctx = v.context()) + for (auto * elem : *ctx) + context.insert(NixStringContextElem::parse(elem, xpSettings)); } std::string_view EvalState::forceString( @@ -2309,7 +2307,9 @@ std::string_view EvalState::forceStringNoCtx(Value & v, const PosIdx pos, std::s auto s = forceString(v, pos, errorCtx); 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]) + "the string '%1%' is not allowed to refer to a store path (such as '%2%')", + v.string_view(), + *v.context()->begin()) .withTrace(pos, errorCtx) .debugThrow(); } diff --git a/src/libexpr/include/nix/expr/value.hh b/src/libexpr/include/nix/expr/value.hh index 706a4fe3f..0fb36e1b2 100644 --- a/src/libexpr/include/nix/expr/value.hh +++ b/src/libexpr/include/nix/expr/value.hh @@ -220,7 +220,66 @@ struct ValueBase struct StringWithContext { const char * c_str; - const char ** context; // must be in sorted order + + /** + * The type of the context itself. + * + * Currently, it is length-prefixed array of pointers to + * null-terminated strings. The strings are specially formatted + * to represent a flattening of the recursive sum type that is a + * context element. + * + * @See NixStringContext for an more easily understood type, + * that of the "builder" for this data structure. + */ + struct Context + { + using value_type = const char *; + using size_type = std::size_t; + using iterator = const value_type *; + + Context(size_type size) + : size_(size) + { + } + + private: + /** + * Number of items in the array + */ + size_type size_; + + /** + * @pre must be in sorted order + */ + value_type elems[]; + + public: + iterator begin() const + { + return elems; + } + + iterator end() const + { + return elems + size(); + } + + size_type size() const + { + return size_; + } + + /** + * @return null pointer when context.empty() + */ + static Context * fromBuilder(const NixStringContext & context); + }; + + /** + * May be null for a string without context. + */ + const Context * context; }; struct Path @@ -991,7 +1050,7 @@ public: setStorage(b); } - void mkStringNoCopy(const char * s, const char ** context = 0) noexcept + void mkStringNoCopy(const char * s, const Value::StringWithContext::Context * context = nullptr) noexcept { setStorage(StringWithContext{.c_str = s, .context = context}); } @@ -1117,7 +1176,7 @@ public: return getStorage().c_str; } - const char ** context() const noexcept + const Value::StringWithContext::Context * context() const noexcept { return getStorage().context; } diff --git a/src/libexpr/include/nix/expr/value/context.hh b/src/libexpr/include/nix/expr/value/context.hh index dcfacbb21..054516bc2 100644 --- a/src/libexpr/include/nix/expr/value/context.hh +++ b/src/libexpr/include/nix/expr/value/context.hh @@ -24,6 +24,14 @@ public: } }; +/** + * @todo This should be renamed to `StringContextBuilderElem`, since: + * + * 1. We use `*Builder` for off-heap temporary data structures + * + * 2. The `Nix*` is totally redundant. (And my mistake from a long time + * ago.) + */ struct NixStringContextElem { /** @@ -77,6 +85,11 @@ struct NixStringContextElem std::string to_string() const; }; +/** + * @todo This should be renamed to `StringContextBuilder`. + * + * @see NixStringContextElem for explanation why. + */ typedef std::set NixStringContext; } // namespace nix