Merge pull request #11682 from NaN-git/opt-str

Remove superfluous `std::string` copy operations

src/libcmd/repl.cc

@@ -645,7 +645,7 @@ ProcessLineResult NixRepl::processLine(std::string line)
 
             logger->cout(trim(renderMarkdownToTerminal(markdown)));
         } else if (fallbackPos) {
-            std::stringstream ss;
+            std::ostringstream ss;
             ss << "Attribute `" << fallbackName << "`\n\n";
             ss << "  … defined at " << state->positions[fallbackPos] << "\n\n";
             if (fallbackDoc) {
@@ -654,7 +654,7 @@ ProcessLineResult NixRepl::processLine(std::string line)
                 ss << "No documentation found.\n\n";
             }
 
-            auto markdown = ss.str();
+            auto markdown = toView(ss);
             logger->cout(trim(renderMarkdownToTerminal(markdown)));
 
         } else

src/libexpr/eval.cc

@@ -539,7 +539,7 @@ std::optional<EvalState::Doc> EvalState::getDoc(Value & v)
     if (v.isLambda()) {
         auto exprLambda = v.payload.lambda.fun;
 
-        std::stringstream s(std::ios_base::out);
+        std::ostringstream s;
         std::string name;
         auto pos = positions[exprLambda->getPos()];
         std::string docStr;
@@ -571,17 +571,12 @@ std::optional<EvalState::Doc> EvalState::getDoc(Value & v)
 
         s << docStr;
 
-        s << '\0'; // for making a c string below
-        std::string ss = s.str();
-
         return Doc {
             .pos = pos,
             .name = name,
             .arity = 0, // FIXME: figure out how deep by syntax only? It's not semantically useful though...
             .args = {},
-            .doc =
+            .doc = makeImmutableString(toView(s)), // NOTE: memory leak when compiled without GC
-                // FIXME: this leaks; make the field std::string?
-                strdup(ss.data()),
         };
     }
     if (isFunctor(v)) {
@@ -1805,11 +1800,9 @@ void ExprIf::eval(EvalState & state, Env & env, Value & v)
 void ExprAssert::eval(EvalState & state, Env & env, Value & v)
 {
     if (!state.evalBool(env, cond, pos, "in the condition of the assert statement")) {
-        auto exprStr = ({
+        std::ostringstream out;
-            std::ostringstream out;
+        cond->show(state.symbols, out);
-            cond->show(state.symbols, out);
+        auto exprStr = toView(out);
-            out.str();
-        });
 
         if (auto eq = dynamic_cast<ExprOpEq *>(cond)) {
             try {

src/libexpr/get-drvs.cc

@@ -374,11 +374,12 @@ static void getDerivations(EvalState & state, Value & vIn,
            bound to the attribute with the "lower" name should take
            precedence). */
         for (auto & i : v.attrs()->lexicographicOrder(state.symbols)) {
+            std::string_view symbol{state.symbols[i->name]};
             try {
-                debug("evaluating attribute '%1%'", state.symbols[i->name]);
+                debug("evaluating attribute '%1%'", symbol);
-                if (!std::regex_match(std::string(state.symbols[i->name]), attrRegex))
+                if (!std::regex_match(symbol.begin(), symbol.end(), attrRegex))
                     continue;
-                std::string pathPrefix2 = addToPath(pathPrefix, state.symbols[i->name]);
+                std::string pathPrefix2 = addToPath(pathPrefix, symbol);
                 if (combineChannels)
                     getDerivations(state, *i->value, pathPrefix2, autoArgs, drvs, done, ignoreAssertionFailures);
                 else if (getDerivation(state, *i->value, pathPrefix2, drvs, done, ignoreAssertionFailures)) {
@@ -392,7 +393,7 @@ static void getDerivations(EvalState & state, Value & vIn,
                     }
                 }
             } catch (Error & e) {
-                e.addTrace(state.positions[i->pos], "while evaluating the attribute '%s'", state.symbols[i->name]);
+                e.addTrace(state.positions[i->pos], "while evaluating the attribute '%s'", symbol);
                 throw;
             }
         }

src/libexpr/primops.cc

@@ -40,6 +40,13 @@ namespace nix {
  * Miscellaneous
  *************************************************************/
 
+static inline Value * mkString(EvalState & state, const std::csub_match & match)
+{
+    Value * v = state.allocValue();
+    v->mkString({match.first, match.second});
+    return v;
+}
+
 StringMap EvalState::realiseContext(const NixStringContext & context, StorePathSet * maybePathsOut, bool isIFD)
 {
     std::vector<DerivedPath::Built> drvs;
@@ -2129,7 +2136,7 @@ static void prim_toXML(EvalState & state, const PosIdx pos, Value * * args, Valu
     std::ostringstream out;
     NixStringContext context;
     printValueAsXML(state, true, false, *args[0], out, context, pos);
-    v.mkString(out.str(), context);
+    v.mkString(toView(out), context);
 }
 
 static RegisterPrimOp primop_toXML({
@@ -2237,7 +2244,7 @@ static void prim_toJSON(EvalState & state, const PosIdx pos, Value * * args, Val
     std::ostringstream out;
     NixStringContext context;
     printValueAsJSON(state, true, *args[0], pos, out, context);
-    v.mkString(out.str(), context);
+    v.mkString(toView(out), context);
 }
 
 static RegisterPrimOp primop_toJSON({
@@ -4268,7 +4275,7 @@ void prim_match(EvalState & state, const PosIdx pos, Value * * args, Value & v)
             if (!match[i + 1].matched)
                 v2 = &state.vNull;
             else
-                (v2 = state.allocValue())->mkString(match[i + 1].str());
+                v2 = mkString(state, match[i + 1]);
         v.mkList(list);
 
     } catch (std::regex_error & e) {
@@ -4352,7 +4359,7 @@ void prim_split(EvalState & state, const PosIdx pos, Value * * args, Value & v)
             auto match = *i;
 
             // Add a string for non-matched characters.
-            (list[idx++] = state.allocValue())->mkString(match.prefix().str());
+            list[idx++] = mkString(state, match.prefix());
 
             // Add a list for matched substrings.
             const size_t slen = match.size() - 1;
@@ -4363,14 +4370,14 @@ void prim_split(EvalState & state, const PosIdx pos, Value * * args, Value & v)
                 if (!match[si + 1].matched)
                     v2 = &state.vNull;
                 else
-                    (v2 = state.allocValue())->mkString(match[si + 1].str());
+                    v2 = mkString(state, match[si + 1]);
             }
 
             (list[idx++] = state.allocValue())->mkList(list2);
 
             // Add a string for non-matched suffix characters.
             if (idx == 2 * len)
-                (list[idx++] = state.allocValue())->mkString(match.suffix().str());
+                list[idx++] = mkString(state, match.suffix());
         }
 
         assert(idx == 2 * len + 1);

src/libexpr/primops/fromTOML.cc

@@ -66,7 +66,7 @@ static void prim_fromTOML(EvalState & state, const PosIdx pos, Value * * args, V
                         attrs.alloc("_type").mkString("timestamp");
                         std::ostringstream s;
                         s << t;
-                        attrs.alloc("value").mkString(s.str());
+                        attrs.alloc("value").mkString(toView(s));
                         v.mkAttrs(attrs);
                     } else {
                         throw std::runtime_error("Dates and times are not supported");

src/libexpr/print.cc

@@ -460,7 +460,7 @@ private:
 
                 std::ostringstream s;
                 s << state.positions[v.payload.lambda.fun->pos];
-                output << " @ " << filterANSIEscapes(s.str());
+                output << " @ " << filterANSIEscapes(toView(s));
             }
         } else if (v.isPrimOp()) {
             if (v.primOp())

src/libmain/progress-bar.cc

@@ -158,10 +158,10 @@ public:
     {
         auto state(state_.lock());
 
-        std::stringstream oss;
+        std::ostringstream oss;
         showErrorInfo(oss, ei, loggerSettings.showTrace.get());
 
-        log(*state, ei.level, oss.str());
+        log(*state, ei.level, toView(oss));
     }
 
     void log(State & state, Verbosity lvl, std::string_view s)

src/libstore/daemon.cc

@@ -90,11 +90,11 @@ struct TunnelLogger : public Logger
     {
         if (ei.level > verbosity) return;
 
-        std::stringstream oss;
+        std::ostringstream oss;
         showErrorInfo(oss, ei, false);
 
         StringSink buf;
-        buf << STDERR_NEXT << oss.str();
+        buf << STDERR_NEXT << toView(oss);
         enqueueMsg(buf.s);
     }
 

src/libstore/outputs-spec.cc

@@ -30,16 +30,15 @@ std::optional<OutputsSpec> OutputsSpec::parseOpt(std::string_view s)
 {
     static std::regex regex(std::string { outputSpecRegexStr });
 
-    std::smatch match;
+    std::cmatch match;
-    std::string s2 { s }; // until some improves std::regex
+    if (!std::regex_match(s.cbegin(), s.cend(), match, regex))
-    if (!std::regex_match(s2, match, regex))
         return std::nullopt;
 
     if (match[1].matched)
         return { OutputsSpec::All {} };
 
     if (match[2].matched)
-        return OutputsSpec::Names { tokenizeString<StringSet>(match[2].str(), ",") };
+        return OutputsSpec::Names { tokenizeString<StringSet>({match[2].first, match[2].second}, ",") };
 
     assert(false);
 }

src/libutil/args.cc

@@ -293,7 +293,7 @@ void RootArgs::parseCmdline(const Strings & _cmdline, bool allowShebang)
                     // We match one space after `nix` so that we preserve indentation.
                     // No space is necessary for an empty line. An empty line has basically no effect.
                     if (std::regex_match(line, match, std::regex("^#!\\s*nix(:? |$)(.*)$")))
-                        shebangContent += match[2].str() + "\n";
+                        shebangContent += std::string_view{match[2].first, match[2].second} + "\n";
                 }
                 for (const auto & word : parseShebangContent(shebangContent)) {
                     cmdline.push_back(word);

src/libutil/logging.cc

@@ -85,10 +85,10 @@ public:
 
     void logEI(const ErrorInfo & ei) override
     {
-        std::stringstream oss;
+        std::ostringstream oss;
         showErrorInfo(oss, ei, loggerSettings.showTrace.get());
 
-        log(ei.level, oss.str());
+        log(ei.level, toView(oss));
     }
 
     void startActivity(ActivityId act, Verbosity lvl, ActivityType type,

src/libutil/strings.cc

@@ -6,6 +6,21 @@
 
 namespace nix {
 
+struct view_stringbuf : public std::stringbuf
+{
+    inline std::string_view toView()
+    {
+        auto begin = pbase();
+        return {begin, begin + pubseekoff(0, std::ios_base::cur, std::ios_base::out)};
+    }
+};
+
+std::string_view toView(const std::ostringstream & os)
+{
+    auto buf = static_cast<view_stringbuf *>(os.rdbuf());
+    return buf->toView();
+}
+
 template std::list<std::string> tokenizeString(std::string_view s, std::string_view separators);
 template std::set<std::string> tokenizeString(std::string_view s, std::string_view separators);
 template std::vector<std::string> tokenizeString(std::string_view s, std::string_view separators);

src/libutil/strings.hh

@@ -8,6 +8,11 @@
 
 namespace nix {
 
+/*
+ * workaround for unavailable view() method (C++20) of std::ostringstream under MacOS with clang-16
+ */
+std::string_view toView(const std::ostringstream & os);
+
 /**
  * String tokenizer.
  *

src/nix-build/nix-build.cc

@@ -36,7 +36,7 @@ extern char * * environ __attribute__((weak));
 /* Recreate the effect of the perl shellwords function, breaking up a
  * string into arguments like a shell word, including escapes
  */
-static std::vector<std::string> shellwords(const std::string & s)
+static std::vector<std::string> shellwords(std::string_view s)
 {
     std::regex whitespace("^\\s+");
     auto begin = s.cbegin();
@@ -51,7 +51,7 @@ static std::vector<std::string> shellwords(const std::string & s)
     auto it = begin;
     for (; it != s.cend(); ++it) {
         if (st == sBegin) {
-            std::smatch match;
+            std::cmatch match;
             if (regex_search(it, s.cend(), match, whitespace)) {
                 cur.append(begin, it);
                 res.push_back(cur);
@@ -173,7 +173,7 @@ static void main_nix_build(int argc, char * * argv)
                     line = chomp(line);
                     std::smatch match;
                     if (std::regex_match(line, match, std::regex("^#!\\s*nix-shell\\s+(.*)$")))
-                        for (const auto & word : shellwords(match[1].str()))
+                        for (const auto & word : shellwords({match[1].first, match[1].second}))
                             args.push_back(word);
                 }
             }
@@ -260,9 +260,9 @@ static void main_nix_build(int argc, char * * argv)
                 // read the shebang to understand which packages to read from. Since
                 // this is handled via nix-shell -p, we wrap our ruby script execution
                 // in ruby -e 'load' which ignores the shebangs.
-                envCommand = fmt("exec %1% %2% -e 'load(ARGV.shift)' -- %3% %4%", execArgs, interpreter, shellEscape(script), joined.str());
+                envCommand = fmt("exec %1% %2% -e 'load(ARGV.shift)' -- %3% %4%", execArgs, interpreter, shellEscape(script), toView(joined));
             } else {
-                envCommand = fmt("exec %1% %2% %3% %4%", execArgs, interpreter, shellEscape(script), joined.str());
+                envCommand = fmt("exec %1% %2% %3% %4%", execArgs, interpreter, shellEscape(script), toView(joined));
             }
         }
 

src/nix-env/user-env.cc

@@ -111,9 +111,7 @@ bool createUserEnv(EvalState & state, PackageInfos & elems,
     auto manifestFile = ({
         std::ostringstream str;
         printAmbiguous(manifest, state.symbols, str, nullptr, std::numeric_limits<int>::max());
-        // TODO with C++20 we can use str.view() instead and avoid copy.
+        StringSource source { toView(str) };
-        std::string str2 = str.str();
-        StringSource source { str2 };
         state.store->addToStoreFromDump(
             source, "env-manifest.nix", FileSerialisationMethod::Flat, ContentAddressMethod::Raw::Text, HashAlgorithm::SHA256, references);
     });

src/nix/config-check.cc

@@ -26,17 +26,17 @@ std::string formatProtocol(unsigned int proto)
     return "unknown";
 }
 
-bool checkPass(const std::string & msg) {
+bool checkPass(std::string_view msg) {
     notice(ANSI_GREEN "[PASS] " ANSI_NORMAL + msg);
     return true;
 }
 
-bool checkFail(const std::string & msg) {
+bool checkFail(std::string_view msg) {
     notice(ANSI_RED "[FAIL] " ANSI_NORMAL + msg);
     return false;
 }
 
-void checkInfo(const std::string & msg) {
+void checkInfo(std::string_view msg) {
     notice(ANSI_BLUE "[INFO] " ANSI_NORMAL + msg);
 }
 
@@ -87,11 +87,11 @@ struct CmdConfigCheck : StoreCommand
         }
 
         if (dirs.size() != 1) {
-            std::stringstream ss;
+            std::ostringstream ss;
             ss << "Multiple versions of nix found in PATH:\n";
             for (auto & dir : dirs)
                 ss << "  " << dir << "\n";
-            return checkFail(ss.str());
+            return checkFail(toView(ss));
         }
 
         return checkPass("PATH contains only one nix version.");
@@ -125,14 +125,14 @@ struct CmdConfigCheck : StoreCommand
         }
 
         if (!dirs.empty()) {
-            std::stringstream ss;
+            std::ostringstream ss;
             ss << "Found profiles outside of " << settings.nixStateDir << "/profiles.\n"
                << "The generation this profile points to might not have a gcroot and could be\n"
                << "garbage collected, resulting in broken symlinks.\n\n";
             for (auto & dir : dirs)
                 ss << "  " << dir << "\n";
             ss << "\n";
-            return checkFail(ss.str());
+            return checkFail(toView(ss));
         }
 
         return checkPass("All profiles are gcroots.");
@@ -145,13 +145,13 @@ struct CmdConfigCheck : StoreCommand
             : PROTOCOL_VERSION;
 
         if (clientProto != storeProto) {
-            std::stringstream ss;
+            std::ostringstream ss;
             ss << "Warning: protocol version of this client does not match the store.\n"
                << "While this is not necessarily a problem it's recommended to keep the client in\n"
                << "sync with the daemon.\n\n"
                << "Client protocol: " << formatProtocol(clientProto) << "\n"
                << "Store protocol: " << formatProtocol(storeProto) << "\n\n";
-            return checkFail(ss.str());
+            return checkFail(toView(ss));
         }
 
         return checkPass("Client protocol matches store protocol.");