Просмотр исходного кода

Add vfs support to toolchain. (#2888)

This adds vfs support to the toolchain, allowing Driver to take in-memory inputs in tests. As a consequence, I'm simplifying SourceBuffer: rather than allowing tests to pass in their own memory buffer, I'm using InMemoryFileSystem to push for greater consistency with production code. This does hit a quirk where I need to be careful about null terminator handling because fuzzer imports don't always have one, but that's probably more robust anyways.

Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Jon Ross-Perkins 2 лет назад
Родитель
Сommit
a93e621488

+ 4 - 10
toolchain/driver/driver.cpp

@@ -10,8 +10,6 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/IR/LLVMContext.h"
-#include "llvm/Support/Error.h"
-#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Format.h"
 #include "toolchain/diagnostics/diagnostic_emitter.h"
 #include "toolchain/diagnostics/sorting_diagnostic_consumer.h"
@@ -168,15 +166,11 @@ auto Driver::RunDumpSubcommand(DiagnosticConsumer& consumer,
   }
 
   CARBON_VLOG() << "*** SourceBuffer::CreateFromFile ***\n";
-  auto source = SourceBuffer::CreateFromFile(input_file_name);
+  auto source = SourceBuffer::CreateFromFile(fs_, input_file_name);
   CARBON_VLOG() << "*** SourceBuffer::CreateFromFile done ***\n";
-  if (!source) {
-    error_stream_ << "ERROR: Unable to open input source file: ";
-    llvm::handleAllErrors(source.takeError(),
-                          [&](const llvm::ErrorInfoBase& ei) {
-                            ei.log(error_stream_);
-                            error_stream_ << "\n";
-                          });
+  if (!source.ok()) {
+    error_stream_ << "ERROR: Unable to open input source file: "
+                  << source.error();
     return false;
   }
 

+ 7 - 6
toolchain/driver/driver.h

@@ -10,6 +10,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Debug.h"
+#include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Support/raw_ostream.h"
 #include "toolchain/diagnostics/diagnostic_emitter.h"
 
@@ -22,14 +23,13 @@ namespace Carbon {
 // with the language.
 class Driver {
  public:
-  // Default constructed driver uses stderr for all error and informational
-  // output.
-  Driver() : output_stream_(llvm::outs()), error_stream_(llvm::errs()) {}
-
   // Constructs a driver with any error or informational output directed to a
   // specified stream.
-  Driver(llvm::raw_ostream& output_stream, llvm::raw_ostream& error_stream)
-      : output_stream_(output_stream), error_stream_(error_stream) {}
+  Driver(llvm::vfs::FileSystem& fs, llvm::raw_ostream& output_stream,
+         llvm::raw_ostream& error_stream)
+      : fs_(fs), output_stream_(output_stream), error_stream_(error_stream) {
+    (void)fs_;
+  }
 
   // Parses the given arguments into both a subcommand to select the operation
   // to perform and any arguments to that subcommand.
@@ -65,6 +65,7 @@ class Driver {
   auto ReportExtraArgs(llvm::StringRef subcommand_text,
                        llvm::ArrayRef<llvm::StringRef> args) -> void;
 
+  llvm::vfs::FileSystem& fs_;
   llvm::raw_ostream& output_stream_;
   llvm::raw_ostream& error_stream_;
   llvm::raw_ostream* vlog_stream_ = nullptr;

+ 11 - 29
toolchain/driver/driver_file_test_base.h

@@ -5,12 +5,13 @@
 #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 "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/VirtualFileSystem.h"
 #include "testing/file_test/file_test_base.h"
 #include "toolchain/driver/driver.h"
 
@@ -25,40 +26,21 @@ class DriverFileTestBase : public FileTestBase {
   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.
+    // Prepare a list of filenames for MakeArgs. Also create the files
+    // in-memory.
     llvm::SmallVector<llvm::StringRef> test_file_names;
+    llvm::vfs::InMemoryFileSystem fs;
     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();
+      if (!fs.addFile(test_file.filename, /*ModificationTime=*/0,
+                      llvm::MemoryBuffer::getMemBuffer(test_file.content))) {
+        ADD_FAILURE() << "File is repeated: " << test_file.filename;
+        return false;
       }
-      // Restore the working dir.
-      std::filesystem::current_path(orig_dir, ec);
-      CARBON_CHECK(!ec) << ec.message();
-    });
+    }
 
-    Driver driver(stdout, stderr);
+    Driver driver(fs, stdout, stderr);
     return driver.RunFullCommand(MakeArgs(test_file_names));
   }
 

+ 2 - 2
toolchain/driver/driver_fuzzer.cpp

@@ -66,10 +66,10 @@ extern "C" auto LLVMFuzzerTestOneInput(const unsigned char* data, size_t size)
     size -= arg_length;
   }
 
+  llvm::vfs::InMemoryFileSystem fs;
   std::string error_text;
   llvm::raw_string_ostream error_stream(error_text);
-  llvm::raw_null_ostream output_stream;
-  Driver d(output_stream, error_stream);
+  Driver d(fs, llvm::nulls(), error_stream);
   if (!d.RunFullCommand(args)) {
     error_stream.flush();
     if (error_text.find("ERROR:") == std::string::npos) {

+ 2 - 1
toolchain/driver/driver_main.cpp

@@ -28,7 +28,8 @@ auto main(int argc, char** argv) -> int {
   llvm::errs().tie(&llvm::outs());
 
   llvm::SmallVector<llvm::StringRef, 16> args(argv + 1, argv + argc);
-  Carbon::Driver driver;
+  auto fs = llvm::vfs::getRealFileSystem();
+  Carbon::Driver driver(*fs, llvm::outs(), llvm::errs());
   bool success = driver.RunFullCommand(args);
   return success ? EXIT_SUCCESS : EXIT_FAILURE;
 }

+ 74 - 96
toolchain/driver/driver_test.cpp

@@ -21,29 +21,38 @@ using ::testing::ElementsAre;
 using ::testing::HasSubstr;
 using ::testing::StrEq;
 
-TEST(DriverTest, FullCommandErrors) {
-  TestRawOstream test_output_stream;
-  TestRawOstream test_error_stream;
-  Driver driver = Driver(test_output_stream, test_error_stream);
+static constexpr llvm::StringLiteral TestFileName = "test_file.carbon";
 
-  EXPECT_FALSE(driver.RunFullCommand({}));
-  EXPECT_THAT(test_error_stream.TakeStr(), HasSubstr("ERROR"));
+class DriverTest : public testing::Test {
+ protected:
+  DriverTest() : driver_(fs_, test_output_stream_, test_error_stream_) {}
 
-  EXPECT_FALSE(driver.RunFullCommand({"foo"}));
-  EXPECT_THAT(test_error_stream.TakeStr(), HasSubstr("ERROR"));
+  auto CreateTestFile(llvm::StringRef text) -> void {
+    fs_.addFile(TestFileName, /*ModificationTime=*/0,
+                llvm::MemoryBuffer::getMemBuffer(text));
+  }
 
-  EXPECT_FALSE(driver.RunFullCommand({"foo --bar --baz"}));
-  EXPECT_THAT(test_error_stream.TakeStr(), HasSubstr("ERROR"));
-}
+  llvm::vfs::InMemoryFileSystem fs_;
+  TestRawOstream test_output_stream_;
+  TestRawOstream test_error_stream_;
+  Driver driver_;
+};
+
+TEST_F(DriverTest, FullCommandErrors) {
+  EXPECT_FALSE(driver_.RunFullCommand({}));
+  EXPECT_THAT(test_error_stream_.TakeStr(), HasSubstr("ERROR"));
+
+  EXPECT_FALSE(driver_.RunFullCommand({"foo"}));
+  EXPECT_THAT(test_error_stream_.TakeStr(), HasSubstr("ERROR"));
 
-TEST(DriverTest, Help) {
-  TestRawOstream test_output_stream;
-  TestRawOstream test_error_stream;
-  Driver driver = Driver(test_output_stream, test_error_stream);
+  EXPECT_FALSE(driver_.RunFullCommand({"foo --bar --baz"}));
+  EXPECT_THAT(test_error_stream_.TakeStr(), HasSubstr("ERROR"));
+}
 
-  EXPECT_TRUE(driver.RunHelpSubcommand(ConsoleDiagnosticConsumer(), {}));
-  EXPECT_THAT(test_error_stream.TakeStr(), StrEq(""));
-  auto help_text = test_output_stream.TakeStr();
+TEST_F(DriverTest, Help) {
+  EXPECT_TRUE(driver_.RunHelpSubcommand(ConsoleDiagnosticConsumer(), {}));
+  EXPECT_THAT(test_error_stream_.TakeStr(), StrEq(""));
+  auto help_text = test_output_stream_.TakeStr();
 
   // Help text should mention each subcommand.
 #define CARBON_SUBCOMMAND(Name, Spelling, ...) \
@@ -51,56 +60,33 @@ TEST(DriverTest, Help) {
 #include "toolchain/driver/flags.def"
 
   // Check that the subcommand dispatch works.
-  EXPECT_TRUE(driver.RunFullCommand({"help"}));
-  EXPECT_THAT(test_error_stream.TakeStr(), StrEq(""));
-  EXPECT_THAT(test_output_stream.TakeStr(), StrEq(help_text));
+  EXPECT_TRUE(driver_.RunFullCommand({"help"}));
+  EXPECT_THAT(test_error_stream_.TakeStr(), StrEq(""));
+  EXPECT_THAT(test_output_stream_.TakeStr(), StrEq(help_text));
 }
 
-TEST(DriverTest, HelpErrors) {
-  TestRawOstream test_output_stream;
-  TestRawOstream test_error_stream;
-  Driver driver = Driver(test_output_stream, test_error_stream);
-
-  EXPECT_FALSE(driver.RunHelpSubcommand(ConsoleDiagnosticConsumer(), {"foo"}));
-  EXPECT_THAT(test_output_stream.TakeStr(), StrEq(""));
-  EXPECT_THAT(test_error_stream.TakeStr(), HasSubstr("ERROR"));
-
-  EXPECT_FALSE(driver.RunHelpSubcommand(ConsoleDiagnosticConsumer(), {"help"}));
-  EXPECT_THAT(test_output_stream.TakeStr(), StrEq(""));
-  EXPECT_THAT(test_error_stream.TakeStr(), HasSubstr("ERROR"));
+TEST_F(DriverTest, HelpErrors) {
+  EXPECT_FALSE(driver_.RunHelpSubcommand(ConsoleDiagnosticConsumer(), {"foo"}));
+  EXPECT_THAT(test_output_stream_.TakeStr(), StrEq(""));
+  EXPECT_THAT(test_error_stream_.TakeStr(), HasSubstr("ERROR"));
 
   EXPECT_FALSE(
-      driver.RunHelpSubcommand(ConsoleDiagnosticConsumer(), {"--xyz"}));
-  EXPECT_THAT(test_output_stream.TakeStr(), StrEq(""));
-  EXPECT_THAT(test_error_stream.TakeStr(), HasSubstr("ERROR"));
-}
+      driver_.RunHelpSubcommand(ConsoleDiagnosticConsumer(), {"help"}));
+  EXPECT_THAT(test_output_stream_.TakeStr(), StrEq(""));
+  EXPECT_THAT(test_error_stream_.TakeStr(), HasSubstr("ERROR"));
 
-auto CreateTestFile(llvm::StringRef text) -> std::string {
-  int fd = -1;
-  llvm::SmallString<1024> path;
-  auto ec = llvm::sys::fs::createTemporaryFile("test_file", ".txt", fd, path);
-  if (ec) {
-    llvm::report_fatal_error(llvm::Twine("Failed to create temporary file: ") +
-                             ec.message());
-  }
-
-  llvm::raw_fd_ostream s(fd, /*shouldClose=*/true);
-  s << text;
-  s.close();
-
-  return path.str().str();
+  EXPECT_FALSE(
+      driver_.RunHelpSubcommand(ConsoleDiagnosticConsumer(), {"--xyz"}));
+  EXPECT_THAT(test_output_stream_.TakeStr(), StrEq(""));
+  EXPECT_THAT(test_error_stream_.TakeStr(), HasSubstr("ERROR"));
 }
 
-TEST(DriverTest, DumpTokens) {
-  TestRawOstream test_output_stream;
-  TestRawOstream test_error_stream;
-  Driver driver = Driver(test_output_stream, test_error_stream);
-
-  auto test_file_path = CreateTestFile("Hello World");
-  EXPECT_TRUE(driver.RunDumpSubcommand(ConsoleDiagnosticConsumer(),
-                                       {"tokens", test_file_path}));
-  EXPECT_THAT(test_error_stream.TakeStr(), StrEq(""));
-  auto tokenized_text = test_output_stream.TakeStr();
+TEST_F(DriverTest, DumpTokens) {
+  CreateTestFile("Hello World");
+  EXPECT_TRUE(driver_.RunDumpSubcommand(ConsoleDiagnosticConsumer(),
+                                        {"tokens", TestFileName}));
+  EXPECT_THAT(test_error_stream_.TakeStr(), StrEq(""));
+  auto tokenized_text = test_output_stream_.TakeStr();
 
   EXPECT_THAT(Yaml::Value::FromText(tokenized_text),
               ElementsAre(Yaml::SequenceValue{
@@ -128,53 +114,45 @@ TEST(DriverTest, DumpTokens) {
                                      {"spelling", ""}}}));
 
   // Check that the subcommand dispatch works.
-  EXPECT_TRUE(driver.RunFullCommand({"dump", "tokens", test_file_path}));
-  EXPECT_THAT(test_error_stream.TakeStr(), StrEq(""));
-  EXPECT_THAT(test_output_stream.TakeStr(), StrEq(tokenized_text));
+  EXPECT_TRUE(driver_.RunFullCommand({"dump", "tokens", TestFileName}));
+  EXPECT_THAT(test_error_stream_.TakeStr(), StrEq(""));
+  EXPECT_THAT(test_output_stream_.TakeStr(), StrEq(tokenized_text));
 }
 
-TEST(DriverTest, DumpErrors) {
-  TestRawOstream test_output_stream;
-  TestRawOstream test_error_stream;
-  Driver driver = Driver(test_output_stream, test_error_stream);
-
-  EXPECT_FALSE(driver.RunDumpSubcommand(ConsoleDiagnosticConsumer(), {"foo"}));
-  EXPECT_THAT(test_output_stream.TakeStr(), StrEq(""));
-  EXPECT_THAT(test_error_stream.TakeStr(), HasSubstr("ERROR"));
+TEST_F(DriverTest, DumpErrors) {
+  EXPECT_FALSE(driver_.RunDumpSubcommand(ConsoleDiagnosticConsumer(), {"foo"}));
+  EXPECT_THAT(test_output_stream_.TakeStr(), StrEq(""));
+  EXPECT_THAT(test_error_stream_.TakeStr(), HasSubstr("ERROR"));
 
   EXPECT_FALSE(
-      driver.RunDumpSubcommand(ConsoleDiagnosticConsumer(), {"--xyz"}));
-  EXPECT_THAT(test_output_stream.TakeStr(), StrEq(""));
-  EXPECT_THAT(test_error_stream.TakeStr(), HasSubstr("ERROR"));
+      driver_.RunDumpSubcommand(ConsoleDiagnosticConsumer(), {"--xyz"}));
+  EXPECT_THAT(test_output_stream_.TakeStr(), StrEq(""));
+  EXPECT_THAT(test_error_stream_.TakeStr(), HasSubstr("ERROR"));
 
   EXPECT_FALSE(
-      driver.RunDumpSubcommand(ConsoleDiagnosticConsumer(), {"tokens"}));
-  EXPECT_THAT(test_output_stream.TakeStr(), StrEq(""));
-  EXPECT_THAT(test_error_stream.TakeStr(), HasSubstr("ERROR"));
-
-  EXPECT_FALSE(driver.RunDumpSubcommand(ConsoleDiagnosticConsumer(),
-                                        {"tokens", "/not/a/real/file/name"}));
-  EXPECT_THAT(test_output_stream.TakeStr(), StrEq(""));
-  EXPECT_THAT(test_error_stream.TakeStr(), HasSubstr("ERROR"));
+      driver_.RunDumpSubcommand(ConsoleDiagnosticConsumer(), {"tokens"}));
+  EXPECT_THAT(test_output_stream_.TakeStr(), StrEq(""));
+  EXPECT_THAT(test_error_stream_.TakeStr(), HasSubstr("ERROR"));
+
+  EXPECT_FALSE(driver_.RunDumpSubcommand(ConsoleDiagnosticConsumer(),
+                                         {"tokens", "/not/a/real/file/name"}));
+  EXPECT_THAT(test_output_stream_.TakeStr(), StrEq(""));
+  EXPECT_THAT(test_error_stream_.TakeStr(), HasSubstr("ERROR"));
 }
 
-TEST(DriverTest, DumpParseTree) {
-  TestRawOstream test_output_stream;
-  TestRawOstream test_error_stream;
-  Driver driver = Driver(test_output_stream, test_error_stream);
-
-  auto test_file_path = CreateTestFile("var v: Int = 42;");
-  EXPECT_TRUE(driver.RunDumpSubcommand(ConsoleDiagnosticConsumer(),
-                                       {"parse-tree", test_file_path}));
-  EXPECT_THAT(test_error_stream.TakeStr(), StrEq(""));
+TEST_F(DriverTest, DumpParseTree) {
+  CreateTestFile("var v: Int = 42;");
+  EXPECT_TRUE(driver_.RunDumpSubcommand(ConsoleDiagnosticConsumer(),
+                                        {"parse-tree", TestFileName}));
+  EXPECT_THAT(test_error_stream_.TakeStr(), StrEq(""));
   // Verify there is output without examining it.
-  EXPECT_FALSE(test_output_stream.TakeStr().empty());
+  EXPECT_FALSE(test_output_stream_.TakeStr().empty());
 
   // Check that the subcommand dispatch works.
-  EXPECT_TRUE(driver.RunFullCommand({"dump", "parse-tree", test_file_path}));
-  EXPECT_THAT(test_error_stream.TakeStr(), StrEq(""));
+  EXPECT_TRUE(driver_.RunFullCommand({"dump", "parse-tree", TestFileName}));
+  EXPECT_THAT(test_error_stream_.TakeStr(), StrEq(""));
   // Verify there is output without examining it.
-  EXPECT_FALSE(test_output_stream.TakeStr().empty());
+  EXPECT_FALSE(test_output_stream_.TakeStr().empty());
 }
 
 }  // namespace

+ 2 - 4
toolchain/lexer/tokenized_buffer.cpp

@@ -15,7 +15,6 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
-#include "llvm/ADT/Twine.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/FormatVariadic.h"
@@ -810,9 +809,8 @@ auto TokenizedBuffer::PrintToken(llvm::raw_ostream& output_stream, Token token,
       "{ index: {0}, kind: {1}, line: {2}, column: {3}, indent: {4}, "
       "spelling: '{5}'",
       llvm::format_decimal(token_index, widths.index),
-      llvm::right_justify(
-          (llvm::Twine("'") + token_info.kind.name() + "'").str(),
-          widths.kind + 2),
+      llvm::right_justify(llvm::formatv("'{0}'", token_info.kind.name()).str(),
+                          widths.kind + 2),
       llvm::format_decimal(GetLineNumber(token_info.token_line), widths.line),
       llvm::format_decimal(GetColumnNumber(token), widths.column),
       llvm::format_decimal(GetIndentColumnNumber(token_info.token_line),

+ 8 - 2
toolchain/lexer/tokenized_buffer_fuzzer.cpp

@@ -25,8 +25,14 @@ extern "C" int LLVMFuzzerTestOneInput(const unsigned char* data,
   if (size > 100000) {
     return 0;
   }
-  auto source = SourceBuffer::CreateFromText(
-      llvm::StringRef(reinterpret_cast<const char*>(data), size));
+  static constexpr llvm::StringLiteral TestFileName = "test.carbon";
+  llvm::vfs::InMemoryFileSystem fs;
+  llvm::StringRef data_ref(reinterpret_cast<const char*>(data), size);
+  CARBON_CHECK(fs.addFile(
+      TestFileName, /*ModificationTime=*/0,
+      llvm::MemoryBuffer::getMemBuffer(data_ref, /*BufferName=*/TestFileName,
+                                       /*RequiresNullTerminator=*/false)));
+  auto source = SourceBuffer::CreateFromFile(fs, TestFileName);
 
   auto buffer = TokenizedBuffer::Lex(*source, NullDiagnosticConsumer());
   if (buffer.has_errors()) {

+ 11 - 7
toolchain/lexer/tokenized_buffer_test.cpp

@@ -12,7 +12,6 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Sequence.h"
 #include "llvm/ADT/SmallString.h"
-#include "llvm/ADT/Twine.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/raw_ostream.h"
 #include "toolchain/common/yaml_test_helpers.h"
@@ -31,19 +30,24 @@ using ::testing::StrEq;
 
 class LexerTest : public ::testing::Test {
  protected:
-  auto GetSourceBuffer(llvm::Twine text) -> SourceBuffer& {
-    source_storage.push_back(
-        std::move(*SourceBuffer::CreateFromText(text.str())));
-    return source_storage.back();
+  auto GetSourceBuffer(llvm::StringRef text) -> SourceBuffer& {
+    std::string filename = llvm::formatv("test{0}.carbon", ++file_index);
+    CARBON_CHECK(fs.addFile(filename, /*ModificationTime=*/0,
+                            llvm::MemoryBuffer::getMemBuffer(text)));
+    source_storage.push_front(
+        std::move(*SourceBuffer::CreateFromFile(fs, filename)));
+    return source_storage.front();
   }
 
-  auto Lex(llvm::Twine text,
+  auto Lex(llvm::StringRef text,
            DiagnosticConsumer& consumer = ConsoleDiagnosticConsumer())
       -> TokenizedBuffer {
     return TokenizedBuffer::Lex(GetSourceBuffer(text), consumer);
   }
 
-  llvm::SmallVector<SourceBuffer, 16> source_storage;
+  llvm::vfs::InMemoryFileSystem fs;
+  int file_index = 0;
+  std::forward_list<SourceBuffer> source_storage;
 };
 
 TEST_F(LexerTest, HandlesEmptyBuffer) {

+ 8 - 3
toolchain/parser/parse_tree_fuzzer.cpp

@@ -22,9 +22,14 @@ extern "C" int LLVMFuzzerTestOneInput(const unsigned char* data,
   if (size > 100000) {
     return 0;
   }
-
-  auto source = SourceBuffer::CreateFromText(
-      llvm::StringRef(reinterpret_cast<const char*>(data), size));
+  static constexpr llvm::StringLiteral TestFileName = "test.carbon";
+  llvm::vfs::InMemoryFileSystem fs;
+  llvm::StringRef data_ref(reinterpret_cast<const char*>(data), size);
+  CARBON_CHECK(fs.addFile(
+      TestFileName, /*ModificationTime=*/0,
+      llvm::MemoryBuffer::getMemBuffer(data_ref, /*BufferName=*/TestFileName,
+                                       /*RequiresNullTerminator=*/false)));
+  auto source = SourceBuffer::CreateFromFile(fs, TestFileName);
 
   // Lex the input.
   auto tokens = TokenizedBuffer::Lex(*source, NullDiagnosticConsumer());

+ 6 - 3
toolchain/parser/parse_tree_test.cpp

@@ -21,18 +21,21 @@ using ::testing::ElementsAre;
 
 class ParseTreeTest : public ::testing::Test {
  protected:
-  auto GetSourceBuffer(llvm::Twine t) -> SourceBuffer& {
+  auto GetSourceBuffer(llvm::StringRef t) -> SourceBuffer& {
+    CARBON_CHECK(fs.addFile("test.carbon", /*ModificationTime=*/0,
+                            llvm::MemoryBuffer::getMemBuffer(t)));
     source_storage.push_front(
-        std::move(*SourceBuffer::CreateFromText(t.str())));
+        std::move(*SourceBuffer::CreateFromFile(fs, "test.carbon")));
     return source_storage.front();
   }
 
-  auto GetTokenizedBuffer(llvm::Twine t) -> TokenizedBuffer& {
+  auto GetTokenizedBuffer(llvm::StringRef t) -> TokenizedBuffer& {
     token_storage.push_front(
         TokenizedBuffer::Lex(GetSourceBuffer(t), consumer));
     return token_storage.front();
   }
 
+  llvm::vfs::InMemoryFileSystem fs;
   std::forward_list<SourceBuffer> source_storage;
   std::forward_list<TokenizedBuffer> token_storage;
   DiagnosticConsumer& consumer = ConsoleDiagnosticConsumer();

+ 1 - 0
toolchain/semantics/BUILD

@@ -109,6 +109,7 @@ cc_test(
         "//toolchain/common:yaml_test_helpers",
         "//toolchain/diagnostics:diagnostic_emitter",
         "//toolchain/lexer:tokenized_buffer",
+        "//toolchain/source:source_buffer",
         "@com_google_googletest//:gtest",
         "@llvm-project//llvm:Support",
     ],

+ 6 - 2
toolchain/semantics/semantics_ir_test.cpp

@@ -12,6 +12,7 @@
 #include "toolchain/common/yaml_test_helpers.h"
 #include "toolchain/diagnostics/diagnostic_emitter.h"
 #include "toolchain/lexer/tokenized_buffer.h"
+#include "toolchain/source/source_buffer.h"
 
 namespace Carbon::Testing {
 namespace {
@@ -27,8 +28,11 @@ using ::testing::Pair;
 
 TEST(SemanticsIRTest, YAML) {
   DiagnosticConsumer& consumer = ConsoleDiagnosticConsumer();
-  llvm::Expected<SourceBuffer> source =
-      SourceBuffer::CreateFromText("var x: i32 = 0;");
+  llvm::vfs::InMemoryFileSystem fs;
+  CARBON_CHECK(fs.addFile("test.carbon", /*ModificationTime=*/0,
+                          llvm::MemoryBuffer::getMemBuffer("var x: i32 = 0;")));
+  ErrorOr<SourceBuffer> source =
+      SourceBuffer::CreateFromFile(fs, "test.carbon");
   TokenizedBuffer tokens = TokenizedBuffer::Lex(*source, consumer);
   ParseTree parse_tree =
       ParseTree::Parse(tokens, consumer, /*vlog_stream=*/nullptr);

+ 1 - 1
toolchain/source/BUILD

@@ -9,7 +9,7 @@ cc_library(
     srcs = ["source_buffer.cpp"],
     hdrs = ["source_buffer.h"],
     deps = [
-        "//common:check",
+        "//common:error",
         "@llvm-project//llvm:Support",
     ],
 )

+ 20 - 120
toolchain/source/source_buffer.cpp

@@ -4,139 +4,39 @@
 
 #include "toolchain/source/source_buffer.h"
 
-#include <fcntl.h>
-#include <sys/mman.h>
-#include <sys/stat.h>
-#include <unistd.h>
-
-#include <cerrno>
-#include <cstdint>
 #include <limits>
-#include <optional>
-#include <system_error>
-#include <utility>
-#include <variant>
 
-#include "common/check.h"
-#include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/FormatVariadic.h"
 
 namespace Carbon {
 
-// Verifies that the content size is within limits.
-static auto CheckContentSize(int64_t size) -> llvm::Error {
-  if (size < std::numeric_limits<int32_t>::max()) {
-    return llvm::Error::success();
-  }
-  return llvm::createStringError(llvm::inconvertibleErrorCode(),
-                                 "Input too large!");
-}
-
-auto SourceBuffer::CreateFromText(llvm::Twine text, llvm::StringRef filename)
-    -> llvm::Expected<SourceBuffer> {
-  std::string buffer = text.str();
-  auto size_check = CheckContentSize(buffer.size());
-  if (size_check) {
-    return std::move(size_check);
-  }
-  return SourceBuffer(filename.str(), std::move(buffer));
-}
-
-static auto ErrnoToError(int errno_value) -> llvm::Error {
-  return llvm::errorCodeToError(
-      std::error_code(errno_value, std::generic_category()));
-}
-
-auto SourceBuffer::CreateFromFile(llvm::StringRef filename)
-    -> llvm::Expected<SourceBuffer> {
-  // Add storage to ensure there's a nul-terminator for open().
-  std::string filename_str = filename.str();
-
-  errno = 0;
-  int file_descriptor = open(filename_str.c_str(), O_RDONLY);
-  if (file_descriptor == -1) {
-    return ErrnoToError(errno);
-  }
-
-  // Now that we have an open file, we need to close it on any error.
-  auto closer =
-      llvm::make_scope_exit([file_descriptor] { close(file_descriptor); });
-
-  struct stat stat_buffer = {};
-  errno = 0;
-  if (fstat(file_descriptor, &stat_buffer) == -1) {
-    return ErrnoToError(errno);
+auto SourceBuffer::CreateFromFile(llvm::vfs::FileSystem& fs,
+                                  llvm::StringRef filename)
+    -> ErrorOr<SourceBuffer> {
+  llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>> file =
+      fs.openFileForRead(filename);
+  if (file.getError()) {
+    return Error(file.getError().message());
   }
 
-  int64_t size = stat_buffer.st_size;
-  if (size == 0) {
-    // Rather than opening an empty file, create an empty buffer.
-    return SourceBuffer(std::move(filename_str), std::string());
-  }
-  auto size_check = CheckContentSize(size);
-  if (size_check) {
-    return std::move(size_check);
+  llvm::ErrorOr<llvm::vfs::Status> status = (*file)->status();
+  if (status.getError()) {
+    return Error(status.getError().message());
   }
-
-  errno = 0;
-  void* mapped_text = mmap(nullptr, size, PROT_READ,
-#if defined(__linux__)
-                           MAP_PRIVATE | MAP_POPULATE,
-#else
-                           MAP_PRIVATE,
-#endif
-                           file_descriptor, /*offset=*/0);
-  // The `MAP_FAILED` macro may expand to a cast to pointer that `clang-tidy`
-  // complains about.
-  // NOLINTNEXTLINE(performance-no-int-to-ptr)
-  if (mapped_text == MAP_FAILED) {
-    return ErrnoToError(errno);
+  auto size = status->getSize();
+  if (size >= std::numeric_limits<int32_t>::max()) {
+    return Error(
+        llvm::formatv("`{0}` is over the 2GiB input limit.", filename));
   }
 
-  errno = 0;
-  closer.release();
-  if (close(file_descriptor) == -1) {
-    // Try to unmap the text. No error handling as this is just best-effort
-    // cleanup.
-    munmap(mapped_text, size);
-    return ErrnoToError(errno);
+  llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> buffer =
+      (*file)->getBuffer(filename, size, /*RequiresNullTerminator=*/false);
+  if (buffer.getError()) {
+    return Error(buffer.getError().message());
   }
 
-  return SourceBuffer(
-      std::move(filename_str),
-      llvm::StringRef(static_cast<const char*>(mapped_text), size));
-}
-
-SourceBuffer::SourceBuffer(SourceBuffer&& arg) noexcept
-    // Sets Uninitialized to ensure the input doesn't release mmapped data.
-    : content_mode_(
-          std::exchange(arg.content_mode_, ContentMode::Uninitialized)),
-      filename_(std::move(arg.filename_)),
-      text_storage_(std::move(arg.text_storage_)),
-      text_(content_mode_ == ContentMode::Owned ? text_storage_ : arg.text_) {}
-
-SourceBuffer::SourceBuffer(std::string filename, std::string text)
-    : content_mode_(ContentMode::Owned),
-      filename_(std::move(filename)),
-      text_storage_(std::move(text)),
-      text_(text_storage_) {}
-
-SourceBuffer::SourceBuffer(std::string filename, llvm::StringRef text)
-    : content_mode_(ContentMode::MMapped),
-      filename_(std::move(filename)),
-      text_(text) {
-  CARBON_CHECK(!text.empty())
-      << "Must not have an empty text when we have mapped data from a file!";
-}
-
-SourceBuffer::~SourceBuffer() {
-  if (content_mode_ == ContentMode::MMapped) {
-    errno = 0;
-    int result =
-        munmap(const_cast<void*>(static_cast<const void*>(text_.data())),
-               text_.size());
-    CARBON_CHECK(result != -1) << "Unmapping text failed!";
-  }
+  return SourceBuffer(filename.str(), std::move(buffer.get()));
 }
 
 }  // namespace Carbon

+ 13 - 30
toolchain/source/source_buffer.h

@@ -5,12 +5,13 @@
 #ifndef CARBON_TOOLCHAIN_SOURCE_SOURCE_BUFFER_H_
 #define CARBON_TOOLCHAIN_SOURCE_SOURCE_BUFFER_H_
 
+#include <memory>
 #include <string>
-#include <utility>
 
+#include "common/error.h"
 #include "llvm/ADT/StringRef.h"
-#include "llvm/ADT/Twine.h"
-#include "llvm/Support/Error.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/VirtualFileSystem.h"
 
 namespace Carbon {
 
@@ -33,43 +34,25 @@ namespace Carbon {
 // some implementation complexity in the future if needed.
 class SourceBuffer {
  public:
-  static auto CreateFromText(llvm::Twine text,
-                             llvm::StringRef filename = "/text")
-      -> llvm::Expected<SourceBuffer>;
-  static auto CreateFromFile(llvm::StringRef filename)
-      -> llvm::Expected<SourceBuffer>;
+  static auto CreateFromFile(llvm::vfs::FileSystem& fs,
+                             llvm::StringRef filename) -> ErrorOr<SourceBuffer>;
 
   // Use one of the factory functions above to create a source buffer.
   SourceBuffer() = delete;
 
-  // Cannot copy as there may be non-trivial owned file data; see the class
-  // comment for details.
-  SourceBuffer(const SourceBuffer& arg) = delete;
-
-  SourceBuffer(SourceBuffer&& arg) noexcept;
-
-  ~SourceBuffer();
-
   [[nodiscard]] auto filename() const -> llvm::StringRef { return filename_; }
 
-  [[nodiscard]] auto text() const -> llvm::StringRef { return text_; }
+  [[nodiscard]] auto text() const -> llvm::StringRef {
+    return text_->getBuffer();
+  }
 
  private:
-  enum class ContentMode {
-    Uninitialized,
-    MMapped,
-    Owned,
-  };
-
-  // Constructor for mmapped content.
-  explicit SourceBuffer(std::string filename, llvm::StringRef text);
-  // Constructor for owned content.
-  explicit SourceBuffer(std::string filename, std::string text);
+  explicit SourceBuffer(std::string filename,
+                        std::unique_ptr<llvm::MemoryBuffer> text)
+      : filename_(std::move(filename)), text_(std::move(text)) {}
 
-  ContentMode content_mode_;
   std::string filename_;
-  std::string text_storage_;
-  llvm::StringRef text_;
+  std::unique_ptr<llvm::MemoryBuffer> text_;
 };
 
 }  // namespace Carbon

+ 36 - 46
toolchain/source/source_buffer_test.cpp

@@ -7,68 +7,58 @@
 #include <gtest/gtest.h>
 
 #include "llvm/ADT/SmallString.h"
-#include "llvm/ADT/Twine.h"
-#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace Carbon::Testing {
 namespace {
 
-TEST(SourceBufferTest, StringRep) {
-  auto buffer = SourceBuffer::CreateFromText(llvm::Twine("Hello") + " World");
-  EXPECT_EQ("/text", buffer->filename());
-  EXPECT_EQ("Hello World", buffer->text());
-}
+static constexpr llvm::StringLiteral TestFileName = "test.carbon";
 
-TEST(SourceBufferText, StringRepWithFilename) {
-  // Give a custom filename.
-  auto buffer =
-      SourceBuffer::CreateFromText("Hello World Again!", "/custom/text");
-  EXPECT_EQ("/custom/text", buffer->filename());
-  EXPECT_EQ("Hello World Again!", buffer->text());
+TEST(SourceBufferTest, MissingFile) {
+  llvm::vfs::InMemoryFileSystem fs;
+  auto buffer = SourceBuffer::CreateFromFile(fs, TestFileName);
+  EXPECT_FALSE(buffer.ok());
 }
 
-auto CreateTestFile(llvm::StringRef text) -> std::string {
-  int fd = -1;
-  llvm::SmallString<1024> path;
-  auto error_code =
-      llvm::sys::fs::createTemporaryFile("test_file", ".txt", fd, path);
-  if (error_code) {
-    llvm::report_fatal_error(llvm::Twine("Failed to create temporary file: ") +
-                             error_code.message());
-  }
-
-  llvm::raw_fd_ostream out_stream(fd, /*shouldClose=*/true);
-  out_stream << text;
-  out_stream.close();
-
-  return path.str().str();
-}
+TEST(SourceBufferTest, SimpleFile) {
+  llvm::vfs::InMemoryFileSystem fs;
+  CARBON_CHECK(fs.addFile(TestFileName, /*ModificationTime=*/0,
+                          llvm::MemoryBuffer::getMemBuffer("Hello World")));
 
-TEST(SourceBufferTest, FileRep) {
-  auto test_file_path = CreateTestFile("Hello World");
+  auto buffer = SourceBuffer::CreateFromFile(fs, TestFileName);
+  ASSERT_TRUE(buffer.ok()) << "Error message: " << buffer.error();
 
-  auto expected_buffer = SourceBuffer::CreateFromFile(test_file_path);
-  ASSERT_TRUE(static_cast<bool>(expected_buffer))
-      << "Error message: " << toString(expected_buffer.takeError());
+  EXPECT_EQ(TestFileName, buffer->filename());
+  EXPECT_EQ("Hello World", buffer->text());
+}
 
-  SourceBuffer& buffer = *expected_buffer;
+TEST(SourceBufferTest, NoNull) {
+  llvm::vfs::InMemoryFileSystem fs;
+  static constexpr char NoNull[3] = {'a', 'b', 'c'};
+  CARBON_CHECK(fs.addFile(
+      TestFileName, /*ModificationTime=*/0,
+      llvm::MemoryBuffer::getMemBuffer(llvm::StringRef(NoNull, sizeof(NoNull)),
+                                       /*BufferName=*/"",
+                                       /*RequiresNullTerminator=*/false)));
 
-  EXPECT_EQ(test_file_path, buffer.filename());
-  EXPECT_EQ("Hello World", buffer.text());
-}
+  auto buffer = SourceBuffer::CreateFromFile(fs, TestFileName);
+  ASSERT_TRUE(buffer.ok()) << "Error message: " << buffer.error();
 
-TEST(SourceBufferTest, FileRepEmpty) {
-  auto test_file_path = CreateTestFile("");
+  EXPECT_EQ(TestFileName, buffer->filename());
+  EXPECT_EQ("abc", buffer->text());
+}
 
-  auto expected_buffer = SourceBuffer::CreateFromFile(test_file_path);
-  ASSERT_TRUE(static_cast<bool>(expected_buffer))
-      << "Error message: " << toString(expected_buffer.takeError());
+TEST(SourceBufferTest, EmptyFile) {
+  llvm::vfs::InMemoryFileSystem fs;
+  CARBON_CHECK(fs.addFile(TestFileName, /*ModificationTime=*/0,
+                          llvm::MemoryBuffer::getMemBuffer("")));
 
-  SourceBuffer& buffer = *expected_buffer;
+  auto buffer = SourceBuffer::CreateFromFile(fs, TestFileName);
+  ASSERT_TRUE(buffer.ok()) << "Error message: " << buffer.error();
 
-  EXPECT_EQ(test_file_path, buffer.filename());
-  EXPECT_EQ("", buffer.text());
+  EXPECT_EQ(TestFileName, buffer->filename());
+  EXPECT_EQ("", buffer->text());
 }
 
 }  // namespace