Explorar o código

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.
Jon Ross-Perkins %!s(int64=2) %!d(string=hai) anos
pai
achega
9ac92ad71b

+ 1 - 1
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<clang::clangd::DocumentSymbol> result;

+ 238 - 145
toolchain/driver/driver.cpp

@@ -4,9 +4,13 @@
 
 #include "toolchain/driver/driver.h"
 
+#include <memory>
+#include <optional>
+
 #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<llvm::StringRef> 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<SortingDiagnosticConsumer> sorting_consumer;
-  if (vlog_stream_ == nullptr && !options.stream_errors) {
-    sorting_consumer = std::make_unique<SortingDiagnosticConsumer>(*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<llvm::LLVMContext>();
+      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 =
+        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<void()> 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<SortingDiagnosticConsumer> sorting_consumer_;
+  DiagnosticConsumer* consumer_;
+
+  // These are initialized as steps are run.
+  std::optional<SourceBuffer> source_;
+  std::optional<Lex::TokenizedBuffer> tokens_;
+  std::optional<Parse::Tree> parse_tree_;
+  std::optional<SemIR::File> sem_ir_;
+  std::unique_ptr<llvm::LLVMContext> llvm_context_;
+  std::unique_ptr<llvm::Module> 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<std::unique_ptr<CompilationUnit>> 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<CompilationUnit>(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<llvm::Module> 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 =
-      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

+ 1 - 0
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.

+ 22 - 0
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;

+ 2 - 1
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_;

+ 1 - 1
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()) {

+ 1 - 1
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();
   }
 

+ 1 - 1
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());

+ 2 - 2
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();
   }
 

+ 1 - 0
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",

+ 14 - 7
toolchain/source/source_buffer.cpp

@@ -7,33 +7,40 @@
 #include <limits>
 
 #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<SourceBuffer> {
+    -> std::optional<SourceBuffer> {
   llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>> 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<llvm::vfs::Status> 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<int32_t>::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<std::unique_ptr<llvm::MemoryBuffer>> 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()));

+ 6 - 2
toolchain/source/source_buffer.h

@@ -8,7 +8,6 @@
 #include <memory>
 #include <string>
 
-#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<SourceBuffer>;
+                             llvm::raw_ostream& error_stream,
+                             llvm::StringRef filename)
+      -> std::optional<SourceBuffer>;
 
   // Use one of the factory functions above to create a source buffer.
   SourceBuffer() = delete;

+ 9 - 8
toolchain/source/source_buffer_test.cpp

@@ -6,6 +6,7 @@
 
 #include <gtest/gtest.h>
 
+#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());