From 4f024410f7adf7b73ef7178b0fb00e09350697f7 Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Tue, 21 Jan 2025 08:52:05 -0800 Subject: [PATCH] Add stdin to driver's streams, and refactor stream passing (#4812) The language server needs stdin, and for tests we should be passing it around. My intent is to pass in a faux stdin to Driver for language server tests. As long as I'm adding a new parameter, I was looking at also changing the way streams are passed in to Driver for style (pointers since they're held past construction lifetime). Since these are all stored in DriverEnv, I thought it might be a net improvement to use the struct directly, getting more explicit parameter names and also removing the need for `SetFuzzing`. I'm trying here to avoid functional changes, but there are a couple additional fixes like removing an obsolete `find_insensitive` and refactoring how `ValidateOptions` handles errors (because it reduces the number of spots that operate on error_stream). --- testing/base/source_gen_test.cpp | 6 +- toolchain/check/check_fuzzer.cpp | 7 +- toolchain/driver/BUILD | 1 + toolchain/driver/clang_subcommand.cpp | 2 +- toolchain/driver/compile_benchmark.cpp | 6 +- toolchain/driver/compile_subcommand.cpp | 79 ++++++++----------- toolchain/driver/compile_subcommand.h | 3 +- toolchain/driver/driver.cpp | 14 ++-- toolchain/driver/driver.h | 16 +--- toolchain/driver/driver_env.h | 12 ++- toolchain/driver/driver_fuzzer.cpp | 16 ++-- toolchain/driver/driver_test.cpp | 6 +- toolchain/driver/format_subcommand.cpp | 11 +-- .../driver/language_server_subcommand.cpp | 13 ++- toolchain/install/busybox_main.cpp | 6 +- toolchain/language_server/language_server.cpp | 5 +- toolchain/language_server/language_server.h | 7 +- toolchain/sem_ir/yaml_test.cpp | 10 ++- toolchain/testing/file_test.cpp | 6 +- 19 files changed, 125 insertions(+), 101 deletions(-) diff --git a/testing/base/source_gen_test.cpp b/testing/base/source_gen_test.cpp index 9f428bbbd1d5..74e46d51f10f 100644 --- a/testing/base/source_gen_test.cpp +++ b/testing/base/source_gen_test.cpp @@ -147,7 +147,11 @@ auto TestCompile(llvm::StringRef source) -> bool { new llvm::vfs::InMemoryFileSystem; InstallPaths installation( InstallPaths::MakeForBazelRunfiles(Testing::GetExePath())); - Driver driver(fs, &installation, llvm::outs(), llvm::errs()); + Driver driver({.fs = fs, + .installation = &installation, + .input_stream = nullptr, + .output_stream = &llvm::outs(), + .error_stream = &llvm::errs()}); AddPreludeFilesToVfs(installation, fs); diff --git a/toolchain/check/check_fuzzer.cpp b/toolchain/check/check_fuzzer.cpp index 463f40bc6a6c..9765bd28070c 100644 --- a/toolchain/check/check_fuzzer.cpp +++ b/toolchain/check/check_fuzzer.cpp @@ -39,7 +39,12 @@ extern "C" int LLVMFuzzerTestOneInput(const unsigned char* data, size_t size) { /*RequiresNullTerminator=*/false))); llvm::raw_null_ostream null_ostream; - Driver driver(fs, install_paths, null_ostream, null_ostream); + Driver driver({.fs = fs, + .installation = install_paths, + .input_stream = nullptr, + .output_stream = &null_ostream, + .error_stream = &null_ostream, + .fuzzing = true}); // TODO: Get checking to a point where it can handle invalid parse trees // without crashing. diff --git a/toolchain/driver/BUILD b/toolchain/driver/BUILD index b5bd1017b58e..72c5b7869e7d 100644 --- a/toolchain/driver/BUILD +++ b/toolchain/driver/BUILD @@ -114,6 +114,7 @@ cc_library( deps = [ ":clang_runner", "//common:command_line", + "//common:error", "//common:ostream", "//common:raw_string_ostream", "//common:version", diff --git a/toolchain/driver/clang_subcommand.cpp b/toolchain/driver/clang_subcommand.cpp index 8b69b5708779..3d43eae846d0 100644 --- a/toolchain/driver/clang_subcommand.cpp +++ b/toolchain/driver/clang_subcommand.cpp @@ -51,7 +51,7 @@ auto ClangSubcommand::Run(DriverEnv& driver_env) -> DriverResult { // Don't run Clang when fuzzing, it is known to not be reliable under fuzzing // due to many unfixed issues. if (driver_env.fuzzing) { - driver_env.error_stream + *driver_env.error_stream << "error: cannot run `clang` subcommand productively when fuzzing\n"; return {.success = false}; } diff --git a/toolchain/driver/compile_benchmark.cpp b/toolchain/driver/compile_benchmark.cpp index 703ff6b73d34..f5b0f56c5588 100644 --- a/toolchain/driver/compile_benchmark.cpp +++ b/toolchain/driver/compile_benchmark.cpp @@ -23,7 +23,11 @@ class CompileBenchmark { public: CompileBenchmark() : installation_(InstallPaths::MakeForBazelRunfiles(GetExePath())), - driver_(fs_, &installation_, llvm::outs(), llvm::errs()) { + driver_({.fs = fs_, + .installation = &installation_, + .input_stream = nullptr, + .output_stream = &llvm::outs(), + .error_stream = &llvm::errs()}) { AddPreludeFilesToVfs(installation_, fs_); } diff --git a/toolchain/driver/compile_subcommand.cpp b/toolchain/driver/compile_subcommand.cpp index fd29a1af39d0..f438d0f360f6 100644 --- a/toolchain/driver/compile_subcommand.cpp +++ b/toolchain/driver/compile_subcommand.cpp @@ -296,34 +296,30 @@ can be written to standard output as these phases progress. CompileSubcommand::CompileSubcommand() : DriverSubcommand(SubcommandInfo) {} -auto CompileSubcommand::ValidateOptions(DriverEnv& driver_env) const -> bool { +// Returns an error for trying to dump a non-executed phase's output. +static auto DumpPhaseError(llvm::StringLiteral requested_dump, + CompileOptions::Phase phase) -> Error { + return Error(llvm::formatv( + "requested dumping {0} but compile phase is limited to `{1}`", + requested_dump, phase)); +} + +auto CompileSubcommand::ValidateOptions() const -> ErrorOr { using Phase = CompileOptions::Phase; switch (options_.phase) { case Phase::Lex: if (options_.dump_parse_tree) { - driver_env.error_stream - << "error: requested dumping the parse tree but compile " - "phase is limited to '" - << options_.phase << "'\n"; - return false; + return DumpPhaseError("parse tree", options_.phase); } [[fallthrough]]; case Phase::Parse: if (options_.dump_sem_ir) { - driver_env.error_stream - << "error: requested dumping the SemIR but compile phase " - "is limited to '" - << options_.phase << "'\n"; - return false; + return DumpPhaseError("SemIR", options_.phase); } [[fallthrough]]; case Phase::Check: if (options_.dump_llvm_ir) { - driver_env.error_stream - << "error: requested dumping the LLVM IR but compile " - "phase is limited to '" - << options_.phase << "'\n"; - return false; + return DumpPhaseError("LLVM IR", options_.phase); } [[fallthrough]]; case Phase::Lower: @@ -331,7 +327,7 @@ auto CompileSubcommand::ValidateOptions(DriverEnv& driver_env) const -> bool { // Everything can be dumped in these phases. break; } - return true; + return Success(); } namespace { @@ -478,7 +474,7 @@ auto CompilationUnit::RunLex() -> void { [&] { tokens_ = Lex::Lex(value_stores_, *source_, *consumer_); }); if (options_.dump_tokens && IncludeInDumps()) { consumer_->Flush(); - tokens_->Print(driver_env_->output_stream, + tokens_->Print(*driver_env_->output_stream, options_.omit_file_boundary_tokens); } if (mem_usage_) { @@ -491,9 +487,6 @@ auto CompilationUnit::RunLex() -> void { } auto CompilationUnit::RunParse() -> void { - CARBON_CHECK(tokens_, "Must call RunLex first"); - CARBON_CHECK(!parse_tree_, "Called RunParse twice"); - LogCall("Parse::Parse", "parse", [&] { parse_tree_ = Parse::Parse(*tokens_, *consumer_, vlog_stream_); }); @@ -501,9 +494,9 @@ auto CompilationUnit::RunParse() -> void { consumer_->Flush(); const auto& tree_and_subtrees = GetParseTreeAndSubtrees(); if (options_.preorder_parse_tree) { - tree_and_subtrees.PrintPreorder(driver_env_->output_stream); + tree_and_subtrees.PrintPreorder(*driver_env_->output_stream); } else { - tree_and_subtrees.Print(driver_env_->output_stream); + tree_and_subtrees.Print(*driver_env_->output_stream); } } if (mem_usage_) { @@ -559,9 +552,9 @@ auto CompilationUnit::PostCheck() -> void { if (options_.dump_raw_sem_ir && IncludeInDumps()) { CARBON_VLOG("*** Raw SemIR::File ***\n{0}\n", *sem_ir_); - sem_ir_->Print(driver_env_->output_stream, options_.builtin_sem_ir); + sem_ir_->Print(*driver_env_->output_stream, options_.builtin_sem_ir); if (options_.dump_sem_ir) { - driver_env_->output_stream << "\n"; + *driver_env_->output_stream << "\n"; } } @@ -596,7 +589,7 @@ auto CompilationUnit::PostCheck() -> void { formatter.Print(*vlog_stream_); } if (print) { - formatter.Print(driver_env_->output_stream); + formatter.Print(*driver_env_->output_stream); } } if (sem_ir_->has_errors()) { @@ -605,9 +598,6 @@ auto CompilationUnit::PostCheck() -> void { } auto CompilationUnit::RunLower() -> void { - CARBON_CHECK(sem_ir_converter_, "Must call PostCheck first"); - CARBON_CHECK(!module_, "Called RunLower twice"); - LogCall("Lower::LowerToLLVM", "lower", [&] { llvm_context_ = std::make_unique(); // TODO: Consider disabling instruction naming by default if we're not @@ -624,7 +614,7 @@ auto CompilationUnit::RunLower() -> void { /*IsForDebug=*/true); } if (options_.dump_llvm_ir && IncludeInDumps()) { - module_->print(driver_env_->output_stream, /*AAW=*/nullptr, + module_->print(*driver_env_->output_stream, /*AAW=*/nullptr, /*ShouldPreserveUseListOrder=*/true); } } @@ -636,16 +626,16 @@ auto CompilationUnit::RunCodeGen() -> void { auto CompilationUnit::PostCompile() -> void { if (options_.dump_shared_values && IncludeInDumps()) { - Yaml::Print(driver_env_->output_stream, + Yaml::Print(*driver_env_->output_stream, value_stores_.OutputYaml(input_filename_)); } if (mem_usage_) { mem_usage_->Collect("value_stores_", value_stores_); - Yaml::Print(driver_env_->output_stream, + Yaml::Print(*driver_env_->output_stream, mem_usage_->OutputYaml(input_filename_)); } if (timings_) { - Yaml::Print(driver_env_->output_stream, + Yaml::Print(*driver_env_->output_stream, timings_->OutputYaml(input_filename_)); } @@ -656,7 +646,7 @@ auto CompilationUnit::PostCompile() -> void { auto CompilationUnit::RunCodeGenHelper() -> bool { std::optional codegen = CodeGen::Make( - *module_, options_.codegen_options.target, driver_env_->error_stream); + *module_, options_.codegen_options.target, *driver_env_->error_stream); if (!codegen) { return false; } @@ -670,11 +660,11 @@ auto CompilationUnit::RunCodeGenHelper() -> bool { // 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_env_->output_stream)) { + if (!codegen->EmitObject(*driver_env_->output_stream)) { return false; } } else { - if (!codegen->EmitAssembly(driver_env_->output_stream)) { + if (!codegen->EmitAssembly(*driver_env_->output_stream)) { return false; } } @@ -683,7 +673,7 @@ auto CompilationUnit::RunCodeGenHelper() -> bool { if (output_filename.empty()) { if (!source_->is_regular_file()) { // Don't invent file names like `-.o` or `/dev/stdin.o`. - driver_env_->error_stream + *driver_env_->error_stream << "error: output file name must be specified for input `" << input_filename_ << "` that is not a regular file\n"; return false; @@ -704,9 +694,9 @@ auto CompilationUnit::RunCodeGenHelper() -> bool { llvm::raw_fd_ostream output_file(output_filename, ec, llvm::sys::fs::OF_None); if (ec) { - driver_env_->error_stream << "error: could not open output file '" - << output_filename << "': " << ec.message() - << "\n"; + *driver_env_->error_stream << "error: could not open output file '" + << output_filename << "': " << ec.message() + << "\n"; return false; } if (options_.asm_output) { @@ -755,7 +745,8 @@ auto CompilationUnit::IncludeInDumps(llvm::StringRef filename) const -> bool { } // namespace auto CompileSubcommand::Run(DriverEnv& driver_env) -> DriverResult { - if (!ValidateOptions(driver_env)) { + if (auto validate = ValidateOptions(); !validate.ok()) { + *driver_env.error_stream << "error: " << validate.error() << "\n"; return {.success = false}; } @@ -768,13 +759,13 @@ auto CompileSubcommand::Run(DriverEnv& driver_env) -> DriverResult { if (auto find = driver_env.installation->ReadPreludeManifest(); find.ok()) { prelude = std::move(*find); } else { - driver_env.error_stream << "error: " << find.error() << "\n"; + *driver_env.error_stream << "error: " << find.error() << "\n"; return {.success = false}; } } // Prepare CompilationUnits before building scope exit handlers. - StreamDiagnosticConsumer stream_consumer(driver_env.error_stream, + StreamDiagnosticConsumer stream_consumer(*driver_env.error_stream, options_.include_diagnostic_kind); llvm::SmallVector> units; units.reserve(prelude.size() + options_.input_filenames.size()); @@ -817,7 +808,7 @@ auto CompileSubcommand::Run(DriverEnv& driver_env) -> DriverResult { unit->FlushForStackTrace(); } stream_consumer.Flush(); - stream_consumer.set_stream(&driver_env.error_stream); + stream_consumer.set_stream(driver_env.error_stream); }); // Returns a DriverResult object. Called whenever Compile returns. diff --git a/toolchain/driver/compile_subcommand.h b/toolchain/driver/compile_subcommand.h index 094ad23319b2..de78e9a9aa01 100644 --- a/toolchain/driver/compile_subcommand.h +++ b/toolchain/driver/compile_subcommand.h @@ -6,6 +6,7 @@ #define CARBON_TOOLCHAIN_DRIVER_COMPILE_SUBCOMMAND_H_ #include "common/command_line.h" +#include "common/error.h" #include "common/ostream.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" @@ -75,7 +76,7 @@ class CompileSubcommand : public DriverSubcommand { private: // Does custom validation of the compile-subcommand options structure beyond // what the command line parsing library supports. - auto ValidateOptions(DriverEnv& driver_env) const -> bool; + auto ValidateOptions() const -> ErrorOr; CompileOptions options_; }; diff --git a/toolchain/driver/driver.cpp b/toolchain/driver/driver.cpp index d89557fa0a9c..762f87dcc4ae 100644 --- a/toolchain/driver/driver.cpp +++ b/toolchain/driver/driver.cpp @@ -84,19 +84,19 @@ auto Options::Build(CommandLine::CommandBuilder& b) -> void { auto Driver::RunCommand(llvm::ArrayRef args) -> DriverResult { if (driver_env_.installation->error()) { - driver_env_.error_stream << "error: " << *driver_env_.installation->error() - << "\n"; + *driver_env_.error_stream << "error: " << *driver_env_.installation->error() + << "\n"; return {.success = false}; } Options options; ErrorOr result = CommandLine::Parse( - args, driver_env_.output_stream, Options::Info, + args, *driver_env_.output_stream, Options::Info, [&](CommandLine::CommandBuilder& b) { options.Build(b); }); if (!result.ok()) { - driver_env_.error_stream << "error: " << result.error() << "\n"; + *driver_env_.error_stream << "error: " << result.error() << "\n"; return {.success = false}; } else if (*result == CommandLine::ParseResult::MetaSuccess) { return {.success = true}; @@ -104,16 +104,14 @@ auto Driver::RunCommand(llvm::ArrayRef args) -> DriverResult { if (options.verbose) { // Note this implies streamed output in order to interleave. - driver_env_.vlog_stream = &driver_env_.error_stream; + driver_env_.vlog_stream = driver_env_.error_stream; } if (options.fuzzing) { - SetFuzzing(); + driver_env_.fuzzing = true; } CARBON_CHECK(options.selected_subcommand != nullptr); return options.selected_subcommand->Run(driver_env_); } -auto Driver::SetFuzzing() -> void { driver_env_.fuzzing = true; } - } // namespace Carbon diff --git a/toolchain/driver/driver.h b/toolchain/driver/driver.h index 276cfbe1bb08..688bbf90ae6a 100644 --- a/toolchain/driver/driver.h +++ b/toolchain/driver/driver.h @@ -20,16 +20,8 @@ namespace Carbon { // with the language. class Driver { public: - // Constructs a driver with any error or informational output directed to a - // specified stream. - Driver(llvm::IntrusiveRefCntPtr fs, - const InstallPaths* installation, - llvm::raw_pwrite_stream& output_stream, - llvm::raw_pwrite_stream& error_stream) - : driver_env_{.fs = fs, - .installation = installation, - .output_stream = output_stream, - .error_stream = error_stream} {} + // Constructs a driver with the provided environment. + explicit Driver(DriverEnv env) : driver_env_(std::move(env)) {} // Parses the given arguments into both a subcommand to select the operation // to perform and any arguments to that subcommand. @@ -39,10 +31,6 @@ class Driver { // error stream (stderr by default). auto RunCommand(llvm::ArrayRef args) -> DriverResult; - // Configure the driver for fuzzing. This allows specific commands to error - // rather than perform operations that aren't well behaved during fuzzing. - auto SetFuzzing() -> void; - private: DriverEnv driver_env_; }; diff --git a/toolchain/driver/driver_env.h b/toolchain/driver/driver_env.h index e46327ee15d0..d6e40f389b20 100644 --- a/toolchain/driver/driver_env.h +++ b/toolchain/driver/driver_env.h @@ -5,6 +5,8 @@ #ifndef CARBON_TOOLCHAIN_DRIVER_DRIVER_ENV_H_ #define CARBON_TOOLCHAIN_DRIVER_DRIVER_ENV_H_ +#include + #include "common/ostream.h" #include "llvm/Support/VirtualFileSystem.h" #include "toolchain/install/install_paths.h" @@ -18,15 +20,19 @@ struct DriverEnv { // Helper to locate the toolchain installation's files. const InstallPaths* installation; + // Standard input; stdin. May be null, to prevent accidental use. + FILE* input_stream; // Standard output; stdout. - llvm::raw_pwrite_stream& output_stream; + llvm::raw_pwrite_stream* output_stream; // Error output; stderr. - llvm::raw_pwrite_stream& error_stream; + llvm::raw_pwrite_stream* error_stream; // For CARBON_VLOG. llvm::raw_pwrite_stream* vlog_stream = nullptr; - // Tracks when the driver is being fuzzed. + // Tracks when the driver is being fuzzed. This allows specific commands to + // error rather than perform operations that aren't well behaved during + // fuzzing. bool fuzzing = false; }; diff --git a/toolchain/driver/driver_fuzzer.cpp b/toolchain/driver/driver_fuzzer.cpp index 91b189c6bf8a..ca837101ab04 100644 --- a/toolchain/driver/driver_fuzzer.cpp +++ b/toolchain/driver/driver_fuzzer.cpp @@ -81,14 +81,16 @@ extern "C" auto LLVMFuzzerTestOneInput(const unsigned char* data, size_t size) llvm::IntrusiveRefCntPtr fs = new llvm::vfs::InMemoryFileSystem; RawStringOstream error_stream; - llvm::raw_null_ostream dest; - Driver d(fs, install_paths, dest, error_stream); - d.SetFuzzing(); - if (!d.RunCommand(args).success) { + llvm::raw_null_ostream null_ostream; + Driver driver({.fs = fs, + .installation = install_paths, + .input_stream = nullptr, + .output_stream = &null_ostream, + .error_stream = &error_stream, + .fuzzing = true}); + if (!driver.RunCommand(args).success) { auto str = error_stream.TakeStr(); - // TODO: Fix command_line to use `error`, switch back to `find`. - if (llvm::StringRef(str).find_insensitive("error:") == - llvm::StringRef::npos) { + if (llvm::StringRef(str).find("error:") == llvm::StringRef::npos) { llvm::errs() << "No error message on a failure!\n"; return 1; } diff --git a/toolchain/driver/driver_test.cpp b/toolchain/driver/driver_test.cpp index 3b2bbae97823..4fba9e5210ff 100644 --- a/toolchain/driver/driver_test.cpp +++ b/toolchain/driver/driver_test.cpp @@ -43,7 +43,11 @@ class DriverTest : public testing::Test { DriverTest() : installation_( InstallPaths::MakeForBazelRunfiles(Testing::GetExePath())), - driver_(fs_, &installation_, test_output_stream_, test_error_stream_) { + driver_({.fs = fs_, + .installation = &installation_, + .input_stream = nullptr, + .output_stream = &test_output_stream_, + .error_stream = &test_error_stream_}) { char* tmpdir_env = getenv("TEST_TMPDIR"); CARBON_CHECK(tmpdir_env != nullptr); test_tmpdir_ = tmpdir_env; diff --git a/toolchain/driver/format_subcommand.cpp b/toolchain/driver/format_subcommand.cpp index 41745b1d16e6..cb9e40b20824 100644 --- a/toolchain/driver/format_subcommand.cpp +++ b/toolchain/driver/format_subcommand.cpp @@ -56,8 +56,9 @@ auto FormatSubcommand::Run(DriverEnv& driver_env) -> DriverResult { DriverResult result = {.success = true}; if (options_.input_filenames.size() > 1 && !options_.output_filename.empty()) { - driver_env.error_stream << "error: cannot format multiple input files when " - "--output is set\n"; + *driver_env.error_stream + << "error: cannot format multiple input files when " + "--output is set\n"; result.success = false; return result; } @@ -67,7 +68,7 @@ auto FormatSubcommand::Run(DriverEnv& driver_env) -> DriverResult { result.per_file_success.back().second = false; }; - StreamDiagnosticConsumer consumer(driver_env.error_stream, + StreamDiagnosticConsumer consumer(*driver_env.error_stream, /*include_diagnostic_kind=*/false); for (auto& f : options_.input_filenames) { // Push a result, which we'll update on failure. @@ -89,11 +90,11 @@ auto FormatSubcommand::Run(DriverEnv& driver_env) -> DriverResult { // TODO: Figure out a multi-file output setup that supports good // multi-file testing. // TODO: Use --output values (and default to overwrite). - driver_env.output_stream << buffer.TakeStr(); + *driver_env.output_stream << buffer.TakeStr(); } else { buffer.clear(); mark_per_file_error(); - driver_env.output_stream << source->text(); + *driver_env.output_stream << source->text(); } } diff --git a/toolchain/driver/language_server_subcommand.cpp b/toolchain/driver/language_server_subcommand.cpp index b7ddab1bd8ba..816a66184da9 100644 --- a/toolchain/driver/language_server_subcommand.cpp +++ b/toolchain/driver/language_server_subcommand.cpp @@ -19,11 +19,16 @@ LanguageServerSubcommand::LanguageServerSubcommand() : DriverSubcommand(SubcommandInfo) {} auto LanguageServerSubcommand::Run(DriverEnv& driver_env) -> DriverResult { - // TODO: Consider a way to override stdin, but it's a `FILE*` so less - // convenient to work with. - auto err = LanguageServer::Run(stdin, driver_env.output_stream); + if (!driver_env.input_stream) { + *driver_env.error_stream + << "error: language-server requires input_stream\n"; + } + + auto err = + LanguageServer::Run(driver_env.input_stream, *driver_env.output_stream, + *driver_env.error_stream); if (!err.ok()) { - driver_env.error_stream << "error: " << err.error() << "\n"; + *driver_env.error_stream << "error: " << err.error() << "\n"; } return {.success = err.ok()}; } diff --git a/toolchain/install/busybox_main.cpp b/toolchain/install/busybox_main.cpp index 1609f8fed129..5a6682f872e7 100644 --- a/toolchain/install/busybox_main.cpp +++ b/toolchain/install/busybox_main.cpp @@ -45,7 +45,11 @@ static auto Main(int argc, char** argv) -> ErrorOr { } args.append(argv + 1, argv + argc); - Driver driver(fs, &install_paths, llvm::outs(), llvm::errs()); + Driver driver({.fs = fs, + .installation = &install_paths, + .input_stream = stdin, + .output_stream = &llvm::outs(), + .error_stream = &llvm::errs()}); bool success = driver.RunCommand(args).success; return success ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/toolchain/language_server/language_server.cpp b/toolchain/language_server/language_server.cpp index 2b154d166fda..ad51e7e7417b 100644 --- a/toolchain/language_server/language_server.cpp +++ b/toolchain/language_server/language_server.cpp @@ -13,14 +13,15 @@ namespace Carbon::LanguageServer { -auto Run(std::FILE* input_stream, llvm::raw_ostream& output_stream) - -> ErrorOr { +auto Run(FILE* input_stream, llvm::raw_ostream& output_stream, + llvm::raw_ostream& /*error_stream*/) -> ErrorOr { // Set up the connection. std::unique_ptr transport( clang::clangd::newJSONTransport(input_stream, output_stream, /*InMirror=*/nullptr, /*Pretty=*/true)); Context context; + // TODO: Use error_stream in IncomingMessages to report dropped errors. IncomingMessages incoming(transport.get(), &context); OutgoingMessages outgoing(transport.get()); diff --git a/toolchain/language_server/language_server.h b/toolchain/language_server/language_server.h index 64be63c9aa97..b48a01ce3eab 100644 --- a/toolchain/language_server/language_server.h +++ b/toolchain/language_server/language_server.h @@ -10,9 +10,10 @@ namespace Carbon::LanguageServer { -// Start the language server. -auto Run(std::FILE* input_stream, llvm::raw_ostream& output_stream) - -> ErrorOr; +// Start the language server. input_stream and output_stream are used by LSP; +// error_stream is primarily for errors that don't fit into LSP. +auto Run(FILE* input_stream, llvm::raw_ostream& output_stream, + llvm::raw_ostream& error_stream) -> ErrorOr; } // namespace Carbon::LanguageServer diff --git a/toolchain/sem_ir/yaml_test.cpp b/toolchain/sem_ir/yaml_test.cpp index 0a77e0fe1ac9..c386c166d279 100644 --- a/toolchain/sem_ir/yaml_test.cpp +++ b/toolchain/sem_ir/yaml_test.cpp @@ -38,10 +38,14 @@ TEST(SemIRTest, YAML) { const auto install_paths = InstallPaths::MakeForBazelRunfiles(Testing::GetExePath()); RawStringOstream print_stream; - Driver d(fs, &install_paths, print_stream, llvm::errs()); + Driver driver({.fs = fs, + .installation = &install_paths, + .input_stream = nullptr, + .output_stream = &print_stream, + .error_stream = &llvm::errs()}); auto run_result = - d.RunCommand({"compile", "--no-prelude-import", "--phase=check", - "--dump-raw-sem-ir", "test.carbon"}); + driver.RunCommand({"compile", "--no-prelude-import", "--phase=check", + "--dump-raw-sem-ir", "test.carbon"}); EXPECT_TRUE(run_result.success); // Matches the ID of an instruction. Instruction counts may change as various diff --git a/toolchain/testing/file_test.cpp b/toolchain/testing/file_test.cpp index 83b0d2a8d0c5..882f535cac0e 100644 --- a/toolchain/testing/file_test.cpp +++ b/toolchain/testing/file_test.cpp @@ -44,7 +44,11 @@ class ToolchainFileTest : public FileTestBase { } } - Driver driver(fs, &installation_, stdout, stderr); + Driver driver({.fs = fs, + .installation = &installation_, + .input_stream = nullptr, + .output_stream = &stdout, + .error_stream = &stderr}); auto driver_result = driver.RunCommand(test_args); // If any diagnostics have been produced, add a trailing newline to make the // last diagnostic match intermediate diagnostics (that have a newline