Use StringData for Nix language value documentation too

This probably isn't much of a perf gain, but it does mean we are
completely rid of non-length prefixed strings. That allows us to delete
some code.

src/libcmd/repl.cc

@@ -656,7 +656,7 @@ ProcessLineResult NixRepl::processLine(std::string line)
                             + "\n\n";
             }
 
-            markdown += stripIndentation(doc->doc);
+            markdown += stripIndentation(doc->doc.view());
 
             logger->cout(trim(renderMarkdownToTerminal(markdown)));
         } else if (fallbackPos) {

src/libexpr-c/nix_api_value.cc

@@ -134,7 +134,7 @@ PrimOp * nix_alloc_primop(
                     .name = name,
                     .args = {},
                     .arity = (size_t) arity,
-                    .doc = doc,
+                    .doc = &nix::StringData::make(doc),
                     .fun = std::bind(nix_c_primop_wrapper, fun, user_data, _1, _2, _3, _4)};
         if (args)
             for (size_t i = 0; args[i]; i++)

src/libexpr/eval.cc

@@ -51,36 +51,6 @@ using json = nlohmann::json;
 
 namespace nix {
 
-/**
- * Just for doc strings. Not for regular string values.
- */
-static char * allocString(size_t size)
-{
-    char * t;
-    t = (char *) GC_MALLOC_ATOMIC(size);
-    if (!t)
-        throw std::bad_alloc();
-    return t;
-}
-
-// When there's no need to write to the string, we can optimize away empty
-// string allocations.
-// This function handles makeImmutableString(std::string_view()) by returning
-// the empty string.
-/**
- * Just for doc strings. Not for regular string values.
- */
-static const char * makeImmutableString(std::string_view s)
-{
-    const size_t size = s.size();
-    if (size == 0)
-        return "";
-    auto t = allocString(size + 1);
-    memcpy(t, s.data(), size);
-    t[size] = '\0';
-    return t;
-}
-
 StringData & StringData::alloc(size_t size)
 {
     void * t = GC_MALLOC_ATOMIC(sizeof(StringData) + size + 1);
@@ -571,7 +541,7 @@ std::optional<EvalState::Doc> EvalState::getDoc(Value & v)
                 .name = v2->primOp()->name,
                 .arity = v2->primOp()->arity,
                 .args = v2->primOp()->args,
-                .doc = doc,
+                .doc = *doc,
             };
     }
     if (v.isLambda()) {
@@ -613,9 +583,8 @@ std::optional<EvalState::Doc> EvalState::getDoc(Value & v)
             .name = name,
             .arity = 0, // FIXME: figure out how deep by syntax only? It's not semantically useful though...
             .args = {},
-            /* N.B. Can't use StringData here, because that would lead to an interior pointer.
+            /* NOTE: memory leak when compiled without GC. */
-               NOTE: memory leak when compiled without GC. */
+            .doc = StringData::make(s.view()),
-            .doc = makeImmutableString(s.view()),
         };
     }
     if (isFunctor(v)) {

src/libexpr/include/nix/expr/eval.hh

@@ -108,7 +108,7 @@ struct PrimOp
     /**
      * Optional free-form documentation about the primop.
      */
-    const char * doc = nullptr;
+    const StringData * doc = nullptr;
 
     /**
      * Add a trace item, while calling the `<name>` builtin.
@@ -156,7 +156,7 @@ struct Constant
     /**
      * Optional free-form documentation about the constant.
      */
-    const char * doc = nullptr;
+    const StringData * doc = nullptr;
 
     /**
      * Whether the constant is impure, and not available in pure mode.
@@ -836,11 +836,7 @@ public:
         std::optional<std::string> name;
         size_t arity;
         std::vector<std::string> args;
-        /**
+        const StringData & doc;
-         * Unlike the other `doc` fields in this file, this one should never be
-         * `null`.
-         */
-        const char * doc;
     };
 
     /**

src/libexpr/primops/context.cc

@@ -1,5 +1,6 @@
 #include "nix/expr/primops.hh"
 #include "nix/expr/eval-inline.hh"
+#include "nix/expr/static-string-data.hh"
 #include "nix/store/derivations.hh"
 #include "nix/store/store-api.hh"
 #include "nix/store/globals.hh"
@@ -17,9 +18,9 @@ static void prim_unsafeDiscardStringContext(EvalState & state, const PosIdx pos,
 static RegisterPrimOp primop_unsafeDiscardStringContext({
     .name = "__unsafeDiscardStringContext",
     .args = {"s"},
-    .doc = R"(
+    .doc = &R"(
         Discard the [string context](@docroot@/language/string-context.md) from a value that can be coerced to a string.
-    )",
+    )"_sds,
     .fun = prim_unsafeDiscardStringContext,
 });
 
@@ -33,7 +34,7 @@ static void prim_hasContext(EvalState & state, const PosIdx pos, Value ** args,
 static RegisterPrimOp primop_hasContext(
     {.name = "__hasContext",
      .args = {"s"},
-     .doc = R"(
+     .doc = &R"(
       Return `true` if string *s* has a non-empty context.
       The context can be obtained with
       [`getContext`](#builtins-getContext).
@@ -50,7 +51,7 @@ static RegisterPrimOp primop_hasContext(
       > then throw "package name cannot contain string context"
       > else { ${name} = meta; }
       > ```
-    )",
+    )"_sds,
      .fun = prim_hasContext});
 
 static void prim_unsafeDiscardOutputDependency(EvalState & state, const PosIdx pos, Value ** args, Value & v)
@@ -75,7 +76,7 @@ static void prim_unsafeDiscardOutputDependency(EvalState & state, const PosIdx p
 static RegisterPrimOp primop_unsafeDiscardOutputDependency(
     {.name = "__unsafeDiscardOutputDependency",
      .args = {"s"},
-     .doc = R"(
+     .doc = &R"(
       Create a copy of the given string where every
       [derivation deep](@docroot@/language/string-context.md#string-context-element-derivation-deep)
       string context element is turned into a
@@ -91,7 +92,7 @@ static RegisterPrimOp primop_unsafeDiscardOutputDependency(
       Replacing a constant string context element with a "derivation deep" element is a safe operation that just enlargens the string context without forgetting anything.
 
       [`builtins.addDrvOutputDependencies`]: #builtins-addDrvOutputDependencies
-    )",
+    )"_sds,
      .fun = prim_unsafeDiscardOutputDependency});
 
 static void prim_addDrvOutputDependencies(EvalState & state, const PosIdx pos, Value ** args, Value & v)
@@ -143,7 +144,7 @@ static void prim_addDrvOutputDependencies(EvalState & state, const PosIdx pos, V
 static RegisterPrimOp primop_addDrvOutputDependencies(
     {.name = "__addDrvOutputDependencies",
      .args = {"s"},
-     .doc = R"(
+     .doc = &R"(
       Create a copy of the given string where a single
       [constant](@docroot@/language/string-context.md#string-context-constant)
       string context element is turned into a
@@ -156,7 +157,7 @@ static RegisterPrimOp primop_addDrvOutputDependencies(
       The latter is supported so this function is idempotent.
 
       This is the opposite of [`builtins.unsafeDiscardOutputDependency`](#builtins-unsafeDiscardOutputDependency).
-    )",
+    )"_sds,
      .fun = prim_addDrvOutputDependencies});
 
 /* Extract the context of a string as a structured Nix value.
@@ -230,7 +231,7 @@ static void prim_getContext(EvalState & state, const PosIdx pos, Value ** args,
 static RegisterPrimOp primop_getContext(
     {.name = "__getContext",
      .args = {"s"},
-     .doc = R"(
+     .doc = &R"(
       Return the string context of *s*.
 
       The string context tracks references to derivations within a string.
@@ -248,7 +249,7 @@ static RegisterPrimOp primop_getContext(
       ```
       { "/nix/store/arhvjaf6zmlyn8vh8fgn55rpwnxq0n7l-a.drv" = { outputs = [ "out" ]; }; }
       ```
-    )",
+    )"_sds,
      .fun = prim_getContext});
 
 /* Append the given context to a given string.

src/libexpr/primops/fetchClosure.cc

@@ -1,4 +1,5 @@
 #include "nix/expr/primops.hh"
+#include "nix/expr/static-string-data.hh"
 #include "nix/store/store-open.hh"
 #include "nix/store/realisation.hh"
 #include "nix/store/make-content-addressed.hh"
@@ -216,7 +217,7 @@ static void prim_fetchClosure(EvalState & state, const PosIdx pos, Value ** args
 static RegisterPrimOp primop_fetchClosure({
     .name = "__fetchClosure",
     .args = {"args"},
-    .doc = R"(
+    .doc = &R"(
       Fetch a store path [closure](@docroot@/glossary.md#gloss-closure) from a binary cache, and return the store path as a string with context.
 
       This function can be invoked in three ways that we will discuss in order of preference.
@@ -284,7 +285,7 @@ static RegisterPrimOp primop_fetchClosure({
       `fetchClosure` is similar to [`builtins.storePath`](#builtins-storePath) in that it allows you to use a previously built store path in a Nix expression.
       However, `fetchClosure` is more reproducible because it specifies a binary cache from which the path can be fetched.
       Also, using content-addressed store paths does not require users to configure [`trusted-public-keys`](@docroot@/command-ref/conf-file.md#conf-trusted-public-keys) to ensure their authenticity.
-    )",
+    )"_sds,
     .fun = prim_fetchClosure,
     .experimentalFeature = Xp::FetchClosure,
 });

src/libexpr/primops/fetchTree.cc

@@ -2,6 +2,7 @@
 #include "nix/expr/primops.hh"
 #include "nix/expr/eval-inline.hh"
 #include "nix/expr/eval-settings.hh"
+#include "nix/expr/static-string-data.hh"
 #include "nix/store/store-api.hh"
 #include "nix/fetchers/fetchers.hh"
 #include "nix/store/filetransfer.hh"
@@ -234,7 +235,7 @@ static void prim_fetchTree(EvalState & state, const PosIdx pos, Value ** args, V
 static RegisterPrimOp primop_fetchTree({
     .name = "fetchTree",
     .args = {"input"},
-    .doc = R"(
+    .doc = &R"(
       Fetch a file system tree or a plain file using one of the supported backends and return an attribute set with:
 
       - the resulting fixed-output [store path](@docroot@/store/store-path.md)
@@ -456,7 +457,7 @@ static RegisterPrimOp primop_fetchTree({
       >   ```nix
       >   builtins.fetchTree "github:NixOS/nixpkgs/ae2e6b3958682513d28f7d633734571fb18285dd"
       >   ```
-    )",
+    )"_sds,
     .fun = prim_fetchTree,
     .experimentalFeature = Xp::FetchTree,
 });
@@ -616,7 +617,7 @@ static void prim_fetchurl(EvalState & state, const PosIdx pos, Value ** args, Va
 static RegisterPrimOp primop_fetchurl({
     .name = "__fetchurl",
     .args = {"arg"},
-    .doc = R"(
+    .doc = &R"(
       Download the specified URL and return the path of the downloaded file.
       `arg` can be either a string denoting the URL, or an attribute set with the following attributes:
 
@@ -630,7 +631,7 @@ static RegisterPrimOp primop_fetchurl({
         characters that are invalid for the store.
 
       Not available in [restricted evaluation mode](@docroot@/command-ref/conf-file.md#conf-restrict-eval).
-    )",
+    )"_sds,
     .fun = prim_fetchurl,
 });
 
@@ -642,7 +643,7 @@ static void prim_fetchTarball(EvalState & state, const PosIdx pos, Value ** args
 static RegisterPrimOp primop_fetchTarball({
     .name = "fetchTarball",
     .args = {"args"},
-    .doc = R"(
+    .doc = &R"(
       Download the specified URL, unpack it and return the path of the
       unpacked tree. The file must be a tape archive (`.tar`) compressed
       with `gzip`, `bzip2` or `xz`. If the tarball consists of a
@@ -680,7 +681,7 @@ static RegisterPrimOp primop_fetchTarball({
       ```
 
       Not available in [restricted evaluation mode](@docroot@/command-ref/conf-file.md#conf-restrict-eval).
-    )",
+    )"_sds,
     .fun = prim_fetchTarball,
 });
 
@@ -693,7 +694,7 @@ static void prim_fetchGit(EvalState & state, const PosIdx pos, Value ** args, Va
 static RegisterPrimOp primop_fetchGit({
     .name = "fetchGit",
     .args = {"args"},
-    .doc = R"(
+    .doc = &R"(
       Fetch a path from git. *args* can be a URL, in which case the HEAD
       of the repo at that URL is fetched. Otherwise, it can be an
       attribute with the following attributes (all except `url` optional):
@@ -896,7 +897,7 @@ static RegisterPrimOp primop_fetchGit({
       given, `fetchGit` uses the current content of the checked-out
       files, even if they are not committed or added to Git's index. It
       only considers files added to the Git repository, as listed by `git ls-files`.
-    )",
+    )"_sds,
     .fun = prim_fetchGit,
 });
 

src/libexpr/primops/fromTOML.cc

@@ -173,7 +173,7 @@ static void prim_fromTOML(EvalState & state, const PosIdx pos, Value ** args, Va
 static RegisterPrimOp primop_fromTOML({
     .name = "fromTOML",
     .args = {"e"},
-    .doc = R"(
+    .doc = &R"(
       Convert a TOML string to a Nix value. For example,
 
       ```nix
@@ -186,7 +186,7 @@ static RegisterPrimOp primop_fromTOML({
       ```
 
       returns the value `{ s = "a"; table = { y = 2; }; x = 1; }`.
-    )",
+    )"_sds,
     .fun = prim_fromTOML,
 });
 

src/libflake/flake-primops.cc

@@ -19,6 +19,7 @@
 #include "nix/expr/eval-settings.hh"
 #include "nix/expr/symbol-table.hh"
 #include "nix/expr/value.hh"
+#include "nix/expr/static-string-data.hh"
 #include "nix/fetchers/attrs.hh"
 #include "nix/fetchers/fetchers.hh"
 #include "nix/util/configuration.hh"
@@ -62,7 +63,7 @@ PrimOp getFlake(const Settings & settings)
     return PrimOp{
         .name = "__getFlake",
         .args = {"args"},
-        .doc = R"(
+        .doc = &R"(
           Fetch a flake from a flake reference, and return its output attributes and some metadata. For example:
 
           ```nix
@@ -76,7 +77,7 @@ PrimOp getFlake(const Settings & settings)
           ```nix
           (builtins.getFlake "github:edolstra/dwarffs").rev
           ```
-        )",
+        )"_sds,
         .fun = prim_getFlake,
         .experimentalFeature = Xp::Flakes,
     };
@@ -104,7 +105,7 @@ static void prim_parseFlakeRef(EvalState & state, const PosIdx pos, Value ** arg
 nix::PrimOp parseFlakeRef({
     .name = "__parseFlakeRef",
     .args = {"flake-ref"},
-    .doc = R"(
+    .doc = &R"(
       Parse a flake reference, and return its exploded form.
 
       For example:
@@ -118,7 +119,7 @@ nix::PrimOp parseFlakeRef({
       ```nix
       { dir = "lib"; owner = "NixOS"; ref = "23.05"; repo = "nixpkgs"; type = "github"; }
       ```
-    )",
+    )"_sds,
     .fun = prim_parseFlakeRef,
     .experimentalFeature = Xp::Flakes,
 });
@@ -162,7 +163,7 @@ static void prim_flakeRefToString(EvalState & state, const PosIdx pos, Value **
 nix::PrimOp flakeRefToString({
     .name = "__flakeRefToString",
     .args = {"attrs"},
-    .doc = R"(
+    .doc = &R"(
       Convert a flake reference from attribute set format to URL format.
 
       For example:
@@ -178,7 +179,7 @@ nix::PrimOp flakeRefToString({
       ```nix
       "github:NixOS/nixpkgs/23.05?dir=lib"
       ```
-    )",
+    )"_sds,
     .fun = prim_flakeRefToString,
     .experimentalFeature = Xp::Flakes,
 });

src/nix/main.cc

@@ -440,7 +440,7 @@ void mainWrapped(int argc, char ** argv)
             if (!primOp->doc)
                 continue;
             b["args"] = primOp->args;
-            b["doc"] = trim(stripIndentation(primOp->doc));
+            b["doc"] = trim(stripIndentation(primOp->doc->view()));
             if (primOp->experimentalFeature)
                 b["experimental-feature"] = primOp->experimentalFeature;
             builtinsJson.emplace(state.symbols[builtin.name], std::move(b));
@@ -449,7 +449,7 @@ void mainWrapped(int argc, char ** argv)
             auto b = nlohmann::json::object();
             if (!info.doc)
                 continue;
-            b["doc"] = trim(stripIndentation(info.doc));
+            b["doc"] = trim(stripIndentation(info.doc->view()));
             b["type"] = showType(info.type, false);
             if (info.impureOnly)
                 b["impure-only"] = true;