From 9ac92ad71b2b8bfe469133d32fe0c3809543b012 Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Wed, 6 Sep 2023 14:34:59 -0700 Subject: [PATCH] Add support for compiling multiple files at once. (#3182) Rearranges driver logic into CompilationUnits in order to associate artifacts from the various stages of compilation. Note, I'm not totally sure what the right thing to do is for lower/codegen, so I'm just doing a rote change there for now that mirrors prior phases (this is all the code supports anyways, so is probably right for now regardless). SourceBuffer error output is moved local for consistency with other steps, and so that it's less ambiguous whether the error should be expected to already include a filename. --- language_server/language_server.cpp | 2 +- toolchain/driver/driver.cpp | 383 +++++++++++------- toolchain/driver/driver.h | 1 + .../testdata/fail_errors_in_two_files.carbon | 22 + toolchain/lex/tokenized_buffer_benchmark.cpp | 3 +- toolchain/lex/tokenized_buffer_fuzzer.cpp | 2 +- toolchain/lex/tokenized_buffer_test.cpp | 2 +- toolchain/parse/parse_fuzzer.cpp | 2 +- toolchain/parse/tree_test.cpp | 4 +- toolchain/source/BUILD | 1 + toolchain/source/source_buffer.cpp | 21 +- toolchain/source/source_buffer.h | 8 +- toolchain/source/source_buffer_test.cpp | 17 +- 13 files changed, 299 insertions(+), 169 deletions(-) create mode 100644 toolchain/driver/testdata/fail_errors_in_two_files.carbon diff --git a/language_server/language_server.cpp b/language_server/language_server.cpp index 564b0ca15e47..4444055f03e7 100644 --- a/language_server/language_server.cpp +++ b/language_server/language_server.cpp @@ -94,7 +94,7 @@ void LanguageServer::OnDocumentSymbol( vfs.addFile(file, /*mtime=*/0, llvm::MemoryBuffer::getMemBufferCopy(files_.at(file))); - auto buf = SourceBuffer::CreateFromFile(vfs, file); + auto buf = SourceBuffer::CreateFromFile(vfs, llvm::nulls(), file); auto lexed = Lex::TokenizedBuffer::Lex(*buf, NullDiagnosticConsumer()); auto parsed = Parse::Tree::Parse(lexed, NullDiagnosticConsumer(), nullptr); std::vector result; diff --git a/toolchain/driver/driver.cpp b/toolchain/driver/driver.cpp index 39ff4f971cec..6958d59448ea 100644 --- a/toolchain/driver/driver.cpp +++ b/toolchain/driver/driver.cpp @@ -4,9 +4,13 @@ #include "toolchain/driver/driver.h" +#include +#include + #include "common/command_line.h" #include "common/vlog.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/IR/LLVMContext.h" @@ -80,7 +84,7 @@ The input Carbon source file to compile. }, [&](auto& arg_b) { arg_b.Required(true); - arg_b.Set(&input_file_name); + arg_b.Append(&input_file_names); }); b.AddOneOfOption( @@ -249,7 +253,7 @@ Dump the generated assembly to stdout after codegen. llvm::StringRef target; llvm::StringRef output_file_name; - llvm::StringRef input_file_name; + llvm::SmallVector input_file_names; bool asm_output = false; bool force_obj_output = false; @@ -376,179 +380,268 @@ auto Driver::ValidateCompileOptions(const CompileOptions& options) const return true; } -auto Driver::Compile(const CompileOptions& options) -> bool { - using Phase = CompileOptions::Phase; - - if (!ValidateCompileOptions(options)) { - return false; +// Ties together information for a file being compiled. +class Driver::CompilationUnit { + public: + explicit CompilationUnit(Driver* driver, const CompileOptions& options, + llvm::StringRef input_file_name) + : driver_(driver), + options_(options), + input_file_name_(input_file_name), + vlog_stream_(driver_->vlog_stream_), + stream_consumer_(driver_->error_stream_) { + if (vlog_stream_ != nullptr || options_.stream_errors) { + consumer_ = &stream_consumer_; + } else { + sorting_consumer_ = SortingDiagnosticConsumer(stream_consumer_); + consumer_ = &*sorting_consumer_; + } } - StreamDiagnosticConsumer stream_consumer(error_stream_); - DiagnosticConsumer* consumer = &stream_consumer; - - // Note, the diagnostics consumer must be flushed before each `return` in this - // function, as diagnostics can refer to state that lives on our stack. - std::unique_ptr sorting_consumer; - if (vlog_stream_ == nullptr && !options.stream_errors) { - sorting_consumer = std::make_unique(*consumer); - consumer = sorting_consumer.get(); + // Loads source and lexes it. Returns true on success. + auto RunLex() -> bool { + LogCall("SourceBuffer::CreateFromFile", [&] { + source_ = SourceBuffer::CreateFromFile( + driver_->fs_, driver_->error_stream_, input_file_name_); + }); + if (!source_) { + return false; + } + CARBON_VLOG() << "*** SourceBuffer ***\n```\n" + << source_->text() << "\n```\n"; + + LogCall("Lex::TokenizedBuffer::Lex", + [&] { tokens_ = Lex::TokenizedBuffer::Lex(*source_, *consumer_); }); + if (options_.dump_tokens) { + consumer_->Flush(); + driver_->output_stream_ << tokens_; + } + CARBON_VLOG() << "*** Lex::TokenizedBuffer ***\n" << tokens_; + return !tokens_->has_errors(); } - CARBON_VLOG() << "*** SourceBuffer::CreateFromFile on '" - << options.input_file_name << "' ***\n"; - auto source = SourceBuffer::CreateFromFile(fs_, options.input_file_name); - CARBON_VLOG() << "*** SourceBuffer::CreateFromFile done ***\n"; - if (!source.ok()) { - error_stream_ << "ERROR: Unable to open input source file: " - << source.error(); - consumer->Flush(); - return false; - } - CARBON_VLOG() << "*** file:\n```\n" << source->text() << "\n```\n"; - - CARBON_VLOG() << "*** Lex::TokenizedBuffer::Lex ***\n"; - auto tokenized_source = Lex::TokenizedBuffer::Lex(*source, *consumer); - bool has_errors = tokenized_source.has_errors(); - CARBON_VLOG() << "*** Lex::TokenizedBuffer::Lex done ***\n"; - if (options.dump_tokens) { - CARBON_VLOG() << "Finishing output."; - consumer->Flush(); - output_stream_ << tokenized_source; + // Parses tokens. Returns true on success. + auto RunParse() -> bool { + LogCall("Parse::Tree::Parse", [&] { + parse_tree_ = Parse::Tree::Parse(*tokens_, *consumer_, vlog_stream_); + }); + if (options_.dump_parse_tree) { + consumer_->Flush(); + parse_tree_->Print(driver_->output_stream_, options_.preorder_parse_tree); + } + CARBON_VLOG() << "*** Parse::Tree ***\n" << parse_tree_; + return !parse_tree_->has_errors(); } - CARBON_VLOG() << "tokenized_buffer: " << tokenized_source; - if (options.phase == Phase::Lex) { - consumer->Flush(); - return !has_errors; + + // Check the parse tree and produce SemIR. Returns true on success. + auto RunCheck(const SemIR::File& builtins) -> bool { + LogCall("Check::CheckParseTree", [&] { + sem_ir_ = Check::CheckParseTree(builtins, *tokens_, *parse_tree_, + *consumer_, vlog_stream_); + }); + + // We've finished all steps that can produce diagnostics. Emit the + // diagnostics now, so that the developer sees them sooner and doesn't need + // to wait for code generation. + consumer_->Flush(); + + CARBON_VLOG() << "*** Raw SemIR::File ***\n" << *sem_ir_ << "\n"; + if (options_.dump_raw_semantics_ir) { + sem_ir_->Print(driver_->output_stream_, options_.builtin_semantics_ir); + if (options_.dump_semantics_ir) { + driver_->output_stream_ << "\n"; + } + } + + if (vlog_stream_) { + CARBON_VLOG() << "*** SemIR::File ***\n"; + SemIR::FormatFile(*tokens_, *parse_tree_, *sem_ir_, *vlog_stream_); + } + if (options_.dump_semantics_ir) { + SemIR::FormatFile(*tokens_, *parse_tree_, *sem_ir_, + driver_->output_stream_); + } + return !sem_ir_->has_errors(); } - CARBON_VLOG() << "*** Parse::Tree::Parse ***\n"; - auto parse_tree = - Parse::Tree::Parse(tokenized_source, *consumer, vlog_stream_); - has_errors |= parse_tree.has_errors(); - CARBON_VLOG() << "*** Parse::Tree::Parse done ***\n"; - if (options.dump_parse_tree) { - consumer->Flush(); - parse_tree.Print(output_stream_, options.preorder_parse_tree); + // Lower SemIR to LLVM IR. + auto RunLower() -> void { + LogCall("Lower::LowerToLLVM", [&] { + llvm_context_ = std::make_unique(); + module_ = Lower::LowerToLLVM(*llvm_context_, input_file_name_, *sem_ir_, + vlog_stream_); + }); + if (vlog_stream_) { + CARBON_VLOG() << "*** llvm::Module ***\n"; + module_->print(*vlog_stream_, /*AAW=*/nullptr, + /*ShouldPreserveUseListOrder=*/false, + /*IsForDebug=*/true); + } + if (options_.dump_llvm_ir) { + module_->print(driver_->output_stream_, /*AAW=*/nullptr, + /*ShouldPreserveUseListOrder=*/true); + } } - CARBON_VLOG() << "parse_tree: " << parse_tree; - if (options.phase == Phase::Parse) { - consumer->Flush(); - return !has_errors; + + // Do codegen. Returns true on success. + auto RunCodeGen() -> bool { + CARBON_VLOG() << "*** CodeGen ***\n"; + std::optional codegen = + CodeGen::Create(*module_, options_.target, driver_->error_stream_); + if (!codegen) { + return false; + } + if (vlog_stream_) { + CARBON_VLOG() << "*** Assembly ***\n"; + codegen->EmitAssembly(*vlog_stream_); + } + + if (options_.output_file_name == "-") { + // TODO: the output file name, forcing object output, and requesting + // textual assembly output are all somewhat linked flags. We should add + // some validation that they are used correctly. + if (options_.force_obj_output) { + if (!codegen->EmitObject(driver_->output_stream_)) { + return false; + } + } else { + if (!codegen->EmitAssembly(driver_->output_stream_)) { + return false; + } + } + } else { + llvm::SmallString<256> output_file_name = options_.output_file_name; + if (output_file_name.empty()) { + output_file_name = input_file_name_; + llvm::sys::path::replace_extension(output_file_name, + options_.asm_output ? ".s" : ".o"); + } + CARBON_VLOG() << "Writing output to: " << output_file_name << "\n"; + + std::error_code ec; + llvm::raw_fd_ostream output_file(output_file_name, ec, + llvm::sys::fs::OF_None); + if (ec) { + driver_->error_stream_ << "ERROR: Could not open output file '" + << output_file_name << "': " << ec.message() + << "\n"; + return false; + } + if (options_.asm_output) { + if (!codegen->EmitAssembly(output_file)) { + return false; + } + } else { + if (!codegen->EmitObject(output_file)) { + return false; + } + } + } + CARBON_VLOG() << "*** CodeGen done ***\n"; + return true; } - const SemIR::File builtin_ir = Check::MakeBuiltins(); - CARBON_VLOG() << "*** SemanticsIR::MakeFromParseTree ***\n"; - const SemIR::File semantics_ir = Check::CheckParseTree( - builtin_ir, tokenized_source, parse_tree, *consumer, vlog_stream_); + // Flushes output. + auto Flush() -> void { consumer_->Flush(); } - // We've finished all steps that can produce diagnostics. Emit the - // diagnostics now, so that the developer sees them sooner and doesn't need - // to wait for code generation. - consumer->Flush(); + private: + // Wraps a call with log statements to indicate start and end. + auto LogCall(llvm::StringLiteral label, llvm::function_ref fn) + -> void { + CARBON_VLOG() << "*** " << label << ": " << input_file_name_ << " ***\n"; + fn(); + CARBON_VLOG() << "*** " << label << " done ***\n"; + } + + Driver* driver_; + const CompileOptions& options_; + llvm::StringRef input_file_name_; + + // Copied from driver_ for CARBON_VLOG. + llvm::raw_pwrite_stream* vlog_stream_; + + // Diagnostics are sent to consumer_, with optional sorting. + StreamDiagnosticConsumer stream_consumer_; + std::optional sorting_consumer_; + DiagnosticConsumer* consumer_; + + // These are initialized as steps are run. + std::optional source_; + std::optional tokens_; + std::optional parse_tree_; + std::optional sem_ir_; + std::unique_ptr llvm_context_; + std::unique_ptr module_; +}; - has_errors |= semantics_ir.has_errors(); - CARBON_VLOG() << "*** SemIR::File::MakeFromParseTree done ***\n"; +auto Driver::Compile(const CompileOptions& options) -> bool { + if (!ValidateCompileOptions(options)) { + return false; + } - CARBON_VLOG() << "*** raw semantics_ir ***\n" << semantics_ir << "\n"; - if (options.dump_raw_semantics_ir) { - semantics_ir.Print(output_stream_, options.builtin_semantics_ir); - if (options.dump_semantics_ir) { - output_stream_ << "\n"; + llvm::SmallVector> units; + auto flush = llvm::make_scope_exit([&]() { + // The diagnostics consumer must be flushed before compilation artifacts are + // destructed, because diagnostics can refer to their state. This ensures + // they're flushed in order of arguments, rather than order of destruction. + for (auto& unit : units) { + unit->Flush(); } + }); + for (const auto& input_file_name : options.input_file_names) { + units.push_back( + std::make_unique(this, options, input_file_name)); } - if (vlog_stream_) { - CARBON_VLOG() << "*** semantics_ir ***\n"; - SemIR::FormatFile(tokenized_source, parse_tree, semantics_ir, - *vlog_stream_); + // Lex. + bool success_before_lower = true; + for (auto& unit : units) { + success_before_lower &= unit->RunLex(); } - if (options.dump_semantics_ir) { - SemIR::FormatFile(tokenized_source, parse_tree, semantics_ir, - output_stream_); + if (options.phase == CompileOptions::Phase::Lex) { + return success_before_lower; } - if (options.phase == Phase::Check) { - return !has_errors; + // Parse. + for (auto& unit : units) { + success_before_lower &= unit->RunParse(); } - - // Unlike previous steps, errors block further progress. - if (has_errors) { - CARBON_VLOG() << "*** Stopping before lowering due to syntax errors ***"; - return false; + if (options.phase == CompileOptions::Phase::Parse) { + return success_before_lower; } - CARBON_VLOG() << "*** Lower::LowerToLLVM ***\n"; - llvm::LLVMContext llvm_context; - const std::unique_ptr module = Lower::LowerToLLVM( - llvm_context, options.input_file_name, semantics_ir, vlog_stream_); - CARBON_VLOG() << "*** Lower::LowerToLLVM done ***\n"; - if (options.dump_llvm_ir) { - module->print(output_stream_, /*AAW=*/nullptr, - /*ShouldPreserveUseListOrder=*/true); - } - if (vlog_stream_) { - CARBON_VLOG() << "module: "; - module->print(*vlog_stream_, /*AAW=*/nullptr, - /*ShouldPreserveUseListOrder=*/false, - /*IsForDebug=*/true); + // Check. + auto builtins = Check::MakeBuiltins(); + // TODO: Organize units to compile in dependency order. + for (auto& unit : units) { + success_before_lower &= unit->RunCheck(builtins); } - if (options.phase == Phase::Lower) { - return true; + if (options.phase == CompileOptions::Phase::Check) { + return success_before_lower; } - CARBON_VLOG() << "*** CodeGen ***\n"; - std::optional codegen = - CodeGen::Create(*module, options.target, error_stream_); - if (!codegen) { + // Unlike previous steps, errors block further progress. + if (!success_before_lower) { + CARBON_VLOG() << "*** Stopping before lowering due to errors ***"; return false; } - if (vlog_stream_) { - CARBON_VLOG() << "assembly:\n"; - codegen->EmitAssembly(*vlog_stream_); + + // Lower. + for (auto& unit : units) { + unit->RunLower(); + } + if (options.phase == CompileOptions::Phase::Lower) { + return true; } + CARBON_CHECK(options.phase == CompileOptions::Phase::CodeGen) + << "CodeGen should be the last stage"; - if (options.output_file_name == "-") { - // TODO: the output file name, forcing object output, and requesting textual - // assembly output are all somewhat linked flags. We should add some - // validation that they are used correctly. - if (options.force_obj_output) { - if (!codegen->EmitObject(output_stream_)) { - return false; - } - } else { - if (!codegen->EmitAssembly(output_stream_)) { - return false; - } - } - } else { - llvm::SmallString<256> output_file_name = options.output_file_name; - if (output_file_name.empty()) { - output_file_name = options.input_file_name; - llvm::sys::path::replace_extension(output_file_name, - options.asm_output ? ".s" : ".o"); - } - CARBON_VLOG() << "Writing output to: " << output_file_name << "\n"; - - std::error_code ec; - llvm::raw_fd_ostream output_file(output_file_name, ec, - llvm::sys::fs::OF_None); - if (ec) { - error_stream_ << "ERROR: Could not open output file '" << output_file_name - << "': " << ec.message() << "\n"; - return false; - } - if (options.asm_output) { - if (!codegen->EmitAssembly(output_file)) { - return false; - } - } else { - if (!codegen->EmitObject(output_file)) { - return false; - } - } + // Codegen. + bool codegen_success = true; + for (auto& unit : units) { + codegen_success &= unit->RunCodeGen(); } - CARBON_VLOG() << "*** CodeGen done ***\n"; - return true; + return codegen_success; } } // namespace Carbon diff --git a/toolchain/driver/driver.h b/toolchain/driver/driver.h index 3c77f3b6cded..b87aa122be2d 100644 --- a/toolchain/driver/driver.h +++ b/toolchain/driver/driver.h @@ -39,6 +39,7 @@ class Driver { private: struct Options; struct CompileOptions; + class CompilationUnit; // Delegates to the command line library to parse the arguments and store the // results in a custom `Options` structure that the rest of the driver uses. diff --git a/toolchain/driver/testdata/fail_errors_in_two_files.carbon b/toolchain/driver/testdata/fail_errors_in_two_files.carbon new file mode 100644 index 000000000000..d440b0c1d0f5 --- /dev/null +++ b/toolchain/driver/testdata/fail_errors_in_two_files.carbon @@ -0,0 +1,22 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// ARGS: compile --phase=lex %s +// +// AUTOUPDATE + +// --- file1.carbon + +// CHECK:STDERR: file1.carbon:[[@LINE+3]]:24: Closing symbol does not match most recent opening symbol. +// CHECK:STDERR: fn run(String program) { +// CHECK:STDERR: ^ +fn run(String program) { + return True; + +// --- file2.carbon + +// CHECK:STDERR: file2.carbon:[[@LINE+3]]:10: Invalid digit 'a' in decimal numeric literal. +// CHECK:STDERR: var x = 3a; +// CHECK:STDERR: ^ +var x = 3a; diff --git a/toolchain/lex/tokenized_buffer_benchmark.cpp b/toolchain/lex/tokenized_buffer_benchmark.cpp index 4f425319fc20..79f546ed8e4f 100644 --- a/toolchain/lex/tokenized_buffer_benchmark.cpp +++ b/toolchain/lex/tokenized_buffer_benchmark.cpp @@ -297,7 +297,8 @@ class LexerBenchHelper { auto MakeSourceBuffer(llvm::StringRef text) -> SourceBuffer { CARBON_CHECK(fs_.addFile(filename_, /*ModificationTime=*/0, llvm::MemoryBuffer::getMemBuffer(text))); - return std::move(*SourceBuffer::CreateFromFile(fs_, filename_)); + return std::move( + *SourceBuffer::CreateFromFile(fs_, llvm::errs(), filename_)); } llvm::vfs::InMemoryFileSystem fs_; diff --git a/toolchain/lex/tokenized_buffer_fuzzer.cpp b/toolchain/lex/tokenized_buffer_fuzzer.cpp index 14565dff9deb..e65dc7a6ac02 100644 --- a/toolchain/lex/tokenized_buffer_fuzzer.cpp +++ b/toolchain/lex/tokenized_buffer_fuzzer.cpp @@ -30,7 +30,7 @@ extern "C" int LLVMFuzzerTestOneInput(const unsigned char* data, TestFileName, /*ModificationTime=*/0, llvm::MemoryBuffer::getMemBuffer(data_ref, /*BufferName=*/TestFileName, /*RequiresNullTerminator=*/false))); - auto source = SourceBuffer::CreateFromFile(fs, TestFileName); + auto source = SourceBuffer::CreateFromFile(fs, llvm::nulls(), TestFileName); auto buffer = Lex::TokenizedBuffer::Lex(*source, NullDiagnosticConsumer()); if (buffer.has_errors()) { diff --git a/toolchain/lex/tokenized_buffer_test.cpp b/toolchain/lex/tokenized_buffer_test.cpp index 27748b1b6d11..b98677e4fad1 100644 --- a/toolchain/lex/tokenized_buffer_test.cpp +++ b/toolchain/lex/tokenized_buffer_test.cpp @@ -35,7 +35,7 @@ class LexerTest : public ::testing::Test { CARBON_CHECK(fs_.addFile(filename, /*ModificationTime=*/0, llvm::MemoryBuffer::getMemBuffer(text))); source_storage_.push_front( - std::move(*SourceBuffer::CreateFromFile(fs_, filename))); + std::move(*SourceBuffer::CreateFromFile(fs_, llvm::errs(), filename))); return source_storage_.front(); } diff --git a/toolchain/parse/parse_fuzzer.cpp b/toolchain/parse/parse_fuzzer.cpp index 25b4d44dc499..63746a254ffb 100644 --- a/toolchain/parse/parse_fuzzer.cpp +++ b/toolchain/parse/parse_fuzzer.cpp @@ -27,7 +27,7 @@ extern "C" int LLVMFuzzerTestOneInput(const unsigned char* data, TestFileName, /*ModificationTime=*/0, llvm::MemoryBuffer::getMemBuffer(data_ref, /*BufferName=*/TestFileName, /*RequiresNullTerminator=*/false))); - auto source = SourceBuffer::CreateFromFile(fs, TestFileName); + auto source = SourceBuffer::CreateFromFile(fs, llvm::nulls(), TestFileName); // Lex the input. auto tokens = Lex::TokenizedBuffer::Lex(*source, NullDiagnosticConsumer()); diff --git a/toolchain/parse/tree_test.cpp b/toolchain/parse/tree_test.cpp index 92b9b4c968e9..1f7d60d25593 100644 --- a/toolchain/parse/tree_test.cpp +++ b/toolchain/parse/tree_test.cpp @@ -26,8 +26,8 @@ class TreeTest : public ::testing::Test { 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::CreateFromFile(fs, "test.carbon"))); + source_storage.push_front(std::move( + *SourceBuffer::CreateFromFile(fs, llvm::errs(), "test.carbon"))); return source_storage.front(); } diff --git a/toolchain/source/BUILD b/toolchain/source/BUILD index 2c2fce67073a..2ba2adb6292d 100644 --- a/toolchain/source/BUILD +++ b/toolchain/source/BUILD @@ -22,6 +22,7 @@ cc_test( srcs = ["source_buffer_test.cpp"], deps = [ ":source_buffer", + "//common:check", "//testing/base:gtest_main", "@com_google_googletest//:gtest", "@llvm-project//llvm:Support", diff --git a/toolchain/source/source_buffer.cpp b/toolchain/source/source_buffer.cpp index bb9a431acdcb..b38e0d06b6c0 100644 --- a/toolchain/source/source_buffer.cpp +++ b/toolchain/source/source_buffer.cpp @@ -7,33 +7,40 @@ #include #include "llvm/Support/ErrorOr.h" -#include "llvm/Support/FormatVariadic.h" namespace Carbon { auto SourceBuffer::CreateFromFile(llvm::vfs::FileSystem& fs, + llvm::raw_ostream& error_stream, llvm::StringRef filename) - -> ErrorOr { + -> std::optional { llvm::ErrorOr> file = fs.openFileForRead(filename); if (file.getError()) { - return Error(file.getError().message()); + error_stream << "Error opening `" << filename + << "`: " << file.getError().message(); + return std::nullopt; } llvm::ErrorOr status = (*file)->status(); if (status.getError()) { - return Error(status.getError().message()); + error_stream << "Error getting status for `" << filename + << "`: " << file.getError().message(); + return std::nullopt; } auto size = status->getSize(); if (size >= std::numeric_limits::max()) { - return Error( - llvm::formatv("`{0}` is over the 2GiB input limit.", filename)); + error_stream << "Cannot load `" << filename + << "`: file is over the 2GiB input limit."; + return std::nullopt; } llvm::ErrorOr> buffer = (*file)->getBuffer(filename, size, /*RequiresNullTerminator=*/false); if (buffer.getError()) { - return Error(buffer.getError().message()); + error_stream << "Error reading `" << filename + << "`: " << file.getError().message(); + return std::nullopt; } return SourceBuffer(filename.str(), std::move(buffer.get())); diff --git a/toolchain/source/source_buffer.h b/toolchain/source/source_buffer.h index cd30280642c9..52d97b3d6725 100644 --- a/toolchain/source/source_buffer.h +++ b/toolchain/source/source_buffer.h @@ -8,7 +8,6 @@ #include #include -#include "common/error.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/VirtualFileSystem.h" @@ -34,8 +33,13 @@ namespace Carbon { // some implementation complexity in the future if needed. class SourceBuffer { public: + // Opens the requested file. Returns a SourceBuffer on success. Prints an + // error and returns nullopt on failure. + // TODO: Switch to using diagnostics. static auto CreateFromFile(llvm::vfs::FileSystem& fs, - llvm::StringRef filename) -> ErrorOr; + llvm::raw_ostream& error_stream, + llvm::StringRef filename) + -> std::optional; // Use one of the factory functions above to create a source buffer. SourceBuffer() = delete; diff --git a/toolchain/source/source_buffer_test.cpp b/toolchain/source/source_buffer_test.cpp index 8fccb457cd18..927243478e99 100644 --- a/toolchain/source/source_buffer_test.cpp +++ b/toolchain/source/source_buffer_test.cpp @@ -6,6 +6,7 @@ #include +#include "common/check.h" #include "llvm/Support/VirtualFileSystem.h" namespace Carbon::Testing { @@ -15,8 +16,8 @@ static constexpr llvm::StringLiteral TestFileName = "test.carbon"; TEST(SourceBufferTest, MissingFile) { llvm::vfs::InMemoryFileSystem fs; - auto buffer = SourceBuffer::CreateFromFile(fs, TestFileName); - EXPECT_FALSE(buffer.ok()); + auto buffer = SourceBuffer::CreateFromFile(fs, llvm::errs(), TestFileName); + EXPECT_FALSE(buffer); } TEST(SourceBufferTest, SimpleFile) { @@ -24,8 +25,8 @@ TEST(SourceBufferTest, SimpleFile) { 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(); + auto buffer = SourceBuffer::CreateFromFile(fs, llvm::errs(), TestFileName); + ASSERT_TRUE(buffer); EXPECT_EQ(TestFileName, buffer->filename()); EXPECT_EQ("Hello World", buffer->text()); @@ -40,8 +41,8 @@ TEST(SourceBufferTest, NoNull) { /*BufferName=*/"", /*RequiresNullTerminator=*/false))); - auto buffer = SourceBuffer::CreateFromFile(fs, TestFileName); - ASSERT_TRUE(buffer.ok()) << "Error message: " << buffer.error(); + auto buffer = SourceBuffer::CreateFromFile(fs, llvm::errs(), TestFileName); + ASSERT_TRUE(buffer); EXPECT_EQ(TestFileName, buffer->filename()); EXPECT_EQ("abc", buffer->text()); @@ -52,8 +53,8 @@ TEST(SourceBufferTest, EmptyFile) { CARBON_CHECK(fs.addFile(TestFileName, /*ModificationTime=*/0, llvm::MemoryBuffer::getMemBuffer(""))); - auto buffer = SourceBuffer::CreateFromFile(fs, TestFileName); - ASSERT_TRUE(buffer.ok()) << "Error message: " << buffer.error(); + auto buffer = SourceBuffer::CreateFromFile(fs, llvm::errs(), TestFileName); + ASSERT_TRUE(buffer); EXPECT_EQ(TestFileName, buffer->filename()); EXPECT_EQ("", buffer->text());