Rust: link_output, depend_output and runtime_outputs for dylibs

Ensure that the rust_dylib and rust_cdylib tool() definition
support the `link_output`, `depend_output` and `runtime_outputs`
argument, when generating shared libraries from Rust sources,
just like the `solink` tool used for C++ sources.

Bug: 377
Change-Id: I2286f42661b9ebbff5d5b8455c2be49aaf45afa9
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/17401
Reviewed-by: Takuto Ikuta <tikuta@google.com>
Commit-Queue: David Turner <digit@google.com>
diff --git a/docs/reference.md b/docs/reference.md
index a9fad5b..b99c339 100644
--- a/docs/reference.md
+++ b/docs/reference.md
@@ -3890,13 +3890,13 @@
 
     link_output  [string with substitutions]
     depend_output  [string with substitutions]
-        Valid for: "solink" only (optional)
+        Valid for: "solink", "rust_dylib" or "rust_cdylib" only (optional)
 
-        These two files specify which of the outputs from the solink tool
-        should be used for linking and dependency tracking. These should match
-        entries in the "outputs". If unspecified, the first item in the
-        "outputs" array will be used for all. See "Separate linking and
-        dependencies for shared libraries" below for more.
+        These two files specify which of the outputs from the tool should
+        be used for linking and dependency tracking. These should match entries
+        in the "outputs". If unspecified, the first item in the "outputs" array
+        will be used for all. See "Separate linking and dependencies for shared
+        libraries" below for more.
 
         On Windows, where the tools produce a .dll shared library and a .lib
         import library, you will want the first two to be the import library
@@ -4142,8 +4142,8 @@
     {{solibs}}
         Extra libraries from shared library dependencies not specified in the
         {{inputs}}. This is the list of link_output files from shared libraries
-        (if the solink tool specifies a "link_output" variable separate from
-        the "depend_output").
+        (if the solink, rust_dylib and rust_cdylib tools specify a "link_output"
+        variable separate from the "depend_output").
 
         These should generally be treated the same as libs by your tool.
 
diff --git a/src/gn/c_tool.cc b/src/gn/c_tool.cc
index 767b324..e0bea41 100644
--- a/src/gn/c_tool.cc
+++ b/src/gn/c_tool.cc
@@ -220,8 +220,7 @@
   if (!ValidateLinkAndDependOutput(depend_output(), "depend_output", err)) {
     return false;
   }
-  if ((!link_output().empty() && depend_output().empty()) ||
-      (link_output().empty() && !depend_output().empty())) {
+  if (link_output().empty() != depend_output().empty()) {
     *err = Err(defined_from(),
                "Both link_output and depend_output should either "
                "be specified or they should both be empty.");
diff --git a/src/gn/c_tool.h b/src/gn/c_tool.h
index c1b5883..86a60e8 100644
--- a/src/gn/c_tool.h
+++ b/src/gn/c_tool.h
@@ -83,14 +83,6 @@
     depend_output_ = std::move(dep_out);
   }
 
-  // Other functions ----------------------------------------------------------
-
-  // Returns true if this tool has separate outputs for dependency tracking
-  // and linking.
-  bool has_separate_solink_files() const {
-    return !link_output_.empty() || !depend_output_.empty();
-  }
-
  private:
   // Initialization functions -------------------------------------------------
   //
@@ -100,9 +92,8 @@
   bool ValidateOutputSubstitution(const Substitution* sub_type) const;
   bool ValidateRuntimeOutputs(Err* err);
   // Validates either link_output or depend_output. To generalize to either,
-  // pass
-  // the associated pattern, and the variable name that should appear in error
-  // messages.
+  // pass the associated pattern, and the variable name that should appear in
+  // error messages.
   bool ValidateLinkAndDependOutput(const SubstitutionPattern& pattern,
                                    const char* variable_name,
                                    Err* err);
diff --git a/src/gn/function_toolchain.cc b/src/gn/function_toolchain.cc
index 1df3845..a97a19c 100644
--- a/src/gn/function_toolchain.cc
+++ b/src/gn/function_toolchain.cc
@@ -509,13 +509,13 @@
 
     link_output  [string with substitutions]
     depend_output  [string with substitutions]
-        Valid for: "solink" only (optional)
+        Valid for: "solink", "rust_dylib" or "rust_cdylib" only (optional)
 
-        These two files specify which of the outputs from the solink tool
-        should be used for linking and dependency tracking. These should match
-        entries in the "outputs". If unspecified, the first item in the
-        "outputs" array will be used for all. See "Separate linking and
-        dependencies for shared libraries" below for more.
+        These two files specify which of the outputs from the tool should
+        be used for linking and dependency tracking. These should match entries
+        in the "outputs". If unspecified, the first item in the "outputs" array
+        will be used for all. See "Separate linking and dependencies for shared
+        libraries" below for more.
 
         On Windows, where the tools produce a .dll shared library and a .lib
         import library, you will want the first two to be the import library
@@ -760,8 +760,8 @@
     {{solibs}}
         Extra libraries from shared library dependencies not specified in the
         {{inputs}}. This is the list of link_output files from shared libraries
-        (if the solink tool specifies a "link_output" variable separate from
-        the "depend_output").
+        (if the solink, rust_dylib and rust_cdylib tools specify a "link_output"
+        variable separate from the "depend_output").
 
         These should generally be treated the same as libs by your tool.
 
diff --git a/src/gn/function_toolchain_unittest.cc b/src/gn/function_toolchain_unittest.cc
index 74cbef9..91d2235 100644
--- a/src/gn/function_toolchain_unittest.cc
+++ b/src/gn/function_toolchain_unittest.cc
@@ -117,6 +117,100 @@
   }
 }
 
+TEST_F(FunctionToolchain, RustRuntimeOutputs) {
+  TestWithScope setup;
+
+  // These runtime outputs are a subset of the outputs so are OK.
+  {
+    TestParseInput input(
+        R"(toolchain("good") {
+          tool("rust_dylib") {
+            command = "rust_dylib"
+            outputs = [ "foo" ]
+            runtime_outputs = [ "foo" ]
+          }
+        })");
+    ASSERT_FALSE(input.has_error());
+
+    Err err;
+    input.parsed()->Execute(setup.scope(), &err);
+    ASSERT_FALSE(err.has_error()) << err.message();
+
+    // It should have generated a toolchain.
+    ASSERT_EQ(1u, setup.items().size());
+    const Toolchain* toolchain = setup.items()[0]->AsToolchain();
+    ASSERT_TRUE(toolchain);
+
+    // The toolchain should have a link tool with the two outputs.
+    const Tool* link = toolchain->GetTool(RustTool::kRsToolDylib);
+    ASSERT_TRUE(link);
+    ASSERT_EQ(1u, link->outputs().list().size());
+    EXPECT_EQ("foo", link->outputs().list()[0].AsString());
+    ASSERT_EQ(1u, link->runtime_outputs().list().size());
+    EXPECT_EQ("foo", link->runtime_outputs().list()[0].AsString());
+  }
+
+  // This one is not a subset so should throw an error.
+  {
+    TestParseInput input(
+        R"(toolchain("bad") {
+          tool("rust_dylib") {
+            outputs = [ "foo" ]
+            runtime_outputs = [ "bar" ]
+          }
+        })");
+    ASSERT_FALSE(input.has_error());
+
+    Err err;
+    input.parsed()->Execute(setup.scope(), &err);
+    ASSERT_TRUE(err.has_error()) << err.message();
+  }
+}
+
+TEST_F(FunctionToolchain, RustLinkDependAndRuntimeOutputs) {
+  TestWithScope setup;
+
+  // These runtime outputs are a subset of the outputs so are OK.
+  {
+    TestParseInput input(
+        R"(toolchain("good") {
+          tool("rust_dylib") {
+            command = "rust_dylib"
+            outputs = [ "interface", "lib", "unstripped", "stripped" ]
+            depend_output = "interface"
+            link_output = "lib"
+            runtime_outputs = [ "stripped" ]
+          }
+        })");
+    ASSERT_FALSE(input.has_error());
+
+    Err err;
+    input.parsed()->Execute(setup.scope(), &err);
+    ASSERT_FALSE(err.has_error()) << err.message();
+
+    // It should have generated a toolchain.
+    ASSERT_EQ(1u, setup.items().size());
+    const Toolchain* toolchain = setup.items()[0]->AsToolchain();
+    ASSERT_TRUE(toolchain);
+
+    // The toolchain should have a link tool with the two outputs.
+    const Tool* link = toolchain->GetTool(RustTool::kRsToolDylib);
+    ASSERT_TRUE(link);
+    ASSERT_EQ(4u, link->outputs().list().size());
+    EXPECT_EQ("interface", link->outputs().list()[0].AsString());
+    EXPECT_EQ("lib", link->outputs().list()[1].AsString());
+    EXPECT_EQ("unstripped", link->outputs().list()[2].AsString());
+    EXPECT_EQ("stripped", link->outputs().list()[3].AsString());
+    ASSERT_EQ(1u, link->runtime_outputs().list().size());
+    EXPECT_EQ("stripped", link->runtime_outputs().list()[0].AsString());
+
+    const RustTool* rust_tool = link->AsRust();
+    ASSERT_TRUE(rust_tool);
+    EXPECT_EQ("interface", rust_tool->depend_output().AsString());
+    EXPECT_EQ("lib", rust_tool->link_output().AsString());
+  }
+}
+
 TEST_F(FunctionToolchain, Command) {
   TestWithScope setup;
 
diff --git a/src/gn/rust_tool.cc b/src/gn/rust_tool.cc
index f799d73..1b0898c 100644
--- a/src/gn/rust_tool.cc
+++ b/src/gn/rust_tool.cc
@@ -54,6 +54,56 @@
 
 void RustTool::SetComplete() {
   SetToolComplete();
+  link_output_.FillRequiredTypes(&substitution_bits_);
+  depend_output_.FillRequiredTypes(&substitution_bits_);
+}
+
+bool RustTool::ValidateRuntimeOutputs(Err* err) {
+  if (runtime_outputs().list().empty())
+    return true;  // Empty is always OK.
+
+  if (name_ != kRsToolDylib && name_ != kRsToolCDylib) {
+    *err = Err(
+        defined_from(), "This tool specifies runtime_outputs.",
+        "This is only valid for linker tools (rust_staticlib doesn't count).");
+    return false;
+  }
+
+  for (const SubstitutionPattern& pattern : runtime_outputs().list()) {
+    if (!IsPatternInOutputList(outputs(), pattern)) {
+      *err = Err(defined_from(), "This tool's runtime_outputs is bad.",
+                 "It must be a subset of the outputs. The bad one is:\n  " +
+                     pattern.AsString());
+      return false;
+    }
+  }
+  return true;
+}
+
+// Validates either link_output or depend_output. To generalize to either, pass
+// the associated pattern, and the variable name that should appear in error
+// messages.
+bool RustTool::ValidateLinkAndDependOutput(const SubstitutionPattern& pattern,
+                                           const char* variable_name,
+                                           Err* err) {
+  if (pattern.empty())
+    return true;  // Empty is always OK.
+
+  // It should only be specified for certain tool types.
+  if (name_ != kRsToolDylib && name_ != kRsToolCDylib) {
+    *err = Err(defined_from(),
+               "This tool specifies a " + std::string(variable_name) + ".",
+               "This is only valid for solink and solink_module tools.");
+    return false;
+  }
+
+  if (!IsPatternInOutputList(outputs(), pattern)) {
+    *err = Err(defined_from(), "This tool's link_output is bad.",
+               "It must match one of the outputs.");
+    return false;
+  }
+
+  return true;
 }
 
 std::string_view RustTool::GetSysroot() const {
@@ -131,6 +181,31 @@
         !ReadString(scope, "dynamic_link_switch", &dynamic_link_switch_, err)) {
       return false;
     }
+
+    if (name_ == kRsToolDylib || name_ == kRsToolCDylib) {
+      if (!ReadPattern(scope, "link_output", &link_output_, err) ||
+          !ReadPattern(scope, "depend_output", &depend_output_, err)) {
+        return false;
+      }
+
+      // Validate link_output and depend_output.
+      if (!ValidateLinkAndDependOutput(link_output(), "link_output", err)) {
+        return false;
+      }
+      if (!ValidateLinkAndDependOutput(depend_output(), "depend_output", err)) {
+        return false;
+      }
+      if (link_output().empty() != depend_output().empty()) {
+        *err = Err(defined_from(),
+                   "Both link_output and depend_output should either "
+                   "be specified or they should both be empty.");
+        return false;
+      }
+    }
+
+    if (!ValidateRuntimeOutputs(err)) {
+      return false;
+    }
   }
 
   return true;
diff --git a/src/gn/rust_tool.h b/src/gn/rust_tool.h
index 857e205..76e8bf1 100644
--- a/src/gn/rust_tool.h
+++ b/src/gn/rust_tool.h
@@ -52,6 +52,19 @@
     dynamic_link_switch_ = std::move(s);
   }
 
+  // Should match files in the outputs() if nonempty.
+  const SubstitutionPattern& link_output() const { return link_output_; }
+  void set_link_output(SubstitutionPattern link_out) {
+    DCHECK(!complete_);
+    link_output_ = std::move(link_out);
+  }
+
+  const SubstitutionPattern& depend_output() const { return depend_output_; }
+  void set_depend_output(SubstitutionPattern dep_out) {
+    DCHECK(!complete_);
+    depend_output_ = std::move(dep_out);
+  }
+
  private:
   std::string rust_sysroot_;
   std::string dynamic_link_switch_;
@@ -62,6 +75,22 @@
                               SubstitutionList* field,
                               Err* err);
 
+  // Initialization functions -------------------------------------------------
+  //
+  // Initialization methods used by InitTool(). If successful, will set the
+  // field and return true, otherwise will return false. Must be called before
+  // SetComplete().
+  bool ValidateRuntimeOutputs(Err* err);
+  // Validates either link_output or depend_output. To generalize to either,
+  // pass the associated pattern, and the variable name that should appear in
+  // error messages.
+  bool ValidateLinkAndDependOutput(const SubstitutionPattern& pattern,
+                                   const char* variable_name,
+                                   Err* err);
+
+  SubstitutionPattern link_output_;
+  SubstitutionPattern depend_output_;
+
   RustTool(const RustTool&) = delete;
   RustTool& operator=(const RustTool&) = delete;
 };
diff --git a/src/gn/target.cc b/src/gn/target.cc
index b6c6ca0..fb8dbef 100644
--- a/src/gn/target.cc
+++ b/src/gn/target.cc
@@ -14,6 +14,7 @@
 #include "gn/deps_iterator.h"
 #include "gn/filesystem_utils.h"
 #include "gn/functions.h"
+#include "gn/rust_tool.h"
 #include "gn/scheduler.h"
 #include "gn/substitution_writer.h"
 #include "gn/tool.h"
@@ -825,42 +826,56 @@
               this, tool, tool->outputs().list()[0]);
       break;
     case RUST_PROC_MACRO:
-    case SHARED_LIBRARY:
+    case SHARED_LIBRARY: {
       CHECK(tool->outputs().list().size() >= 1);
       check_tool_outputs = true;
+
+      const SubstitutionPattern* link_output_ptr = nullptr;
+      const SubstitutionPattern* depend_output_ptr = nullptr;
+      const SubstitutionList* runtime_outputs_ptr = nullptr;
+
       if (const CTool* ctool = tool->AsC()) {
-        if (ctool->link_output().empty() && ctool->depend_output().empty()) {
-          // Default behavior, use the first output file for both.
-          link_output_file_ = dependency_output_file_ =
-              SubstitutionWriter::ApplyPatternToLinkerAsOutputFile(
-                  this, tool, tool->outputs().list()[0]);
-        } else {
-          // Use the tool-specified ones.
-          if (!ctool->link_output().empty()) {
-            link_output_file_ =
-                SubstitutionWriter::ApplyPatternToLinkerAsOutputFile(
-                    this, tool, ctool->link_output());
-          }
-          if (!ctool->depend_output().empty()) {
-            dependency_output_file_ =
-                SubstitutionWriter::ApplyPatternToLinkerAsOutputFile(
-                    this, tool, ctool->depend_output());
-          }
-        }
-        if (tool->runtime_outputs().list().empty()) {
-          // Default to the link output for the runtime output.
-          runtime_outputs_.push_back(link_output_file_);
-        } else {
-          SubstitutionWriter::ApplyListToLinkerAsOutputFile(
-              this, tool, tool->runtime_outputs(), &runtime_outputs_);
-        }
-      } else if (tool->AsRust()) {
+        link_output_ptr =
+            ctool->link_output().empty() ? nullptr : &ctool->link_output();
+        depend_output_ptr =
+            ctool->depend_output().empty() ? nullptr : &ctool->depend_output();
+        runtime_outputs_ptr = &ctool->runtime_outputs();
+      } else if (const RustTool* rust_tool = tool->AsRust()) {
+        link_output_ptr = rust_tool->link_output().empty()
+                              ? nullptr
+                              : &rust_tool->link_output();
+        depend_output_ptr = rust_tool->depend_output().empty()
+                                ? nullptr
+                                : &rust_tool->depend_output();
+        runtime_outputs_ptr = &rust_tool->runtime_outputs();
+      }
+
+      if (!link_output_ptr && !depend_output_ptr) {
         // Default behavior, use the first output file for both.
         link_output_file_ = dependency_output_file_ =
             SubstitutionWriter::ApplyPatternToLinkerAsOutputFile(
                 this, tool, tool->outputs().list()[0]);
+      } else {
+        if (link_output_ptr) {
+          link_output_file_ =
+              SubstitutionWriter::ApplyPatternToLinkerAsOutputFile(
+                  this, tool, *link_output_ptr);
+        }
+        if (depend_output_ptr) {
+          dependency_output_file_ =
+              SubstitutionWriter::ApplyPatternToLinkerAsOutputFile(
+                  this, tool, *depend_output_ptr);
+        }
+      }
+      if (!runtime_outputs_ptr || runtime_outputs_ptr->list().empty()) {
+        // Default to the link output for the runtime output.
+        runtime_outputs_.push_back(link_output_file_);
+      } else {
+        SubstitutionWriter::ApplyListToLinkerAsOutputFile(
+            this, tool, *runtime_outputs_ptr, &runtime_outputs_);
       }
       break;
+    }
     case UNKNOWN:
     default:
       NOTREACHED();
diff --git a/src/gn/target_unittest.cc b/src/gn/target_unittest.cc
index 8af8f6f..5cc2182 100644
--- a/src/gn/target_unittest.cc
+++ b/src/gn/target_unittest.cc
@@ -553,6 +553,48 @@
   EXPECT_EQ("./liba.so", target.runtime_outputs()[0].value());
 }
 
+TEST_F(TargetTest, RustLinkAndDepOutputs) {
+  TestWithScope setup;
+  Err err;
+
+  Toolchain toolchain(setup.settings(), Label(SourceDir("//tc/"), "tc"));
+
+  std::unique_ptr<Tool> tool = Tool::CreateTool(RustTool::kRsToolDylib);
+  RustTool* rust_tool = tool->AsRust();
+  rust_tool->set_output_prefix("lib");
+  rust_tool->set_default_output_extension(".so");
+
+  const char kLinkPattern[] =
+      "{{root_out_dir}}/{{target_output_name}}{{output_extension}}";
+  SubstitutionPattern link_output =
+      SubstitutionPattern::MakeForTest(kLinkPattern);
+  rust_tool->set_link_output(link_output);
+
+  const char kDependPattern[] =
+      "{{root_out_dir}}/{{target_output_name}}{{output_extension}}.TOC";
+  SubstitutionPattern depend_output =
+      SubstitutionPattern::MakeForTest(kDependPattern);
+  rust_tool->set_depend_output(depend_output);
+
+  rust_tool->set_outputs(
+      SubstitutionList::MakeForTest(kLinkPattern, kDependPattern));
+
+  toolchain.SetTool(std::move(tool));
+
+  Target target(setup.settings(), Label(SourceDir("//a/"), "a"));
+  target.source_types_used().Set(SourceFile::SOURCE_RS);
+  target.rust_values().set_crate_type(RustValues::CRATE_DYLIB);
+  target.set_output_type(Target::SHARED_LIBRARY);
+  target.SetToolchain(&toolchain);
+  ASSERT_TRUE(target.OnResolved(&err));
+
+  EXPECT_EQ("./liba.so", target.link_output_file().value());
+  EXPECT_EQ("./liba.so.TOC", target.dependency_output_file().value());
+
+  ASSERT_EQ(1u, target.runtime_outputs().size());
+  EXPECT_EQ("./liba.so", target.runtime_outputs()[0].value());
+}
+
 // Tests that runtime_outputs works without an explicit link_output for
 // solink tools.
 //
@@ -609,6 +651,59 @@
   EXPECT_EQ("//out/Debug/a.pdb", computed_outputs[2].value());
 }
 
+TEST_F(TargetTest, RustRuntimeOuputs) {
+  TestWithScope setup;
+  Err err;
+
+  Toolchain toolchain(setup.settings(), Label(SourceDir("//tc/"), "tc"));
+
+  std::unique_ptr<Tool> tool = Tool::CreateTool(RustTool::kRsToolCDylib);
+  RustTool* rust_tool = tool->AsRust();
+  rust_tool->set_output_prefix("");
+  rust_tool->set_default_output_extension(".dll");
+
+  // Say the linker makes a DLL< an import library, and a symbol file we want
+  // to treat as a runtime output.
+  const char kLibPattern[] =
+      "{{root_out_dir}}/{{target_output_name}}{{output_extension}}.lib";
+  const char kDllPattern[] =
+      "{{root_out_dir}}/{{target_output_name}}{{output_extension}}";
+  const char kPdbPattern[] = "{{root_out_dir}}/{{target_output_name}}.pdb";
+  SubstitutionPattern pdb_pattern =
+      SubstitutionPattern::MakeForTest(kPdbPattern);
+
+  rust_tool->set_outputs(
+      SubstitutionList::MakeForTest(kLibPattern, kDllPattern, kPdbPattern));
+
+  // Say we only want the DLL and symbol file treaded as runtime outputs.
+  rust_tool->set_runtime_outputs(
+      SubstitutionList::MakeForTest(kDllPattern, kPdbPattern));
+
+  toolchain.SetTool(std::move(tool));
+
+  Target target(setup.settings(), Label(SourceDir("//a/"), "a"));
+  target.source_types_used().Set(SourceFile::SOURCE_RS);
+  target.rust_values().set_crate_type(RustValues::CRATE_CDYLIB);
+  target.set_output_type(Target::SHARED_LIBRARY);
+  target.SetToolchain(&toolchain);
+  ASSERT_TRUE(target.OnResolved(&err));
+
+  EXPECT_EQ("./a.dll.lib", target.link_output_file().value());
+  EXPECT_EQ("./a.dll.lib", target.dependency_output_file().value());
+
+  ASSERT_EQ(2u, target.runtime_outputs().size());
+  EXPECT_EQ("./a.dll", target.runtime_outputs()[0].value());
+  EXPECT_EQ("./a.pdb", target.runtime_outputs()[1].value());
+
+  // Test GetOutputsAsSourceFiles().
+  std::vector<SourceFile> computed_outputs;
+  EXPECT_TRUE(target.GetOutputsAsSourceFiles(LocationRange(), true,
+                                             &computed_outputs, &err));
+  ASSERT_EQ(3u, computed_outputs.size());
+  EXPECT_EQ("//out/Debug/a.dll.lib", computed_outputs[0].value());
+  EXPECT_EQ("//out/Debug/a.dll", computed_outputs[1].value());
+  EXPECT_EQ("//out/Debug/a.pdb", computed_outputs[2].value());
+}
 // Tests Target::GetOutputFilesForSource for binary targets (these require a
 // tool definition). Also tests GetOutputsAsSourceFiles() for source sets.
 TEST_F(TargetTest, GetOutputFilesForSource_Binary) {