Improve pointer stability of OutputFile objects.

Due to C++'s "Small string optimization", a moved-from string
invalidates any string_views pointing to it. Hence, calling append on a vector
of OutputFiles invalidates all string_views.

By changing this to a vector<char>, we disable small string
optimization, which wasn't really helping us anyway, since OutputFiles
almost universally are not small enough to fit into 23 bytes.

This results in a very minor ~2% performance improvement when
benchmarking running `gn gen` on chrome, though this could be statistically
insignificant - the main reason to do this is the pointer stability.

Bug: None
Change-Id: I6708dba71db71af6e6d2542e6292a90a6a6a6964
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/23180
Commit-Queue: Matt Stark <msta@google.com>
Reviewed-by: Takuto Ikuta <tikuta@google.com>
diff --git a/src/gn/command_outputs.cc b/src/gn/command_outputs.cc
index 90d1b1e..a1db939 100644
--- a/src/gn/command_outputs.cc
+++ b/src/gn/command_outputs.cc
@@ -147,8 +147,10 @@
   }
 
   // Print.
-  for (const OutputFile& output_file : outputs)
-    printf("%s\n", output_file.value().c_str());
+  for (const OutputFile& output_file : outputs) {
+    OutputString(output_file.value());
+    OutputString("\n");
+  }
   return 0;
 }
 
diff --git a/src/gn/desc_builder.cc b/src/gn/desc_builder.cc
index 1823520..143726d 100644
--- a/src/gn/desc_builder.cc
+++ b/src/gn/desc_builder.cc
@@ -785,7 +785,7 @@
         }
       }
 
-      res->AppendString(str + pair.first.value());
+      res->AppendString(str + std::string(pair.first.value()));
     }
 
     return res;
diff --git a/src/gn/filesystem_utils.cc b/src/gn/filesystem_utils.cc
index ec1d093..e7244e3 100644
--- a/src/gn/filesystem_utils.cc
+++ b/src/gn/filesystem_utils.cc
@@ -181,11 +181,10 @@
   const std::string& build_dir = build_settings->build_dir().value();
 
   if (source_dir.value().starts_with(build_dir)) {
-    size_t build_dir_size = build_dir.size();
-    result->value().append(&source_dir.value()[build_dir_size],
-                           source_dir.value().size() - build_dir_size);
+    result->append(
+        std::string_view(source_dir.value()).substr(build_dir.size()));
   } else {
-    result->value().append("ABS_PATH");
+    result->append("ABS_PATH");
 #if defined(OS_WIN)
     // Windows absolute path contains ':' after drive letter. Remove it to
     // avoid inserting ':' in the middle of path (eg. "ABS_PATH/C:/").
@@ -196,7 +195,7 @@
 #else
     const std::string& src_dir_value = source_dir.value();
 #endif
-    result->value().append(src_dir_value);
+    result->append(src_dir_value);
   }
 }
 
@@ -230,8 +229,8 @@
 #endif
 }
 
-size_t FindExtensionOffset(const std::string& path) {
-  for (int i = static_cast<int>(path.size()); i >= 0; i--) {
+size_t FindExtensionOffset(std::string_view path) {
+  for (int i = static_cast<int>(path.size()) - 1; i >= 0; i--) {
     if (IsSlash(path[i]))
       break;
     if (path[i] == '.')
@@ -1046,11 +1045,11 @@
   DCHECK(result.value().empty() || result.value().back() == '/');
 
   if (type == BuildDirType::GEN)
-    result.value().append("gen/");
+    result.append("gen/");
   else if (type == BuildDirType::OBJ)
-    result.value().append("obj/");
+    result.append("obj/");
   else if (type == BuildDirType::PHONY)
-    result.value().append("phony/");
+    result.append("phony/");
   return result;
 }
 
@@ -1078,14 +1077,14 @@
       // it with `BUILD_DIR`. This will create results like `obj/BUILD_DIR/gen`
       // or `toolchain2/obj/BUILD_DIR/toolchain1/gen` which look surprising,
       // but guarantee unicity.
-      result.value().append("BUILD_DIR/");
-      result.value().append(source_dir_path.substr(build_dir.size()));
+      result.append("BUILD_DIR/");
+      result.append(source_dir_path.substr(build_dir.size()));
 
     } else {
       // The source dir is source-absolute, so we trim off the two leading
       // slashes to append to the toolchain object directory.
-      result.value().append(&source_dir.value()[2],
-                            source_dir.value().size() - 2);
+      result.append(std::string_view(&source_dir.value()[2],
+                                     source_dir.value().size() - 2));
     }
   } else {
     // System-absolute.
diff --git a/src/gn/filesystem_utils.h b/src/gn/filesystem_utils.h
index 590f9e3..091a37c 100644
--- a/src/gn/filesystem_utils.h
+++ b/src/gn/filesystem_utils.h
@@ -27,7 +27,7 @@
 // Returns the index of the extension (character after the last dot not after a
 // slash). Returns std::string::npos if not found. Returns path.size() if the
 // file ends with a dot.
-size_t FindExtensionOffset(const std::string& path);
+size_t FindExtensionOffset(std::string_view path);
 
 // Returns a string piece pointing into the input string identifying the
 // extension. Note that the input pointer must outlive the output.
diff --git a/src/gn/ninja_binary_target_writer.cc b/src/gn/ninja_binary_target_writer.cc
index b075df3..c4bbd2b 100644
--- a/src/gn/ninja_binary_target_writer.cc
+++ b/src/gn/ninja_binary_target_writer.cc
@@ -198,15 +198,15 @@
     CHECK(!inputs.empty());
     stamp_or_phony =
         GetBuildDirForTargetAsOutputFile(target_, BuildDirType::PHONY);
-    stamp_or_phony.value().append(target_->label().name());
-    stamp_or_phony.value().append(".inputs");
+    stamp_or_phony.append(target_->label().name());
+    stamp_or_phony.append(".inputs");
     tool = BuiltinTool::kBuiltinToolPhony;
   } else {
     // Make a stamp target.
     stamp_or_phony =
         GetBuildDirForTargetAsOutputFile(target_, BuildDirType::OBJ);
-    stamp_or_phony.value().append(target_->label().name());
-    stamp_or_phony.value().append(".inputs.stamp");
+    stamp_or_phony.append(target_->label().name());
+    stamp_or_phony.append(".inputs.stamp");
     tool = GetNinjaRulePrefixForToolchain(settings_) +
            GeneralTool::kGeneralToolStamp;
   }
diff --git a/src/gn/ninja_build_writer.cc b/src/gn/ninja_build_writer.cc
index d8d7c39..12e323f 100644
--- a/src/gn/ninja_build_writer.cc
+++ b/src/gn/ninja_build_writer.cc
@@ -161,7 +161,7 @@
 
   Err result(matches[0]->defined_from(), "Duplicate output file.",
              "Two or more targets generate the same output:\n  " +
-                 bad_output.value() +
+                 std::string(bad_output.value()) +
                  "\n\n"
                  "This is can often be fixed by changing one of the target "
                  "names, or by \n"
diff --git a/src/gn/ninja_c_binary_target_writer.cc b/src/gn/ninja_c_binary_target_writer.cc
index 812803e..6d870da 100644
--- a/src/gn/ninja_c_binary_target_writer.cc
+++ b/src/gn/ninja_c_binary_target_writer.cc
@@ -431,7 +431,7 @@
       const CTool* tool = target_->toolchain()->GetToolAsC(tool_name);
       if (tool->precompiled_header_type() != CTool::PCH_NONE) {
         for (const auto& dep : pch_deps) {
-          const std::string& output_value = dep.value();
+          std::string_view output_value = dep.value();
           size_t extension_offset = FindExtensionOffset(output_value);
           if (extension_offset == std::string::npos)
             continue;
@@ -565,7 +565,7 @@
   }
 
   OutputFile link_phony = target_->dependency_output();
-  link_phony.value().append(".linkdeps");
+  link_phony.append(".linkdeps");
 
   out_ << "build ";
   path_output_.WriteFile(out_, link_phony);
@@ -814,14 +814,14 @@
 
 bool NinjaCBinaryTargetWriter::CheckForDuplicateObjectFiles(
     const std::vector<OutputFile>& files) const {
-  std::set<std::string> set;
+  std::set<std::string_view> set;
   for (const auto& file : files) {
     if (!set.insert(file.value()).second) {
       Err err(
           target_->defined_from(), "Duplicate object file",
           "The target " + target_->label().GetUserVisibleName(false) +
               "\ngenerates two object files with the same name:\n  " +
-              file.value() +
+              std::string(file.value()) +
               "\n"
               "\n"
               "It could be you accidentally have a file listed twice in the\n"
diff --git a/src/gn/ninja_create_bundle_target_writer.cc b/src/gn/ninja_create_bundle_target_writer.cc
index ca305e6..5b9e733 100644
--- a/src/gn/ninja_create_bundle_target_writer.cc
+++ b/src/gn/ninja_create_bundle_target_writer.cc
@@ -348,15 +348,14 @@
   if (settings_->build_settings()->no_stamp_files()) {
     xcassets_input_stamp_or_phony =
         GetBuildDirForTargetAsOutputFile(target_, BuildDirType::PHONY);
-
-    xcassets_input_stamp_or_phony.value().append(target_->label().name());
-    xcassets_input_stamp_or_phony.value().append(".xcassets.inputdeps");
+    xcassets_input_stamp_or_phony.append(target_->label().name());
+    xcassets_input_stamp_or_phony.append(".xcassets.inputdeps");
     tool = BuiltinTool::kBuiltinToolPhony;
   } else {
     xcassets_input_stamp_or_phony =
         GetBuildDirForTargetAsOutputFile(target_, BuildDirType::OBJ);
-    xcassets_input_stamp_or_phony.value().append(target_->label().name());
-    xcassets_input_stamp_or_phony.value().append(".xcassets.inputdeps.stamp");
+    xcassets_input_stamp_or_phony.append(target_->label().name());
+    xcassets_input_stamp_or_phony.append(".xcassets.inputdeps.stamp");
     tool = GetNinjaRulePrefixForToolchain(settings_) +
            GeneralTool::kGeneralToolStamp;
   }
@@ -436,15 +435,15 @@
     // as those would have been peeled off already.
     stamp_or_phony =
         GetBuildDirForTargetAsOutputFile(target_, BuildDirType::PHONY);
-    stamp_or_phony.value().append(target_->label().name());
-    stamp_or_phony.value().append(".postprocessing.inputdeps");
+    stamp_or_phony.append(target_->label().name());
+    stamp_or_phony.append(".postprocessing.inputdeps");
     tool = BuiltinTool::kBuiltinToolPhony;
   } else {
     // Make a stamp target.
     stamp_or_phony =
         GetBuildDirForTargetAsOutputFile(target_, BuildDirType::OBJ);
-    stamp_or_phony.value().append(target_->label().name());
-    stamp_or_phony.value().append(".postprocessing.inputdeps.stamp");
+    stamp_or_phony.append(target_->label().name());
+    stamp_or_phony.append(".postprocessing.inputdeps.stamp");
     tool = GetNinjaRulePrefixForToolchain(settings_) +
            GeneralTool::kGeneralToolStamp;
   }
diff --git a/src/gn/ninja_target_command_util.cc b/src/gn/ninja_target_command_util.cc
index a691ae3..928851b 100644
--- a/src/gn/ninja_target_command_util.cc
+++ b/src/gn/ninja_target_command_util.cc
@@ -33,10 +33,10 @@
   // Use "obj/{dir}/{target_name}_{lang}.pch" which ends up
   // looking like "obj/chrome/browser/browser_cc.pch"
   OutputFile ret = GetBuildDirForTargetAsOutputFile(target, BuildDirType::OBJ);
-  ret.value().append(target->label().name());
-  ret.value().push_back('_');
-  ret.value().append(GetPCHLangSuffixForToolType(tool_name));
-  ret.value().append(".pch");
+  ret.append(target->label().name());
+  ret.append("_");
+  ret.append(GetPCHLangSuffixForToolType(tool_name));
+  ret.append(".pch");
 
   return ret;
 }
@@ -89,9 +89,8 @@
         // Trim the .gch suffix for the -include flag.
         // e.g. for gch file foo/bar/target.precompiled.h.gch:
         //          -include foo/bar/target.precompiled.h
-        std::string pch_file = outputs[0].value();
-        pch_file.erase(pch_file.length() - 4);
-        out << " -include " << pch_file;
+        std::string_view pch_file = outputs[0].value();
+        out << " -include " << pch_file.substr(0, pch_file.length() - 4);
       }
     } else {
       RecursiveTargetConfigStringsToStream(config, target, getter,
@@ -126,21 +125,24 @@
   if (outputs->size() > 1)
     outputs->resize(1);  // Only link the first output from the compiler tool.
 
-  std::string& output_value = (*outputs)[0].value();
-  size_t extension_offset = FindExtensionOffset(output_value);
+  OutputFile& output_value = (*outputs)[0];
+  std::string_view output_value_str = output_value.value();
+  size_t extension_offset = FindExtensionOffset(output_value_str);
   if (extension_offset == std::string::npos) {
     // No extension found.
     return;
   }
   DCHECK(extension_offset >= 1);
-  DCHECK(output_value[extension_offset - 1] == '.');
+  // Trim the "."
+  extension_offset--;
+  DCHECK(output_value_str[extension_offset] == '.');
 
   std::string output_extension;
   CTool::PrecompiledHeaderType header_type = tool->precompiled_header_type();
   switch (header_type) {
     case CTool::PCH_MSVC:
       output_extension = GetWindowsPCHObjectExtension(
-          tool_name, output_value.substr(extension_offset - 1));
+          tool_name, output_value_str.substr(extension_offset));
       break;
     case CTool::PCH_GCC:
       output_extension = GetGCCPCHOutputExtension(tool_name);
@@ -149,8 +151,8 @@
       NOTREACHED() << "No outputs for no PCH type.";
       break;
   }
-  output_value.replace(extension_offset - 1, std::string::npos,
-                       output_extension);
+  output_value.resize(extension_offset);
+  output_value.append(output_extension);
 }
 
 std::string GetGCCPCHOutputExtension(const char* tool_name) {
@@ -169,7 +171,7 @@
 }
 
 std::string GetWindowsPCHObjectExtension(const char* tool_name,
-                                         const std::string& obj_extension) {
+                                         std::string_view obj_extension) {
   const char* lang_suffix = GetPCHLangSuffixForToolType(tool_name);
   std::string result = ".";
   // For MSVC, annotate the obj files with the language type. For example:
diff --git a/src/gn/ninja_target_command_util.h b/src/gn/ninja_target_command_util.h
index 1ec2e33..09b78c6 100644
--- a/src/gn/ninja_target_command_util.h
+++ b/src/gn/ninja_target_command_util.h
@@ -130,6 +130,6 @@
 
 std::string GetGCCPCHOutputExtension(const char* tool_name);
 std::string GetWindowsPCHObjectExtension(const char* tool_name,
-                                         const std::string& obj_extension);
+                                         std::string_view obj_extension);
 
 #endif  // TOOLS_GN_NINJA_TARGET_COMMAND_WRITER_H_
diff --git a/src/gn/ninja_target_writer.cc b/src/gn/ninja_target_writer.cc
index affc7d6..825439a 100644
--- a/src/gn/ninja_target_writer.cc
+++ b/src/gn/ninja_target_writer.cc
@@ -585,15 +585,15 @@
     // as we would return early if there were no inputs.
     input_stamp_or_phony =
         GetBuildDirForTargetAsOutputFile(target_, BuildDirType::PHONY);
-    input_stamp_or_phony.value().append(target_->label().name());
-    input_stamp_or_phony.value().append(".inputdeps");
+    input_stamp_or_phony.append(target_->label().name());
+    input_stamp_or_phony.append(".inputdeps");
     tool = BuiltinTool::kBuiltinToolPhony;
   } else {
     // Make a stamp file.
     input_stamp_or_phony =
         GetBuildDirForTargetAsOutputFile(target_, BuildDirType::OBJ);
-    input_stamp_or_phony.value().append(target_->label().name());
-    input_stamp_or_phony.value().append(".inputdeps.stamp");
+    input_stamp_or_phony.append(target_->label().name());
+    input_stamp_or_phony.append(".inputdeps.stamp");
 
     tool = GetNinjaRulePrefixForToolchain(settings_) +
            GeneralTool::kGeneralToolStamp;
diff --git a/src/gn/output_file.cc b/src/gn/output_file.cc
index ad25afa..6028ad1 100644
--- a/src/gn/output_file.cc
+++ b/src/gn/output_file.cc
@@ -7,33 +7,35 @@
 #include "gn/filesystem_utils.h"
 #include "gn/source_file.h"
 
-OutputFile::OutputFile(std::string&& v) : value_(std::move(v)) {}
-
-OutputFile::OutputFile(const std::string& v) : value_(v) {}
-
 OutputFile::OutputFile(const BuildSettings* build_settings,
-                       const SourceFile& source_file)
-    : value_(RebasePath(source_file.value(),
-                        build_settings->build_dir(),
-                        build_settings->root_path_utf8())) {}
+                       const SourceFile& source_file) {
+  std::string rebased =
+      RebasePath(source_file.value(), build_settings->build_dir(),
+                 build_settings->root_path_utf8());
+  if (!rebased.empty()) {
+    value_.assign(rebased.begin(), rebased.end());
+  }
+}
 
 SourceFile OutputFile::AsSourceFile(const BuildSettings* build_settings) const {
-  DCHECK(!value_.empty());
-  DCHECK(value_[value_.size() - 1] != '/');
+  std::string_view val = value();
+  DCHECK(!val.empty());
+  DCHECK(val.back() != '/');
 
   std::string path = build_settings->build_dir().value();
-  path.append(value_);
+  path.append(val);
   return SourceFile(std::move(path));
 }
 
 SourceDir OutputFile::AsSourceDir(const BuildSettings* build_settings) const {
-  if (!value_.empty()) {
+  std::string_view val = value();
+  if (!val.empty()) {
     // Empty means the root build dir. Otherwise, we expect it to end in a
     // slash.
-    DCHECK(value_[value_.size() - 1] == '/');
+    DCHECK(val.back() == '/');
   }
   std::string path = build_settings->build_dir().value();
-  path.append(value_);
+  path.append(val);
   NormalizePath(&path);
   return SourceDir(std::move(path));
 }
diff --git a/src/gn/output_file.h b/src/gn/output_file.h
index e3e79a8..9ae8cdf 100644
--- a/src/gn/output_file.h
+++ b/src/gn/output_file.h
@@ -7,26 +7,33 @@
 
 #include <stddef.h>
 
-#include <string>
+#include <string_view>
+#include <vector>
 
 class BuildSettings;
 class SourceDir;
 class SourceFile;
 
-// A simple wrapper around a string that indicates the string is a path
+// A simple wrapper around a vector of chars that indicates the path
 // relative to the output directory.
 class OutputFile {
  public:
   OutputFile() = default;
 
-  explicit OutputFile(std::string&& v);
-  explicit OutputFile(const std::string& v);
+  explicit OutputFile(std::string_view v) { value_.assign(v.begin(), v.end()); }
 
   OutputFile(const BuildSettings* build_settings,
              const SourceFile& source_file);
 
-  std::string& value() { return value_; }
-  const std::string& value() const { return value_; }
+  std::string_view value() const {
+    return std::string_view(value_.data(), value_.size());
+  }
+
+  void resize(std::size_t n) { value_.resize(n); }
+
+  void append(std::string_view v) {
+    value_.insert(value_.end(), v.begin(), v.end());
+  }
 
   // Converts to a SourceFile by prepending the build directory to the file.
   // The *Dir version requires that the current OutputFile ends in a slash, and
@@ -40,7 +47,13 @@
   std::strong_ordering operator<=>(const OutputFile& other) const = default;
 
  private:
-  std::string value_;
+  // Storing this as a vector<char> instead of a std::string has some tradeoffs.
+  // * When OutputFile is moved (eg. vector<OutputFile>.push_back), string_views
+  //   pointing to OutputFile stay valid
+  // * We now lose small string optimization. This is probably fine, and may in
+  //   fact even be an improvement, as OutputFiles are almost universally very
+  //   long and thus this may help with branch prediction.
+  std::vector<char> value_;
 };
 
 namespace std {
@@ -48,8 +61,7 @@
 template <>
 struct hash<OutputFile> {
   std::size_t operator()(const OutputFile& v) const {
-    hash<std::string> h;
-    return h(v.value());
+    return hash<std::string_view>()(v.value());
   }
 };
 
diff --git a/src/gn/resolved_target_data.cc b/src/gn/resolved_target_data.cc
index db5a8ed..bce9839 100644
--- a/src/gn/resolved_target_data.cc
+++ b/src/gn/resolved_target_data.cc
@@ -331,7 +331,7 @@
   } else if (target->has_dependency_output()) {
     OutputFile dep_output = target->dependency_output();
     if (target->output_type() == Target::SOURCE_SET) {
-      dep_output.value().append(".linkdeps");
+      dep_output.append(".linkdeps");
     }
     all_order_only_deps.push_back(dep_output);
   }
diff --git a/src/gn/runtime_deps.cc b/src/gn/runtime_deps.cc
index 26bbda9..1eacee5 100644
--- a/src/gn/runtime_deps.cc
+++ b/src/gn/runtime_deps.cc
@@ -120,7 +120,7 @@
   // The initial target is not considered a data dependency so that actions's
   // outputs (if the current target is an action) are not automatically
   // considered data deps.
-  auto on_file = [&out](const std::string& output_file,
+  auto on_file = [&out](std::string_view output_file,
                         const Target* target) -> void {
     out << output_file << std::endl;
   };
@@ -183,17 +183,19 @@
       // Force the first output for shared-library-type linker outputs since
       // the dependency output files might not be the main output.
       CHECK(!target->computed_outputs().empty());
-      output_file.emplace(target->computed_outputs()[0].value() + extension);
+      output_file.emplace(target->computed_outputs()[0]);
+      output_file->append(extension);
     } else if (target->has_dependency_output_file()) {
-      output_file.emplace(target->dependency_output_file().value() + extension);
+      output_file.emplace(target->dependency_output_file());
+      output_file->append(extension);
     } else {
       // If there is no dependency_output_file, this target's dependency output
       // is either a phony alias or was elided entirely (due to lack of real
       // inputs). In either case, there is no file to add an additional
       // extension to, so we should compute our own name in the OBJ BuildDir.
       output_file = GetBuildDirForTargetAsOutputFile(target, BuildDirType::OBJ);
-      output_file->value().append(target->GetComputedOutputName());
-      output_file->value().append(extension);
+      output_file->append(target->GetComputedOutputName());
+      output_file->append(extension);
     }
     if (output_file)
       files_to_write->emplace_back(*output_file, target);
@@ -292,8 +294,7 @@
   RuntimeDepsVector result;
   std::unordered_map<const Target*, bool> seen_targets;
 
-  auto on_file = [&result](const std::string& output_file,
-                           const Target* target) {
+  auto on_file = [&result](std::string_view output_file, const Target* target) {
     result.emplace_back(OutputFile(output_file), target);
   };
   // The initial target is not considered a data dependency so that actions's
diff --git a/src/gn/runtime_deps_unittest.cc b/src/gn/runtime_deps_unittest.cc
index baa14e4..5c1a281 100644
--- a/src/gn/runtime_deps_unittest.cc
+++ b/src/gn/runtime_deps_unittest.cc
@@ -34,7 +34,7 @@
   for (size_t i = 0; i < v.size(); i++) {
     if (i != 0)
       result.append(", ");
-    result.append("\"" + v[i].first.value() + "\"");
+    result.append("\"" + std::string(v[i].first.value()) + "\"");
   }
   return result;
 }
diff --git a/src/gn/settings.cc b/src/gn/settings.cc
index c9a36c9..173159f 100644
--- a/src/gn/settings.cc
+++ b/src/gn/settings.cc
@@ -16,11 +16,12 @@
   } else {
     // We guarantee this ends in a slash.
     DCHECK(output_subdir_name[output_subdir_name.size() - 1] == '/');
-    toolchain_output_subdir_.value().append(output_subdir_name);
+    toolchain_output_subdir_ = OutputFile(output_subdir_name);
 
     DCHECK(!build_settings->build_dir().is_null());
-    toolchain_output_dir_ = SourceDir(build_settings->build_dir().value() +
-                                      toolchain_output_subdir_.value());
+    std::string dir = build_settings->build_dir().value();
+    dir.append(toolchain_output_subdir_.value());
+    toolchain_output_dir_ = SourceDir(std::move(dir));
   }
   // The output dir will be null in some tests and when invoked to parsed
   // one-off data without doing generation.
diff --git a/src/gn/substitution_writer.cc b/src/gn/substitution_writer.cc
index a1f0254..76a5a5c 100644
--- a/src/gn/substitution_writer.cc
+++ b/src/gn/substitution_writer.cc
@@ -24,11 +24,11 @@
 // slash from the directory (SourceDirs and OutputFiles representing
 // directories will end in a trailing slash). If the directory is empty,
 // it will be replaced with a ".".
-void SetDirOrDotWithNoSlash(const std::string& dir, std::string* dest) {
-  if (!dir.empty() && dir[dir.size() - 1] == '/')
+void SetDirOrDotWithNoSlash(std::string_view dir, std::string* dest) {
+  if (!dir.empty() && dir.back() == '/')
     dest->assign(dir.data(), dir.size() - 1);
   else
-    dest->assign(dir);
+    dest->assign(dir.data(), dir.size());
 
   if (dest->empty())
     dest->push_back('.');
@@ -481,10 +481,9 @@
   OutputFile result;
   for (const auto& subrange : pattern.ranges()) {
     if (subrange.type == &SubstitutionLiteral) {
-      result.value().append(subrange.literal);
+      result.append(subrange.literal);
     } else {
-      result.value().append(
-          GetCompilerSubstitution(target, source, subrange.type));
+      result.append(GetCompilerSubstitution(target, source, subrange.type));
     }
   }
   return result;
@@ -524,9 +523,9 @@
   OutputFile result;
   for (const auto& subrange : pattern.ranges()) {
     if (subrange.type == &SubstitutionLiteral) {
-      result.value().append(subrange.literal);
+      result.append(subrange.literal);
     } else {
-      result.value().append(GetLinkerSubstitution(target, tool, subrange.type));
+      result.append(GetLinkerSubstitution(target, tool, subrange.type));
     }
   }
   return result;
@@ -559,9 +558,9 @@
     // path it wants), or the tool's default (which will contain further
     // expansions).
     if (target->output_dir().is_null()) {
-      return ApplyPatternToLinkerAsOutputFile(target, tool,
-                                              tool->default_output_dir())
-          .value();
+      return std::string(ApplyPatternToLinkerAsOutputFile(
+                             target, tool, tool->default_output_dir())
+                             .value());
     }
     SetDirOrDotWithNoSlash(
         RebasePath(target->output_dir().value(),
diff --git a/src/gn/target.cc b/src/gn/target.cc
index 91cb72c..0048341 100644
--- a/src/gn/target.cc
+++ b/src/gn/target.cc
@@ -980,7 +980,7 @@
         if (HasRealInputs()) {
           dependency_output_alias_ =
               GetBuildDirForTargetAsOutputFile(this, BuildDirType::PHONY);
-          dependency_output_alias_.value().append(label().name());
+          dependency_output_alias_.append(label().name());
         }
       } else {
         // These don't get linked to and use stamps which should be the first
@@ -990,8 +990,8 @@
         // name.
         dependency_output_file_ =
             GetBuildDirForTargetAsOutputFile(this, BuildDirType::OBJ);
-        dependency_output_file_.value().append(label().name());
-        dependency_output_file_.value().append(".stamp");
+        dependency_output_file_.append(label().name());
+        dependency_output_file_.append(".stamp");
       }
       break;
     }
@@ -1096,8 +1096,11 @@
     // {{some_var}}/{{output_name}} which expands to "./foo", but this won't
     // match "foo" which is what we'll compute when converting a SourceFile to
     // an OutputFile.
-    for (auto& out : computed_outputs_)
-      NormalizePath(&out.value());
+    for (auto& out : computed_outputs_) {
+      std::string path(out.value());
+      NormalizePath(&path);
+      out = OutputFile(std::move(path));
+    }
   }
 
   // Also count anything the target has declared to be an output.
diff --git a/src/gn/visual_studio_writer.cc b/src/gn/visual_studio_writer.cc
index 243fedd..d660be8 100644
--- a/src/gn/visual_studio_writer.cc
+++ b/src/gn/visual_studio_writer.cc
@@ -640,11 +640,11 @@
         compile_type = "CustomBuild";
         std::unique_ptr<XmlElementWriter> build = group->SubElement(
             compile_type, "Include", SourceFileWriter(path_output, file));
-        build->SubElement("Command")->Text("call " + ninja_exe +
-                                           " -C $(OutDir) " + ninja_extra_args +
-                                           " " + tool_outputs[0].value());
-        build->SubElement("Outputs")->Text("$(OutDir)" +
-                                           tool_outputs[0].value());
+        build->SubElement("Command")->Text(
+            "call " + ninja_exe + " -C $(OutDir) " + ninja_extra_args + " " +
+            std::string(tool_outputs[0].value()));
+        build->SubElement("Outputs")->Text(
+            "$(OutDir)" + std::string(tool_outputs[0].value()));
       } else {
         compile_type = "None";
         group->SubElement(compile_type, "Include",