From a93e621488fb2040371c6358c9c0d591c09e9f12 Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Mon, 12 Jun 2023 13:19:57 -0700 Subject: [PATCH] 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 --- toolchain/driver/driver.cpp | 14 +- toolchain/driver/driver.h | 13 +- toolchain/driver/driver_file_test_base.h | 40 ++--- toolchain/driver/driver_fuzzer.cpp | 4 +- toolchain/driver/driver_main.cpp | 3 +- toolchain/driver/driver_test.cpp | 170 +++++++++----------- toolchain/lexer/tokenized_buffer.cpp | 6 +- toolchain/lexer/tokenized_buffer_fuzzer.cpp | 10 +- toolchain/lexer/tokenized_buffer_test.cpp | 18 ++- toolchain/parser/parse_tree_fuzzer.cpp | 11 +- toolchain/parser/parse_tree_test.cpp | 9 +- toolchain/semantics/BUILD | 1 + toolchain/semantics/semantics_ir_test.cpp | 8 +- toolchain/source/BUILD | 2 +- toolchain/source/source_buffer.cpp | 142 +++------------- toolchain/source/source_buffer.h | 43 ++--- toolchain/source/source_buffer_test.cpp | 90 +++++------ 17 files changed, 217 insertions(+), 367 deletions(-) diff --git a/toolchain/driver/driver.cpp b/toolchain/driver/driver.cpp index 123f9965ad63..8e10a4659162 100644 --- a/toolchain/driver/driver.cpp +++ b/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; } diff --git a/toolchain/driver/driver.h b/toolchain/driver/driver.h index 694e9452c794..898e22f74a9f 100644 --- a/toolchain/driver/driver.h +++ b/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 args) -> void; + llvm::vfs::FileSystem& fs_; llvm::raw_ostream& output_stream_; llvm::raw_ostream& error_stream_; llvm::raw_ostream* vlog_stream_ = nullptr; diff --git a/toolchain/driver/driver_file_test_base.h b/toolchain/driver/driver_file_test_base.h index 63d3b34d3113..c0a16986dfd8 100644 --- a/toolchain/driver/driver_file_test_base.h +++ b/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 #include #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& 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 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; + if (!fs.addFile(test_file.filename, /*ModificationTime=*/0, + llvm::MemoryBuffer::getMemBuffer(test_file.content))) { + ADD_FAILURE() << "File is repeated: " << test_file.filename; + return false; + } } - 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); + Driver driver(fs, stdout, stderr); return driver.RunFullCommand(MakeArgs(test_file_names)); } diff --git a/toolchain/driver/driver_fuzzer.cpp b/toolchain/driver/driver_fuzzer.cpp index 012cf946bfef..610d4b8e6262 100644 --- a/toolchain/driver/driver_fuzzer.cpp +++ b/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) { diff --git a/toolchain/driver/driver_main.cpp b/toolchain/driver/driver_main.cpp index 1c916bd3eddc..7d5853cb1fdb 100644 --- a/toolchain/driver/driver_main.cpp +++ b/toolchain/driver/driver_main.cpp @@ -28,7 +28,8 @@ auto main(int argc, char** argv) -> int { llvm::errs().tie(&llvm::outs()); llvm::SmallVector 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; } diff --git a/toolchain/driver/driver_test.cpp b/toolchain/driver/driver_test.cpp index c4f06cb50e03..ae7b9792d2c4 100644 --- a/toolchain/driver/driver_test.cpp +++ b/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")); + + EXPECT_FALSE(driver_.RunFullCommand({"foo --bar --baz"})); + 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_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")); + + EXPECT_FALSE( + driver_.RunHelpSubcommand(ConsoleDiagnosticConsumer(), {"--xyz"})); + 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(); -} - -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")); + 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")); + 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 diff --git a/toolchain/lexer/tokenized_buffer.cpp b/toolchain/lexer/tokenized_buffer.cpp index 41ff40f8269c..c17799e53cc0 100644 --- a/toolchain/lexer/tokenized_buffer.cpp +++ b/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), diff --git a/toolchain/lexer/tokenized_buffer_fuzzer.cpp b/toolchain/lexer/tokenized_buffer_fuzzer.cpp index 5c88e8c5f30b..302bba0a5602 100644 --- a/toolchain/lexer/tokenized_buffer_fuzzer.cpp +++ b/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(data), size)); + static constexpr llvm::StringLiteral TestFileName = "test.carbon"; + llvm::vfs::InMemoryFileSystem fs; + llvm::StringRef data_ref(reinterpret_cast(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()) { diff --git a/toolchain/lexer/tokenized_buffer_test.cpp b/toolchain/lexer/tokenized_buffer_test.cpp index 73c90f180d8a..50ed849d8aeb 100644 --- a/toolchain/lexer/tokenized_buffer_test.cpp +++ b/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 source_storage; + llvm::vfs::InMemoryFileSystem fs; + int file_index = 0; + std::forward_list source_storage; }; TEST_F(LexerTest, HandlesEmptyBuffer) { diff --git a/toolchain/parser/parse_tree_fuzzer.cpp b/toolchain/parser/parse_tree_fuzzer.cpp index 7dd1404a9086..0fd150970665 100644 --- a/toolchain/parser/parse_tree_fuzzer.cpp +++ b/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(data), size)); + static constexpr llvm::StringLiteral TestFileName = "test.carbon"; + llvm::vfs::InMemoryFileSystem fs; + llvm::StringRef data_ref(reinterpret_cast(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()); diff --git a/toolchain/parser/parse_tree_test.cpp b/toolchain/parser/parse_tree_test.cpp index a9db43a5623d..65ec2193e23b 100644 --- a/toolchain/parser/parse_tree_test.cpp +++ b/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 source_storage; std::forward_list token_storage; DiagnosticConsumer& consumer = ConsoleDiagnosticConsumer(); diff --git a/toolchain/semantics/BUILD b/toolchain/semantics/BUILD index 55392afb1484..f26e4239a31c 100644 --- a/toolchain/semantics/BUILD +++ b/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", ], diff --git a/toolchain/semantics/semantics_ir_test.cpp b/toolchain/semantics/semantics_ir_test.cpp index b4bbfdb82142..3134f3c2c4b7 100644 --- a/toolchain/semantics/semantics_ir_test.cpp +++ b/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 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 source = + SourceBuffer::CreateFromFile(fs, "test.carbon"); TokenizedBuffer tokens = TokenizedBuffer::Lex(*source, consumer); ParseTree parse_tree = ParseTree::Parse(tokens, consumer, /*vlog_stream=*/nullptr); diff --git a/toolchain/source/BUILD b/toolchain/source/BUILD index b4ea1feea030..a33bb89400e4 100644 --- a/toolchain/source/BUILD +++ b/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", ], ) diff --git a/toolchain/source/source_buffer.cpp b/toolchain/source/source_buffer.cpp index 31b810f3bca7..872e3b50cfd2 100644 --- a/toolchain/source/source_buffer.cpp +++ b/toolchain/source/source_buffer.cpp @@ -4,139 +4,39 @@ #include "toolchain/source/source_buffer.h" -#include -#include -#include -#include - -#include -#include #include -#include -#include -#include -#include -#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::max()) { - return llvm::Error::success(); - } - return llvm::createStringError(llvm::inconvertibleErrorCode(), - "Input too large!"); -} - -auto SourceBuffer::CreateFromText(llvm::Twine text, llvm::StringRef filename) - -> llvm::Expected { - 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 { - // 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); +auto SourceBuffer::CreateFromFile(llvm::vfs::FileSystem& fs, + llvm::StringRef filename) + -> ErrorOr { + llvm::ErrorOr> file = + fs.openFileForRead(filename); + if (file.getError()) { + return Error(file.getError().message()); } - // 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); + llvm::ErrorOr status = (*file)->status(); + if (status.getError()) { + return Error(status.getError().message()); + } + auto size = status->getSize(); + if (size >= std::numeric_limits::max()) { + return Error( + llvm::formatv("`{0}` is over the 2GiB input limit.", filename)); } - 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> buffer = + (*file)->getBuffer(filename, size, /*RequiresNullTerminator=*/false); + if (buffer.getError()) { + return Error(buffer.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); - } - - 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); - } - - return SourceBuffer( - std::move(filename_str), - llvm::StringRef(static_cast(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(static_cast(text_.data())), - text_.size()); - CARBON_CHECK(result != -1) << "Unmapping text failed!"; - } + return SourceBuffer(filename.str(), std::move(buffer.get())); } } // namespace Carbon diff --git a/toolchain/source/source_buffer.h b/toolchain/source/source_buffer.h index 69a5993b7718..cd30280642c9 100644 --- a/toolchain/source/source_buffer.h +++ b/toolchain/source/source_buffer.h @@ -5,12 +5,13 @@ #ifndef CARBON_TOOLCHAIN_SOURCE_SOURCE_BUFFER_H_ #define CARBON_TOOLCHAIN_SOURCE_SOURCE_BUFFER_H_ +#include #include -#include +#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; - static auto CreateFromFile(llvm::StringRef filename) - -> llvm::Expected; + static auto CreateFromFile(llvm::vfs::FileSystem& fs, + llvm::StringRef filename) -> ErrorOr; // 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, - }; + explicit SourceBuffer(std::string filename, + std::unique_ptr text) + : filename_(std::move(filename)), text_(std::move(text)) {} - // Constructor for mmapped content. - explicit SourceBuffer(std::string filename, llvm::StringRef text); - // Constructor for owned content. - explicit SourceBuffer(std::string filename, std::string text); - - ContentMode content_mode_; std::string filename_; - std::string text_storage_; - llvm::StringRef text_; + std::unique_ptr text_; }; } // namespace Carbon diff --git a/toolchain/source/source_buffer_test.cpp b/toolchain/source/source_buffer_test.cpp index c7b305c3ea15..2b58215b9bf2 100644 --- a/toolchain/source/source_buffer_test.cpp +++ b/toolchain/source/source_buffer_test.cpp @@ -7,68 +7,58 @@ #include #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()); +static constexpr llvm::StringLiteral TestFileName = "test.carbon"; + +TEST(SourceBufferTest, MissingFile) { + llvm::vfs::InMemoryFileSystem fs; + auto buffer = SourceBuffer::CreateFromFile(fs, TestFileName); + EXPECT_FALSE(buffer.ok()); +} + +TEST(SourceBufferTest, SimpleFile) { + llvm::vfs::InMemoryFileSystem fs; + CARBON_CHECK(fs.addFile(TestFileName, /*ModificationTime=*/0, + llvm::MemoryBuffer::getMemBuffer("Hello World"))); + + auto buffer = SourceBuffer::CreateFromFile(fs, TestFileName); + ASSERT_TRUE(buffer.ok()) << "Error message: " << buffer.error(); + + EXPECT_EQ(TestFileName, buffer->filename()); EXPECT_EQ("Hello World", buffer->text()); } -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, 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))); + + auto buffer = SourceBuffer::CreateFromFile(fs, TestFileName); + ASSERT_TRUE(buffer.ok()) << "Error message: " << buffer.error(); + + EXPECT_EQ(TestFileName, buffer->filename()); + EXPECT_EQ("abc", buffer->text()); } -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()); - } +TEST(SourceBufferTest, EmptyFile) { + llvm::vfs::InMemoryFileSystem fs; + CARBON_CHECK(fs.addFile(TestFileName, /*ModificationTime=*/0, + llvm::MemoryBuffer::getMemBuffer(""))); - llvm::raw_fd_ostream out_stream(fd, /*shouldClose=*/true); - out_stream << text; - out_stream.close(); + auto buffer = SourceBuffer::CreateFromFile(fs, TestFileName); + ASSERT_TRUE(buffer.ok()) << "Error message: " << buffer.error(); - return path.str().str(); -} - -TEST(SourceBufferTest, FileRep) { - auto test_file_path = CreateTestFile("Hello World"); - - auto expected_buffer = SourceBuffer::CreateFromFile(test_file_path); - ASSERT_TRUE(static_cast(expected_buffer)) - << "Error message: " << toString(expected_buffer.takeError()); - - SourceBuffer& buffer = *expected_buffer; - - EXPECT_EQ(test_file_path, buffer.filename()); - EXPECT_EQ("Hello World", buffer.text()); -} - -TEST(SourceBufferTest, FileRepEmpty) { - auto test_file_path = CreateTestFile(""); - - auto expected_buffer = SourceBuffer::CreateFromFile(test_file_path); - ASSERT_TRUE(static_cast(expected_buffer)) - << "Error message: " << toString(expected_buffer.takeError()); - - SourceBuffer& buffer = *expected_buffer; - - EXPECT_EQ(test_file_path, buffer.filename()); - EXPECT_EQ("", buffer.text()); + EXPECT_EQ(TestFileName, buffer->filename()); + EXPECT_EQ("", buffer->text()); } } // namespace