Przeglądaj źródła

Switch FileTest to use StringRefs instead of files. (#2885)

In explorer, we already support parsing a string_view, so use that. In toolchain, we need to build support, probably using vfs, so that's a todo.

bazel test //explorer:file_test --runs_per_test=5

- branch: Stats over 250 runs: max = 18.3s, min = 5.2s, avg = 11.2s, dev = 2.9s
- trunk: Stats over 250 runs: max = 22.3s, min = 5.9s, avg = 12.1s, dev = 2.8s

Not a dramatic improvement, but maybe more effective long-term, and this'd been requested on #2876
Jon Ross-Perkins 2 lat temu
rodzic
commit
c43839e1b1

+ 4 - 4
explorer/file_test.cpp

@@ -32,7 +32,7 @@ class ParseAndExecuteTestFile : public FileTestBase {
     }
   }
 
-  auto RunWithFiles(const llvm::SmallVector<std::string>& test_files,
+  auto RunWithFiles(const llvm::SmallVector<TestFile>& test_files,
                     llvm::raw_ostream& stdout, llvm::raw_ostream& stderr)
       -> bool override {
     if (test_files.size() != 1) {
@@ -58,9 +58,9 @@ class ParseAndExecuteTestFile : public FileTestBase {
 
     // Run the parse. Parser debug output is always off because it's difficult
     // to redirect.
-    auto result =
-        ParseAndExecuteFile(prelude_path, path().filename().string(),
-                            /*parser_debug=*/false, &trace_stream, &stdout);
+    auto result = ParseAndExecute(
+        prelude_path, test_files[0].filename, test_files[0].content,
+        /*parser_debug=*/false, &trace_stream, &stdout);
     // This mirrors printing currently done by main.cpp.
     if (result.ok()) {
       stdout << "result: " << *result << "\n";

+ 3 - 1
explorer/fuzzing/fuzzer_util.cpp

@@ -39,7 +39,9 @@ auto ParseAndExecuteProto(const Fuzzing::Carbon& carbon) -> ErrorOr<int> {
   CARBON_CHECK(prelude_path.ok()) << prelude_path.error();
 
   const std::string source = ProtoToCarbon(carbon, /*maybe_add_main=*/true);
-  return ParseAndExecute(*prelude_path, source);
+  TraceStream trace_stream;
+  return ParseAndExecute(*prelude_path, "fuzzer.carbon", source,
+                         /*parser_debug=*/false, &trace_stream, &llvm::nulls());
 }
 
 }  // namespace Carbon::Testing

+ 10 - 10
explorer/parse_and_execute/parse_and_execute.cpp

@@ -38,7 +38,7 @@ static auto PrintTimingOnExit(TraceStream* trace_stream, const char* label,
 }
 
 static auto ParseAndExecuteHelper(std::function<ErrorOr<AST>(Arena*)> parse,
-                                  const std::string& prelude_path,
+                                  std::string_view prelude_path,
                                   Nonnull<TraceStream*> trace_stream,
                                   Nonnull<llvm::raw_ostream*> print_stream)
     -> ErrorOr<int> {
@@ -90,8 +90,8 @@ static auto ParseAndExecuteHelper(std::function<ErrorOr<AST>(Arena*)> parse,
   });
 }
 
-auto ParseAndExecuteFile(const std::string& prelude_path,
-                         const std::string& input_file_name, bool parser_debug,
+auto ParseAndExecuteFile(std::string_view prelude_path,
+                         std::string_view input_file_name, bool parser_debug,
                          Nonnull<TraceStream*> trace_stream,
                          Nonnull<llvm::raw_ostream*> print_stream)
     -> ErrorOr<int> {
@@ -101,15 +101,15 @@ auto ParseAndExecuteFile(const std::string& prelude_path,
   return ParseAndExecuteHelper(parse, prelude_path, trace_stream, print_stream);
 }
 
-auto ParseAndExecute(const std::string& prelude_path, const std::string& source)
-    -> ErrorOr<int> {
+auto ParseAndExecute(std::string_view prelude_path,
+                     std::string_view input_file_name,
+                     std::string_view file_contents, bool parser_debug,
+                     Nonnull<TraceStream*> trace_stream,
+                     Nonnull<llvm::raw_ostream*> print_stream) -> ErrorOr<int> {
   auto parse = [&](Arena* arena) {
-    return ParseFromString(arena, "test.carbon", source,
-                           /*parser_debug=*/false);
+    return ParseFromString(arena, input_file_name, file_contents, parser_debug);
   };
-  TraceStream trace_stream;
-  return ParseAndExecuteHelper(parse, prelude_path, &trace_stream,
-                               &llvm::nulls());
+  return ParseAndExecuteHelper(parse, prelude_path, trace_stream, print_stream);
 }
 
 }  // namespace Carbon

+ 7 - 4
explorer/parse_and_execute/parse_and_execute.h

@@ -12,16 +12,19 @@ namespace Carbon {
 
 // Parses and executes the input file, returning the program result on success.
 // This API is intended for use by main execution.
-auto ParseAndExecuteFile(const std::string& prelude_path,
-                         const std::string& input_file_name, bool parser_debug,
+auto ParseAndExecuteFile(std::string_view prelude_path,
+                         std::string_view input_file_name, bool parser_debug,
                          Nonnull<TraceStream*> trace_stream,
                          Nonnull<llvm::raw_ostream*> print_stream)
     -> ErrorOr<int>;
 
 // Parses and executes the source, returning the program result on success.
 // Discards output.
-auto ParseAndExecute(const std::string& prelude_path, const std::string& source)
-    -> ErrorOr<int>;
+auto ParseAndExecute(std::string_view prelude_path,
+                     std::string_view input_file_name,
+                     std::string_view file_contents, bool parser_debug,
+                     Nonnull<TraceStream*> trace_stream,
+                     Nonnull<llvm::raw_ostream*> print_stream) -> ErrorOr<int>;
 
 }  // namespace Carbon
 

+ 4 - 1
explorer/parse_and_execute/parse_and_execute_test.cpp

@@ -31,7 +31,10 @@ TEST(ParseAndExecuteTest, Recursion) {
         ;
     }
   )";
-  auto err = ParseAndExecute("explorer/data/prelude.carbon", source);
+  TraceStream trace_stream;
+  auto err =
+      ParseAndExecute("explorer/data/prelude.carbon", "test.carbon", source,
+                      /*parser_debug=*/false, &trace_stream, &llvm::nulls());
   ASSERT_FALSE(err.ok());
   EXPECT_THAT(err.error().message(),
               Eq("RUNTIME ERROR: overflow:1: stack overflow: too many "

+ 58 - 76
testing/file_test/file_test_base.cpp

@@ -8,7 +8,6 @@
 #include <fstream>
 
 #include "common/check.h"
-#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/InitLLVM.h"
 
@@ -18,20 +17,9 @@ namespace Carbon::Testing {
 static int base_dir_len = 0;
 // The name of the `.subset` target.
 static std::string* subset_target = nullptr;
-// The original working directory for restoration after each test.
-static std::filesystem::path* orig_working_dir = nullptr;
 
 using ::testing::Eq;
 
-FileTestBase::FileTestBase(const std::filesystem::path& path) : path_(&path) {}
-
-FileTestBase::~FileTestBase() {
-  // Restore the original working directory.
-  std::error_code ec;
-  std::filesystem::current_path(*orig_working_dir, ec);
-  CARBON_CHECK(!ec) << ec.message();
-}
-
 void FileTestBase::RegisterTests(
     const char* fixture_label,
     const llvm::SmallVector<std::filesystem::path>& paths,
@@ -47,6 +35,15 @@ void FileTestBase::RegisterTests(
   }
 }
 
+// Reads a file to string.
+static auto ReadFile(std::filesystem::path path) -> std::string {
+  std::ifstream proto_file(path);
+  std::stringstream buffer;
+  buffer << proto_file.rdbuf();
+  proto_file.close();
+  return buffer.str();
+}
+
 // Splits outputs to string_view because gtest handles string_view by default.
 static auto SplitOutput(llvm::StringRef output)
     -> llvm::SmallVector<std::string_view> {
@@ -68,20 +65,14 @@ auto FileTestBase::TestBody() -> void {
   llvm::errs() << "\nTo test this file alone, run:\n  bazel test "
                << *subset_target << " --test_arg=" << test_file << "\n\n";
 
-  SetTmpAsWorkingDir();
+  // Store the file so that test_files can use references to content.
+  std::string test_content = ReadFile(path());
 
   // Load expected output.
-  llvm::SmallVector<std::string> test_files;
-  auto cleanup_test_files = llvm::make_scope_exit([&]() {
-    // Remove all the created files. Even if no files are split, there will be a
-    // symlink to remove.
-    for (const auto& file : test_files) {
-      std::filesystem::remove(file);
-    }
-  });
+  llvm::SmallVector<TestFile> test_files;
   llvm::SmallVector<testing::Matcher<std::string>> expected_stdout;
   llvm::SmallVector<testing::Matcher<std::string>> expected_stderr;
-  ProcessTestFile(test_files, expected_stdout, expected_stderr);
+  ProcessTestFile(test_content, test_files, expected_stdout, expected_stderr);
   if (HasFailure()) {
     return;
   }
@@ -105,70 +96,66 @@ auto FileTestBase::TestBody() -> void {
   EXPECT_THAT(SplitOutput(stderr), ElementsAreArray(expected_stderr));
 }
 
-auto FileTestBase::SetTmpAsWorkingDir() -> void {
-  char* tmpdir = getenv("TEST_TMPDIR");
-  CARBON_CHECK(tmpdir);
-
-  // Run from the tmpdir.
-  std::error_code ec;
-  std::filesystem::current_path(tmpdir, ec);
-  CARBON_CHECK(!ec) << ec.message();
-}
-
 auto FileTestBase::ProcessTestFile(
-    llvm::SmallVector<std::string>& test_files,
+    llvm::StringRef file_content, llvm::SmallVector<TestFile>& test_files,
     llvm::SmallVector<testing::Matcher<std::string>>& expected_stdout,
     llvm::SmallVector<testing::Matcher<std::string>>& expected_stderr) -> void {
-  std::ifstream file_content(path());
-  std::ofstream split_file;
+  llvm::StringRef cursor = file_content;
   bool found_content_pre_split = false;
   int line_index = 0;
-  std::string line_str;
-  while (std::getline(file_content, line_str)) {
-    llvm::StringRef line = line_str;
-    if (line.consume_front("// ---")) {
-      ASSERT_FALSE(found_content_pre_split)
-          << "When using split files, there must be no content before the "
-             "first split file.";
-      test_files.push_back(line.trim().str());
-      const auto& test_file = test_files.back();
-      ASSERT_FALSE(std::filesystem::path(test_file).has_parent_path())
-          << "Only filenames are supported, not subdirectories: " << test_file;
-      if (split_file.is_open()) {
-        split_file.close();
+  llvm::StringRef current_file_name;
+  const char* current_file_start = nullptr;
+  while (!cursor.empty()) {
+    auto [line, next_cursor] = cursor.split("\n");
+    cursor = next_cursor;
+
+    static constexpr llvm::StringLiteral SplitPrefix = "// ---";
+    if (line.consume_front(SplitPrefix)) {
+      // On a file split, add the previous file, then start a new one.
+      if (current_file_start) {
+        test_files.push_back(TestFile(
+            current_file_name.str(),
+            llvm::StringRef(
+                current_file_start,
+                line.begin() - current_file_start - SplitPrefix.size())));
+      } else {
+        // For the first split, we make sure there was no content prior.
+        ASSERT_FALSE(found_content_pre_split)
+            << "When using split files, there must be no content before the "
+               "first split file.";
       }
-      split_file.open(test_file);
-      ASSERT_TRUE(split_file.is_open()) << "Failed to open " << test_file;
+      current_file_name = line.trim();
+      current_file_start = cursor.begin();
       line_index = 0;
       continue;
-    } else if (split_file.is_open()) {
-      split_file << line_str << "\n";
-    } else if (!line.starts_with("//") && !line.trim().empty()) {
+    } else if (!current_file_start && !line.starts_with("//") &&
+               !line.trim().empty()) {
       found_content_pre_split = true;
     }
     ++line_index;
 
-    line = line.ltrim();
-    if (!line.consume_front("// CHECK")) {
+    // Process expectations when found.
+    auto line_trimmed = line.ltrim();
+    if (!line_trimmed.consume_front("// CHECK")) {
       continue;
     }
-    if (line.consume_front(":STDOUT:")) {
-      expected_stdout.push_back(TransformExpectation(line_index, line));
-    } else if (line.consume_front(":STDERR:")) {
-      expected_stderr.push_back(TransformExpectation(line_index, line));
+    if (line_trimmed.consume_front(":STDOUT:")) {
+      expected_stdout.push_back(TransformExpectation(line_index, line_trimmed));
+    } else if (line_trimmed.consume_front(":STDERR:")) {
+      expected_stderr.push_back(TransformExpectation(line_index, line_trimmed));
     } else {
-      FAIL() << "Unexpected CHECK in input: " << line_str;
+      FAIL() << "Unexpected CHECK in input: " << line.str();
     }
   }
 
-  if (test_files.empty()) {
-    // Symlink the main test file and provide it in test_files.
-    test_files.push_back(path().filename());
-    std::error_code ec;
-    std::filesystem::create_symlink(path(), path().filename(), ec);
-    CARBON_CHECK(!ec) << ec.message();
+  if (current_file_start) {
+    test_files.push_back(
+        TestFile(current_file_name.str(),
+                 llvm::StringRef(current_file_start,
+                                 file_content.end() - current_file_start)));
   } else {
-    split_file.close();
+    // If no file splitting happened, use the main file as the test file.
+    test_files.push_back(TestFile(path().filename().string(), file_content));
   }
 
   // Assume there is always a suffix `\n` in output.
@@ -288,20 +275,15 @@ auto main(int argc, char** argv) -> int {
   }
   Carbon::Testing::subset_target = &subset_target_storage;
 
-  // Save the working directory for later restoration.
-  std::error_code ec;
-  std::filesystem::path orig_working_dir_storage =
-      std::filesystem::current_path(ec);
-  CARBON_CHECK(!ec) << ec.message();
-  Carbon::Testing::orig_working_dir = &orig_working_dir_storage;
-
   // Configure the base directory for test names.
   llvm::StringRef target_dir = target;
+  std::error_code ec;
+  std::filesystem::path working_dir = std::filesystem::current_path(ec);
+  CARBON_CHECK(!ec) << ec.message();
   // Leaves one slash.
   CARBON_CHECK(target_dir.consume_front("/"));
   target_dir = target_dir.substr(0, target_dir.rfind(":"));
-  std::string base_dir =
-      orig_working_dir_storage.string() + target_dir.str() + "/";
+  std::string base_dir = working_dir.string() + target_dir.str() + "/";
   Carbon::Testing::base_dir_len = base_dir.size();
 
   // Register tests based on their absolute path.

+ 20 - 7
testing/file_test/file_test_base.h

@@ -14,6 +14,7 @@
 
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/raw_os_ostream.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace Carbon::Testing {
@@ -34,8 +35,23 @@ namespace Carbon::Testing {
 // `autoupdate_testdata.py` automatically constructs compatible lines.
 class FileTestBase : public testing::Test {
  public:
-  explicit FileTestBase(const std::filesystem::path& path);
-  ~FileTestBase() override;
+  struct TestFile {
+    explicit TestFile(std::string filename, llvm::StringRef content)
+        : filename(std::move(filename)), content(content) {}
+
+    friend void PrintTo(const TestFile& f, std::ostream* os) {
+      // Print content escaped.
+      llvm::raw_os_ostream os_wrap(*os);
+      os_wrap << "TestFile(" << f.filename << ", \"";
+      os_wrap.write_escaped(f.content);
+      os_wrap << "\")";
+    }
+
+    std::string filename;
+    llvm::StringRef content;
+  };
+
+  explicit FileTestBase(const std::filesystem::path& path) : path_(&path) {}
 
   // Used by children to register tests with gtest.
   static auto RegisterTests(
@@ -56,7 +72,7 @@ class FileTestBase : public testing::Test {
   // Implemented by children to run the test. Called by the TestBody
   // implementation, which will validate stdout and stderr. The return value
   // should be false when "fail_" is in the filename.
-  virtual auto RunWithFiles(const llvm::SmallVector<std::string>& test_files,
+  virtual auto RunWithFiles(const llvm::SmallVector<TestFile>& test_files,
                             llvm::raw_ostream& stdout,
                             llvm::raw_ostream& stderr) -> bool = 0;
 
@@ -68,12 +84,9 @@ class FileTestBase : public testing::Test {
   auto path() -> const std::filesystem::path& { return *path_; };
 
  private:
-  // Sets TEST_TMPDIR as the working directory.
-  auto SetTmpAsWorkingDir() -> void;
-
   // Processes the test input, producing test files and expected output.
   auto ProcessTestFile(
-      llvm::SmallVector<std::string>& test_files,
+      llvm::StringRef file_content, llvm::SmallVector<TestFile>& test_files,
       llvm::SmallVector<testing::Matcher<std::string>>& expected_stdout,
       llvm::SmallVector<testing::Matcher<std::string>>& expected_stderr)
       -> void;

+ 22 - 18
testing/file_test/file_test_base_test.cpp

@@ -16,19 +16,30 @@
 namespace Carbon::Testing {
 namespace {
 
+using ::testing::AllOf;
 using ::testing::ElementsAre;
+using ::testing::Eq;
+using ::testing::Field;
 
 class FileTestBaseTest : public FileTestBase {
  public:
   explicit FileTestBaseTest(const std::filesystem::path& path)
       : FileTestBase(path) {}
 
-  auto RunWithFiles(const llvm::SmallVector<std::string>& test_files,
+  static auto HasFilename(std::string filename) -> testing::Matcher<TestFile> {
+    return Field("filename", &TestFile::filename, Eq(filename));
+  }
+
+  static auto HasContent(std::string content) -> testing::Matcher<TestFile> {
+    return Field("content", &TestFile::content, Eq(content));
+  }
+
+  auto RunWithFiles(const llvm::SmallVector<TestFile>& test_files,
                     llvm::raw_ostream& stdout, llvm::raw_ostream& stderr)
       -> bool override {
     auto filename = path().filename();
     if (filename == "example.carbon") {
-      EXPECT_THAT(test_files, ElementsAre("example.carbon"));
+      EXPECT_THAT(test_files, ElementsAre(HasFilename("example.carbon")));
       stdout << "something\n"
                 "\n"
                 "8: Line delta\n"
@@ -37,29 +48,22 @@ class FileTestBaseTest : public FileTestBase {
                 "Foo baz\n";
       return true;
     } else if (filename == "fail_example.carbon") {
-      EXPECT_THAT(test_files, ElementsAre("fail_example.carbon"));
+      EXPECT_THAT(test_files, ElementsAre(HasFilename("fail_example.carbon")));
       stderr << "Oops\n";
       return false;
     } else if (filename == "two_files.carbon") {
       int i = 0;
       for (const auto& file : test_files) {
         // Prints line numbers to validate per-file.
-        stdout << file << ": " << ++i << "\n";
-
-        // Make sure the split files have appropriate content.
-        std::ifstream file_in(file);
-        std::stringstream content;
-        content << file_in.rdbuf();
-        if (file == "a.carbon") {
-          EXPECT_THAT(content.str(),
-                      testing::Eq("// CHECK:STDOUT: a.carbon: [[@LINE+0]]\n\n"))
-              << "Checking " << file;
-        } else {
-          EXPECT_THAT(content.str(),
-                      testing::Eq("// CHECK:STDOUT: b.carbon: [[@LINE+1]]\n"))
-              << "Checking " << file;
-        }
+        stdout << file.filename << ": " << ++i << "\n";
       }
+      EXPECT_THAT(
+          test_files,
+          ElementsAre(
+              AllOf(HasFilename("a.carbon"),
+                    HasContent("// CHECK:STDOUT: a.carbon: [[@LINE+0]]\n\n")),
+              AllOf(HasFilename("b.carbon"),
+                    HasContent("// CHECK:STDOUT: b.carbon: [[@LINE+1]]\n"))));
       return true;
     } else {
       ADD_FAILURE() << "Unexpected file: " << filename;

+ 40 - 3
toolchain/driver/driver_file_test_base.h

@@ -5,6 +5,10 @@
 #ifndef CARBON_TOOLCHAIN_DRIVER_DRIVER_FILE_TEST_BASE_H_
 #define CARBON_TOOLCHAIN_DRIVER_DRIVER_FILE_TEST_BASE_H_
 
+#include <filesystem>
+#include <fstream>
+
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "testing/file_test/file_test_base.h"
@@ -18,14 +22,47 @@ class DriverFileTestBase : public FileTestBase {
  public:
   using FileTestBase::FileTestBase;
 
-  auto RunWithFiles(const llvm::SmallVector<std::string>& test_files,
+  auto RunWithFiles(const llvm::SmallVector<TestFile>& test_files,
                     llvm::raw_ostream& stdout, llvm::raw_ostream& stderr)
       -> bool override {
+    // TODO: The working dir and file creation logic should be replaced with vfs
+    // use. That will require modifying Driver and SourceBuffer to use vfs.
+    // Get the working dir.
+    std::error_code ec;
+    auto orig_dir = std::filesystem::current_path(ec);
+    CARBON_CHECK(!ec) << ec.message();
+
+    // Set the working dir.
+    const char* tmpdir = getenv("TEST_TMPDIR");
+    CARBON_CHECK(tmpdir);
+    std::filesystem::current_path(tmpdir, ec);
+    CARBON_CHECK(!ec) << ec.message();
+
+    // Prepare a list of filenames for MakeArgs. Also create the files.
+    llvm::SmallVector<llvm::StringRef> test_file_names;
+    for (const auto& test_file : test_files) {
+      test_file_names.push_back(test_file.filename);
+
+      std::ofstream f(test_file.filename);
+      f << test_file.content;
+    }
+
+    auto cleanup = llvm::make_scope_exit([&]() {
+      // Remove the files.
+      for (const auto& test_file : test_files) {
+        std::filesystem::remove(test_file.filename, ec);
+        CARBON_CHECK(!ec) << ec.message();
+      }
+      // Restore the working dir.
+      std::filesystem::current_path(orig_dir, ec);
+      CARBON_CHECK(!ec) << ec.message();
+    });
+
     Driver driver(stdout, stderr);
-    return driver.RunFullCommand(MakeArgs(test_files));
+    return driver.RunFullCommand(MakeArgs(test_file_names));
   }
 
-  virtual auto MakeArgs(const llvm::SmallVector<std::string>& test_files)
+  virtual auto MakeArgs(const llvm::SmallVector<llvm::StringRef>& test_files)
       -> llvm::SmallVector<llvm::StringRef> = 0;
 };
 

+ 2 - 4
toolchain/lexer/lexer_file_test.cpp

@@ -16,12 +16,10 @@ class LexerFileTest : public DriverFileTestBase {
  public:
   using DriverFileTestBase::DriverFileTestBase;
 
-  auto MakeArgs(const llvm::SmallVector<std::string>& test_files)
+  auto MakeArgs(const llvm::SmallVector<llvm::StringRef>& test_files)
       -> llvm::SmallVector<llvm::StringRef> override {
     llvm::SmallVector<llvm::StringRef> args({"dump", "tokens"});
-    for (const auto& file : test_files) {
-      args.push_back(file);
-    }
+    args.insert(args.end(), test_files.begin(), test_files.end());
     return args;
   }
 };

+ 2 - 4
toolchain/lowering/lowering_file_test.cpp

@@ -16,12 +16,10 @@ class LoweringFileTest : public DriverFileTestBase {
  public:
   using DriverFileTestBase::DriverFileTestBase;
 
-  auto MakeArgs(const llvm::SmallVector<std::string>& test_files)
+  auto MakeArgs(const llvm::SmallVector<llvm::StringRef>& test_files)
       -> llvm::SmallVector<llvm::StringRef> override {
     llvm::SmallVector<llvm::StringRef> args({"dump", "llvm-ir"});
-    for (const auto& file : test_files) {
-      args.push_back(file);
-    }
+    args.insert(args.end(), test_files.begin(), test_files.end());
     return args;
   }
 };

+ 2 - 4
toolchain/parser/parse_tree_file_test.cpp

@@ -16,12 +16,10 @@ class ParseTreeFileTest : public DriverFileTestBase {
  public:
   using DriverFileTestBase::DriverFileTestBase;
 
-  auto MakeArgs(const llvm::SmallVector<std::string>& test_files)
+  auto MakeArgs(const llvm::SmallVector<llvm::StringRef>& test_files)
       -> llvm::SmallVector<llvm::StringRef> override {
     llvm::SmallVector<llvm::StringRef> args({"dump", "parse-tree"});
-    for (const auto& file : test_files) {
-      args.push_back(file);
-    }
+    args.insert(args.end(), test_files.begin(), test_files.end());
     return args;
   }
 };

+ 2 - 4
toolchain/semantics/semantics_file_test.cpp

@@ -16,12 +16,10 @@ class SemanticsFileTest : public DriverFileTestBase {
  public:
   using DriverFileTestBase::DriverFileTestBase;
 
-  auto MakeArgs(const llvm::SmallVector<std::string>& test_files)
+  auto MakeArgs(const llvm::SmallVector<llvm::StringRef>& test_files)
       -> llvm::SmallVector<llvm::StringRef> override {
     llvm::SmallVector<llvm::StringRef> args({"dump", "semantics-ir"});
-    for (const auto& file : test_files) {
-      args.push_back(file);
-    }
+    args.insert(args.end(), test_files.begin(), test_files.end());
     return args;
   }
 };