diff --git a/toolchain/check/check_unit.cpp b/toolchain/check/check_unit.cpp index cff5de495bdf..ab533d79b4d2 100644 --- a/toolchain/check/check_unit.cpp +++ b/toolchain/check/check_unit.cpp @@ -366,7 +366,7 @@ auto CheckUnit::ProcessNodeIds() -> bool { auto converted = unit_and_imports_->unit->node_converter->ConvertLoc( node_id, [](DiagnosticLoc, const DiagnosticBase<>&) {}); converted.loc.FormatLocation(output); - output << ": checking " << context_.parse_tree().node_kind(node_id) << "\n"; + output << "checking " << context_.parse_tree().node_kind(node_id) << "\n"; // Crash output has a tab indent; try to indent slightly past that. converted.loc.FormatSnippet(output, /*indent=*/10); }); diff --git a/toolchain/check/context.cpp b/toolchain/check/context.cpp index 48ad7cae5e60..9417e4665ce6 100644 --- a/toolchain/check/context.cpp +++ b/toolchain/check/context.cpp @@ -223,7 +223,7 @@ auto Context::DiagnoseDuplicateName(SemIRLoc dup_def, SemIRLoc prev_def) .Emit(); } -auto Context::DiagnosePoisonedName(SemIRLoc loc) -> void { +auto Context::DiagnosePoisonedName(SemIR::InstId loc) -> void { // TODO: Improve the diagnostic to replace NodeId::None with the location // where the name was poisoned. See discussion in // https://github.com/carbon-language/carbon-lang/pull/4654#discussion_r1876607172 diff --git a/toolchain/check/context.h b/toolchain/check/context.h index d1eea852373a..6679c237d1ec 100644 --- a/toolchain/check/context.h +++ b/toolchain/check/context.h @@ -266,7 +266,7 @@ class Context { auto DiagnoseDuplicateName(SemIRLoc dup_def, SemIRLoc prev_def) -> void; // Prints a diagnostic for a poisoned name. - auto DiagnosePoisonedName(SemIRLoc loc) -> void; + auto DiagnosePoisonedName(SemIR::InstId loc) -> void; // Prints a diagnostic for a missing name. auto DiagnoseNameNotFound(SemIRLoc loc, SemIR::NameId name_id) -> void; diff --git a/toolchain/check/testdata/basics/builtin_insts.carbon b/toolchain/check/testdata/basics/builtin_insts.carbon index 03c55f892990..15533f0bec9d 100644 --- a/toolchain/check/testdata/basics/builtin_insts.carbon +++ b/toolchain/check/testdata/basics/builtin_insts.carbon @@ -2,7 +2,7 @@ // Exceptions. See /LICENSE for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // -// ARGS: compile --no-prelude-import --phase=check --dump-raw-sem-ir --builtin-sem-ir %s +// ARGS: --include-diagnostic-kind compile --no-prelude-import --phase=check --dump-raw-sem-ir --builtin-sem-ir %s // // AUTOUPDATE // TIP: To test this file alone, run: diff --git a/toolchain/check/testdata/interop/cpp/no_prelude/fail_fuzzing.carbon b/toolchain/check/testdata/interop/cpp/no_prelude/fail_fuzzing.carbon new file mode 100644 index 000000000000..6a4538772f84 --- /dev/null +++ b/toolchain/check/testdata/interop/cpp/no_prelude/fail_fuzzing.carbon @@ -0,0 +1,17 @@ +// 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: --include-diagnostic-kind --fuzzing compile --no-prelude-import --phase=check %s +// +// AUTOUPDATE +// TIP: To test this file alone, run: +// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/interop/cpp/no_prelude/fail_fuzzing.carbon +// TIP: To dump output, run: +// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/interop/cpp/no_prelude/fail_fuzzing.carbon + +// CHECK:STDERR: fail_fuzzing.carbon:[[@LINE+4]]:1: error: `Cpp` import found during fuzzing [CppInteropFuzzing] +// CHECK:STDERR: import Cpp library "file.h"; +// CHECK:STDERR: ^~~~~~ +// CHECK:STDERR: +import Cpp library "file.h"; diff --git a/toolchain/codegen/testdata/assembly/basic.carbon b/toolchain/codegen/testdata/assembly/basic.carbon index 7fdeee2802c5..dfcb97b1d94b 100644 --- a/toolchain/codegen/testdata/assembly/basic.carbon +++ b/toolchain/codegen/testdata/assembly/basic.carbon @@ -2,7 +2,7 @@ // Exceptions. See /LICENSE for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // -// ARGS: compile --no-prelude-import --target=x86_64-unknown-linux-gnu --output=- %s +// ARGS: --include-diagnostic-kind compile --no-prelude-import --target=x86_64-unknown-linux-gnu --output=- %s // // To test this file alone, run: // bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/codegen/testdata/assembly/basic.carbon diff --git a/toolchain/codegen/testdata/fail_target_triple.carbon b/toolchain/codegen/testdata/fail_target_triple.carbon index 8a24666c74b4..ff044f846fa3 100644 --- a/toolchain/codegen/testdata/fail_target_triple.carbon +++ b/toolchain/codegen/testdata/fail_target_triple.carbon @@ -2,7 +2,7 @@ // Exceptions. See /LICENSE for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // -// ARGS: compile --no-prelude-import --target=x86_687-unknown-linux-gnu --output=- %s +// ARGS: --include-diagnostic-kind compile --no-prelude-import --target=x86_687-unknown-linux-gnu --output=- %s // No autoupdate because the message comes from LLVM. // To test this file alone, run: // bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/codegen/testdata/fail_target_triple.carbon diff --git a/toolchain/codegen/testdata/objcode/basic.carbon b/toolchain/codegen/testdata/objcode/basic.carbon index 2e253c32689f..b1a44b03efe3 100644 --- a/toolchain/codegen/testdata/objcode/basic.carbon +++ b/toolchain/codegen/testdata/objcode/basic.carbon @@ -2,7 +2,7 @@ // Exceptions. See /LICENSE for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // -// ARGS: compile --no-prelude-import --target=x86_64-unknown-linux-gnu --output=%t %s +// ARGS: --include-diagnostic-kind compile --no-prelude-import --target=x86_64-unknown-linux-gnu --output=%t %s // // TODO: Add a way to write some basic tests for object file outputs. // AUTOUPDATE diff --git a/toolchain/diagnostics/BUILD b/toolchain/diagnostics/BUILD index 1799a0dcedb3..00fb77f999c5 100644 --- a/toolchain/diagnostics/BUILD +++ b/toolchain/diagnostics/BUILD @@ -81,6 +81,15 @@ cc_test( ], ) +cc_library( + name = "file_diagnostics", + hdrs = ["file_diagnostics.h"], + deps = [ + ":diagnostic_emitter", + "@llvm-project//llvm:Support", + ], +) + cc_library( name = "format_providers", srcs = ["format_providers.cpp"], diff --git a/toolchain/diagnostics/coverage_test.cpp b/toolchain/diagnostics/coverage_test.cpp index bd6fe368c5eb..37ce41d5c91e 100644 --- a/toolchain/diagnostics/coverage_test.cpp +++ b/toolchain/diagnostics/coverage_test.cpp @@ -24,8 +24,10 @@ constexpr DiagnosticKind UntestedDiagnosticKinds[] = { DiagnosticKind::TestDiagnostic, DiagnosticKind::TestDiagnosticNote, - // Driver specific. - DiagnosticKind::CppInteropFuzzing, + // Diagnosing erroneous install conditions, but test environments are + // typically correct. + DiagnosticKind::CompilePreludeManifestError, + DiagnosticKind::DriverInstallInvalid, // These diagnose filesystem issues that are hard to unit test. DiagnosticKind::ErrorReadingFile, diff --git a/toolchain/diagnostics/diagnostic.cpp b/toolchain/diagnostics/diagnostic.cpp index 6134c5988ce5..bdafa7325668 100644 --- a/toolchain/diagnostics/diagnostic.cpp +++ b/toolchain/diagnostics/diagnostic.cpp @@ -10,6 +10,9 @@ namespace Carbon { auto DiagnosticLoc::FormatLocation(llvm::raw_ostream& out) const -> void { + if (filename.empty()) { + return; + } out << filename; if (line_number > 0) { out << ":" << line_number; @@ -17,6 +20,7 @@ auto DiagnosticLoc::FormatLocation(llvm::raw_ostream& out) const -> void { out << ":" << column_number; } } + out << ": "; } auto DiagnosticLoc::FormatSnippet(llvm::raw_ostream& out, int indent) const diff --git a/toolchain/diagnostics/diagnostic.h b/toolchain/diagnostics/diagnostic.h index 80861df82ced..95e51fb9d2d7 100644 --- a/toolchain/diagnostics/diagnostic.h +++ b/toolchain/diagnostics/diagnostic.h @@ -50,8 +50,9 @@ enum class DiagnosticLevel : int8_t { // is required to be less than SourceBuffer that it refers to due to the // contained filename and line references. struct DiagnosticLoc { - // Write the filename, line number, and column number corresponding to this - // location to the given stream. + // Writes the location to the given stream. It will be formatted as + // `::: ` with parts dropped when + // unknown. auto FormatLocation(llvm::raw_ostream& out) const -> void; // Write the source snippet corresponding to this location to the given diff --git a/toolchain/diagnostics/diagnostic_consumer.cpp b/toolchain/diagnostics/diagnostic_consumer.cpp index cada656a61db..6d1a6de8d208 100644 --- a/toolchain/diagnostics/diagnostic_consumer.cpp +++ b/toolchain/diagnostics/diagnostic_consumer.cpp @@ -20,18 +20,18 @@ auto StreamDiagnosticConsumer::HandleDiagnostic(Diagnostic diagnostic) -> void { message.loc.FormatLocation(*stream_); switch (message.level) { case DiagnosticLevel::Error: - *stream_ << ": error"; + *stream_ << "error: "; break; case DiagnosticLevel::Warning: - *stream_ << ": warning"; + *stream_ << "warning: "; break; case DiagnosticLevel::Note: - *stream_ << ": note"; + *stream_ << "note: "; break; case DiagnosticLevel::LocationInfo: break; } - *stream_ << ": " << message.format_fn(message); + *stream_ << message.format_fn(message); if (include_diagnostic_kind_) { *stream_ << " [" << message.kind << "]"; } @@ -46,8 +46,7 @@ auto StreamDiagnosticConsumer::HandleDiagnostic(Diagnostic diagnostic) -> void { } auto ConsoleDiagnosticConsumer() -> DiagnosticConsumer& { - static auto* consumer = new StreamDiagnosticConsumer( - llvm::errs(), /*include_diagnostic_kind=*/false); + static auto* consumer = new StreamDiagnosticConsumer(&llvm::errs()); return *consumer; } diff --git a/toolchain/diagnostics/diagnostic_consumer.h b/toolchain/diagnostics/diagnostic_consumer.h index 39dfff115c14..dcb7aa6a33cd 100644 --- a/toolchain/diagnostics/diagnostic_consumer.h +++ b/toolchain/diagnostics/diagnostic_consumer.h @@ -39,14 +39,16 @@ class DiagnosticConsumer { // A diagnostic consumer that prints to a stream. class StreamDiagnosticConsumer : public DiagnosticConsumer { public: - explicit StreamDiagnosticConsumer(llvm::raw_ostream& stream, - bool include_diagnostic_kind) - : stream_(&stream), include_diagnostic_kind_(include_diagnostic_kind) {} + explicit StreamDiagnosticConsumer(llvm::raw_ostream* stream) + : stream_(stream) {} auto HandleDiagnostic(Diagnostic diagnostic) -> void override; auto Flush() -> void override { stream_->flush(); } auto set_stream(llvm::raw_ostream* stream) -> void { stream_ = stream; } + auto set_include_diagnostic_kind(bool value) -> void { + include_diagnostic_kind_ = value; + } private: auto Print(const DiagnosticMessage& message, llvm::StringRef prefix) -> void; @@ -54,7 +56,7 @@ class StreamDiagnosticConsumer : public DiagnosticConsumer { llvm::raw_ostream* stream_; // Whether to include the diagnostic kind when printing. - bool include_diagnostic_kind_; + bool include_diagnostic_kind_ = false; // Whethere we've printed a diagnostic. Used for printing separators. bool printed_diagnostic_ = false; diff --git a/toolchain/diagnostics/diagnostic_emitter.h b/toolchain/diagnostics/diagnostic_emitter.h index 792508e007e9..2ca96b6fd2c6 100644 --- a/toolchain/diagnostics/diagnostic_emitter.h +++ b/toolchain/diagnostics/diagnostic_emitter.h @@ -173,9 +173,12 @@ class DiagnosticEmitter { // The `converter` and `consumer` are required to outlive the diagnostic // emitter. + // TODO: Adjust construction to take stored arguments as pointers, and + // consider taking `converter` by value (move/copy) instead of reference. explicit DiagnosticEmitter(DiagnosticConverter& converter, DiagnosticConsumer& consumer) : converter_(&converter), consumer_(&consumer) {} + ~DiagnosticEmitter() = default; // Emits an error. @@ -229,6 +232,37 @@ class DiagnosticEmitter { annotate_fns_; }; +// This relies on `void*` location handling on `DiagnosticEmitter`. +// +// TODO: Based on how this ends up used or if we get more distinct emitters, it +// might be worth considering having diagnostics specify that they don't apply +// to source-location carrying emitters. For example, this might look like a +// `CARBON_NO_LOC_DIAGNOSTIC` macro, or some other factoring. But it might end +// up being more noise than it is worth. +class NoLocDiagnosticEmitter : public DiagnosticEmitter { + public: + // This constructor only applies to NoLocDiagnosticEmitter. + explicit NoLocDiagnosticEmitter(DiagnosticConsumer* consumer) + : DiagnosticEmitter(converter_, *consumer) {} + + // Emits an error. This specialization only applies to + // `NoLocDiagnosticEmitter`. + template + auto Emit(const DiagnosticBase& diagnostic_base, + Internal::NoTypeDeduction... args) -> void { + DiagnosticEmitter::Emit(nullptr, diagnostic_base, args...); + } + + private: + struct Converter : DiagnosticConverter { + auto ConvertLoc(void* /*loc*/, ContextFnT /*context_fn*/) const + -> ConvertedDiagnosticLoc override { + return {.loc = {.filename = ""}, .last_byte_offset = -1}; + } + }; + Converter converter_; +}; + // An RAII object that denotes a scope in which any diagnostic produced should // be annotated in some way. // diff --git a/toolchain/diagnostics/diagnostic_kind.def b/toolchain/diagnostics/diagnostic_kind.def index d2ff4958c61b..74bc7fd8b741 100644 --- a/toolchain/diagnostics/diagnostic_kind.def +++ b/toolchain/diagnostics/diagnostic_kind.def @@ -16,6 +16,21 @@ #error "Must define the x-macro to use this file." #endif +// ============================================================================ +// Driver and subcommand diagnostics +// ============================================================================ + +CARBON_DIAGNOSTIC_KIND(DriverInstallInvalid) +CARBON_DIAGNOSTIC_KIND(DriverCommandLineParseFailed) +CARBON_DIAGNOSTIC_KIND(ClangFuzzingDisallowed) +CARBON_DIAGNOSTIC_KIND(CompilePhaseFlagConflict) +CARBON_DIAGNOSTIC_KIND(CompilePreludeManifestError) +CARBON_DIAGNOSTIC_KIND(CompileInputNotRegularFile) +CARBON_DIAGNOSTIC_KIND(CompileOutputFileOpenError) +CARBON_DIAGNOSTIC_KIND(FormatMultipleFilesToOneOutput) +CARBON_DIAGNOSTIC_KIND(LanguageServerMissingInputStream) +CARBON_DIAGNOSTIC_KIND(LanguageServerTransportError) + // ============================================================================ // SourceBuffer diagnostics // ============================================================================ diff --git a/toolchain/diagnostics/file_diagnostics.h b/toolchain/diagnostics/file_diagnostics.h new file mode 100644 index 000000000000..45d75dccd560 --- /dev/null +++ b/toolchain/diagnostics/file_diagnostics.h @@ -0,0 +1,37 @@ +// 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 + +#ifndef CARBON_TOOLCHAIN_DIAGNOSTICS_FILE_DIAGNOSTICS_H_ +#define CARBON_TOOLCHAIN_DIAGNOSTICS_FILE_DIAGNOSTICS_H_ + +#include "toolchain/diagnostics/diagnostic_emitter.h" + +namespace Carbon { + +// We frequently want a `DiagnosticEmitter` that directly uses a filename. Note +// that an empty string can be used for a diagnostic that has no particular +// location. +// +// Note this provides no way to set a line or column on diagnostics. More +// specific emitters must be used for that. +class FileDiagnosticEmitter : public DiagnosticEmitter { + public: + explicit FileDiagnosticEmitter(DiagnosticConsumer* consumer) + : DiagnosticEmitter(converter_, *consumer) {} + + private: + // Converts a filename directly to the diagnostic location. + struct FileDiagnosticConverter : DiagnosticConverter { + auto ConvertLoc(llvm::StringRef filename, ContextFnT /*context_fn*/) const + -> ConvertedDiagnosticLoc override { + return {.loc = {.filename = filename}, .last_byte_offset = -1}; + } + }; + + FileDiagnosticConverter converter_; +}; + +} // namespace Carbon + +#endif // CARBON_TOOLCHAIN_DIAGNOSTICS_FILE_DIAGNOSTICS_H_ diff --git a/toolchain/diagnostics/testdata/fail_multiline_token.carbon b/toolchain/diagnostics/testdata/fail_multiline_token.carbon index 2c759d421b91..c01a736e1602 100644 --- a/toolchain/diagnostics/testdata/fail_multiline_token.carbon +++ b/toolchain/diagnostics/testdata/fail_multiline_token.carbon @@ -2,7 +2,7 @@ // Exceptions. See /LICENSE for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // -// ARGS: compile --phase=parse %s +// ARGS: --include-diagnostic-kind compile --phase=parse %s // // AUTOUPDATE // TIP: To test this file alone, run: @@ -10,7 +10,7 @@ // TIP: To dump output, run: // TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/diagnostics/testdata/fail_multiline_token.carbon -// CHECK:STDERR: fail_multiline_token.carbon:[[@LINE+4]]:1: error: unrecognized declaration introducer +// CHECK:STDERR: fail_multiline_token.carbon:[[@LINE+4]]:1: error: unrecognized declaration introducer [UnrecognizedDecl] // CHECK:STDERR: ''' // CHECK:STDERR: ^~~ // CHECK:STDERR: diff --git a/toolchain/driver/clang_subcommand.cpp b/toolchain/driver/clang_subcommand.cpp index 3d43eae846d0..6c1e39abe126 100644 --- a/toolchain/driver/clang_subcommand.cpp +++ b/toolchain/driver/clang_subcommand.cpp @@ -51,8 +51,10 @@ 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 - << "error: cannot run `clang` subcommand productively when fuzzing\n"; + CARBON_DIAGNOSTIC( + ClangFuzzingDisallowed, Error, + "preventing fuzzing of `clang` subcommand due to library crashes"); + driver_env.emitter.Emit(ClangFuzzingDisallowed); return {.success = false}; } diff --git a/toolchain/driver/compile_subcommand.cpp b/toolchain/driver/compile_subcommand.cpp index f438d0f360f6..321457b53291 100644 --- a/toolchain/driver/compile_subcommand.cpp +++ b/toolchain/driver/compile_subcommand.cpp @@ -11,6 +11,7 @@ #include "toolchain/base/timings.h" #include "toolchain/check/check.h" #include "toolchain/codegen/codegen.h" +#include "toolchain/diagnostics/diagnostic_emitter.h" #include "toolchain/diagnostics/sorting_diagnostic_consumer.h" #include "toolchain/lex/lex.h" #include "toolchain/lower/lower.h" @@ -22,28 +23,6 @@ namespace Carbon { -auto operator<<(llvm::raw_ostream& out, CompileOptions::Phase phase) - -> llvm::raw_ostream& { - switch (phase) { - case CompileOptions::Phase::Lex: - out << "lex"; - break; - case CompileOptions::Phase::Parse: - out << "parse"; - break; - case CompileOptions::Phase::Check: - out << "check"; - break; - case CompileOptions::Phase::Lower: - out << "lower"; - break; - case CompileOptions::Phase::CodeGen: - out << "codegen"; - break; - } - return out; -} - auto CompileOptions::Build(CommandLine::CommandBuilder& b) -> void { b.AddStringPositionalArg( { @@ -128,15 +107,6 @@ of a binary object file instead. Ignored for other `--output` values. }, [&](auto& arg_b) { arg_b.Set(&force_obj_output); }); - b.AddFlag( - { - .name = "include-diagnostic-kind", - .help = R"""( -When printing diagnostics, include the diagnostic kind as part of output. This -applies to each message that forms a diagnostic, not just the primary message. -)""", - }, - [&](auto& arg_b) { arg_b.Set(&include_diagnostic_kind); }); b.AddFlag( { .name = "stream-errors", @@ -296,30 +266,49 @@ can be written to standard output as these phases progress. CompileSubcommand::CompileSubcommand() : DriverSubcommand(SubcommandInfo) {} -// 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)); +// Returns a string for printing the phase in a diagnostic. +static auto PhaseToString(CompileOptions::Phase phase) -> std::string { + switch (phase) { + case CompileOptions::Phase::Lex: + return "lex"; + case CompileOptions::Phase::Parse: + return "parse"; + case CompileOptions::Phase::Check: + return "check"; + case CompileOptions::Phase::Lower: + return "lower"; + case CompileOptions::Phase::CodeGen: + return "codegen"; + } } -auto CompileSubcommand::ValidateOptions() const -> ErrorOr { +auto CompileSubcommand::ValidateOptions(NoLocDiagnosticEmitter& emitter) const + -> bool { + CARBON_DIAGNOSTIC( + CompilePhaseFlagConflict, Error, + "requested dumping {0} but compile phase is limited to `{1}`", + std::string, std::string); using Phase = CompileOptions::Phase; switch (options_.phase) { case Phase::Lex: if (options_.dump_parse_tree) { - return DumpPhaseError("parse tree", options_.phase); + emitter.Emit(CompilePhaseFlagConflict, "parse tree", + PhaseToString(options_.phase)); + return false; } [[fallthrough]]; case Phase::Parse: if (options_.dump_sem_ir) { - return DumpPhaseError("SemIR", options_.phase); + emitter.Emit(CompilePhaseFlagConflict, "SemIR", + PhaseToString(options_.phase)); + return false; } [[fallthrough]]; case Phase::Check: if (options_.dump_llvm_ir) { - return DumpPhaseError("LLVM IR", options_.phase); + emitter.Emit(CompilePhaseFlagConflict, "LLVM IR", + PhaseToString(options_.phase)); + return false; } [[fallthrough]]; case Phase::Lower: @@ -327,7 +316,7 @@ auto CompileSubcommand::ValidateOptions() const -> ErrorOr { // Everything can be dumped in these phases. break; } - return Success(); + return true; } namespace { @@ -673,9 +662,13 @@ 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 - << "error: output file name must be specified for input `" - << input_filename_ << "` that is not a regular file\n"; + // TODO: Consider rephrasing the diagnostic to use the file as the + // `Emit` location. + CARBON_DIAGNOSTIC(CompileInputNotRegularFile, Error, + "output file name must be specified for input `{0}` " + "that is not a regular file", + std::string); + driver_env_->emitter.Emit(CompileInputNotRegularFile, input_filename_); return false; } output_filename = input_filename_; @@ -694,9 +687,13 @@ 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"; + // TODO: Consider rephrasing the diagnostic to use the file as the `Emit` + // location. + CARBON_DIAGNOSTIC(CompileOutputFileOpenError, Error, + "could not open output file `{0}`: {1}", std::string, + std::string); + driver_env_->emitter.Emit(CompileOutputFileOpenError, + output_filename.str().str(), ec.message()); return false; } if (options_.asm_output) { @@ -745,8 +742,7 @@ auto CompilationUnit::IncludeInDumps(llvm::StringRef filename) const -> bool { } // namespace auto CompileSubcommand::Run(DriverEnv& driver_env) -> DriverResult { - if (auto validate = ValidateOptions(); !validate.ok()) { - *driver_env.error_stream << "error: " << validate.error() << "\n"; + if (!ValidateOptions(driver_env.emitter)) { return {.success = false}; } @@ -759,27 +755,28 @@ 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"; + // TODO: Change ReadPreludeManifest to produce diagnostics. + CARBON_DIAGNOSTIC(CompilePreludeManifestError, Error, "{0}", std::string); + driver_env.emitter.Emit(CompilePreludeManifestError, + PrintToString(find.error())); return {.success = false}; } } // Prepare CompilationUnits before building scope exit handlers. - StreamDiagnosticConsumer stream_consumer(*driver_env.error_stream, - options_.include_diagnostic_kind); llvm::SmallVector> units; units.reserve(prelude.size() + options_.input_filenames.size()); // Add the prelude files. for (const auto& input_filename : prelude) { units.push_back(std::make_unique( - driver_env, options_, &stream_consumer, input_filename)); + driver_env, options_, &driver_env.consumer, input_filename)); } // Add the input source files. for (const auto& input_filename : options_.input_filenames) { units.push_back(std::make_unique( - driver_env, options_, &stream_consumer, input_filename)); + driver_env, options_, &driver_env.consumer, input_filename)); } auto on_exit = llvm::make_scope_exit([&]() { @@ -789,7 +786,7 @@ auto CompileSubcommand::Run(DriverEnv& driver_env) -> DriverResult { unit->PostCompile(); } - stream_consumer.Flush(); + driver_env.consumer.Flush(); }); PrettyStackTraceFunction flush_on_crash([&](llvm::raw_ostream& out) { @@ -801,14 +798,14 @@ auto CompileSubcommand::Run(DriverEnv& driver_env) -> DriverResult { out << "Flushing diagnostics\n"; } else { out << "Pending diagnostics:\n"; - stream_consumer.set_stream(&out); + driver_env.consumer.set_stream(&out); } for (auto& unit : units) { unit->FlushForStackTrace(); } - stream_consumer.Flush(); - stream_consumer.set_stream(driver_env.error_stream); + driver_env.consumer.Flush(); + driver_env.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 de78e9a9aa01..b51469c753c9 100644 --- a/toolchain/driver/compile_subcommand.h +++ b/toolchain/driver/compile_subcommand.h @@ -52,7 +52,6 @@ struct CompileOptions { bool dump_asm = false; bool dump_mem_usage = false; bool dump_timings = false; - bool include_diagnostic_kind = false; bool stream_errors = false; bool preorder_parse_tree = false; bool builtin_sem_ir = false; @@ -75,8 +74,9 @@ class CompileSubcommand : public DriverSubcommand { private: // Does custom validation of the compile-subcommand options structure beyond - // what the command line parsing library supports. - auto ValidateOptions() const -> ErrorOr; + // what the command line parsing library supports. Diagnoses and returns false + // on failure. + auto ValidateOptions(NoLocDiagnosticEmitter& emitter) const -> bool; CompileOptions options_; }; diff --git a/toolchain/driver/driver.cpp b/toolchain/driver/driver.cpp index 762f87dcc4ae..e0bd0381e0eb 100644 --- a/toolchain/driver/driver.cpp +++ b/toolchain/driver/driver.cpp @@ -24,8 +24,9 @@ struct Options { auto Build(CommandLine::CommandBuilder& b) -> void; - bool verbose; - bool fuzzing; + bool verbose = false; + bool fuzzing = false; + bool include_diagnostic_kind = false; ClangSubcommand clang; CompileSubcommand compile; @@ -73,6 +74,16 @@ auto Options::Build(CommandLine::CommandBuilder& b) -> void { }, [&](CommandLine::FlagBuilder& arg_b) { arg_b.Set(&fuzzing); }); + b.AddFlag( + { + .name = "include-diagnostic-kind", + .help = R"""( +When printing diagnostics, include the diagnostic kind as part of output. This +applies to each message that forms a diagnostic, not just the primary message. +)""", + }, + [&](auto& arg_b) { arg_b.Set(&include_diagnostic_kind); }); + clang.AddTo(b, &selected_subcommand); compile.AddTo(b, &selected_subcommand); format.AddTo(b, &selected_subcommand); @@ -84,8 +95,9 @@ 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"; + CARBON_DIAGNOSTIC(DriverInstallInvalid, Error, "{0}", std::string); + driver_env_.emitter.Emit(DriverInstallInvalid, + driver_env_.installation->error()->str()); return {.success = false}; } @@ -95,8 +107,15 @@ auto Driver::RunCommand(llvm::ArrayRef args) -> DriverResult { args, *driver_env_.output_stream, Options::Info, [&](CommandLine::CommandBuilder& b) { options.Build(b); }); + // Regardless of whether the parse succeeded, try to use the diagnostic kind + // flag. + driver_env_.consumer.set_include_diagnostic_kind( + options.include_diagnostic_kind); + if (!result.ok()) { - *driver_env_.error_stream << "error: " << result.error() << "\n"; + CARBON_DIAGNOSTIC(DriverCommandLineParseFailed, Error, "{0}", std::string); + driver_env_.emitter.Emit(DriverCommandLineParseFailed, + PrintToString(result.error())); return {.success = false}; } else if (*result == CommandLine::ParseResult::MetaSuccess) { return {.success = true}; diff --git a/toolchain/driver/driver_env.h b/toolchain/driver/driver_env.h index 4e48e41c24fb..bf71a4f2a6fc 100644 --- a/toolchain/driver/driver_env.h +++ b/toolchain/driver/driver_env.h @@ -6,9 +6,11 @@ #define CARBON_TOOLCHAIN_DRIVER_DRIVER_ENV_H_ #include +#include #include "common/ostream.h" #include "llvm/Support/VirtualFileSystem.h" +#include "toolchain/diagnostics/diagnostic_emitter.h" #include "toolchain/install/install_paths.h" namespace Carbon { @@ -24,7 +26,9 @@ struct DriverEnv { input_stream(input_stream), output_stream(output_stream), error_stream(error_stream), - fuzzing(fuzzing) {} + fuzzing(fuzzing), + consumer(error_stream), + emitter(&consumer) {} // The filesystem for source code. llvm::IntrusiveRefCntPtr fs; @@ -39,13 +43,19 @@ struct DriverEnv { // Error output; stderr. llvm::raw_pwrite_stream* error_stream; - // For CARBON_VLOG. - llvm::raw_pwrite_stream* vlog_stream = nullptr; - // 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; + + // A diagnostic consumer, to be able to connect output. + StreamDiagnosticConsumer consumer; + + // A diagnostic emitter that has no locations. + NoLocDiagnosticEmitter emitter; + + // For CARBON_VLOG. + llvm::raw_pwrite_stream* vlog_stream = nullptr; }; } // namespace Carbon diff --git a/toolchain/driver/driver_test.cpp b/toolchain/driver/driver_test.cpp index cec1790804f6..3c7188d32c75 100644 --- a/toolchain/driver/driver_test.cpp +++ b/toolchain/driver/driver_test.cpp @@ -225,10 +225,5 @@ TEST_F(DriverTest, FileOutput) { EXPECT_THAT(ReadFile("test.s"), ContainsRegex("Main:")); } -TEST_F(DriverTest, LanguageServerNoStdin) { - EXPECT_FALSE(driver_.RunCommand({"language-server"}).success); - EXPECT_THAT(test_error_stream_.TakeStr(), HasSubstr("requires input_stream")); -} - } // namespace } // namespace Carbon diff --git a/toolchain/driver/format_subcommand.cpp b/toolchain/driver/format_subcommand.cpp index cb9e40b20824..3d684d1b5faf 100644 --- a/toolchain/driver/format_subcommand.cpp +++ b/toolchain/driver/format_subcommand.cpp @@ -56,9 +56,10 @@ 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"; + CARBON_DIAGNOSTIC(FormatMultipleFilesToOneOutput, Error, + "multiple input files are being provided; --output only " + "works with one input"); + driver_env.emitter.Emit(FormatMultipleFilesToOneOutput); result.success = false; return result; } @@ -68,22 +69,20 @@ auto FormatSubcommand::Run(DriverEnv& driver_env) -> DriverResult { result.per_file_success.back().second = false; }; - 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. result.per_file_success.push_back({f.str(), true}); // TODO: Consider refactoring this for sharing with compile. // TODO: Decide what to do with `-` when there are multiple arguments. - auto source = - SourceBuffer::MakeFromFileOrStdin(*driver_env.fs, f, consumer); + auto source = SourceBuffer::MakeFromFileOrStdin(*driver_env.fs, f, + driver_env.consumer); if (!source) { mark_per_file_error(); continue; } SharedValueStores value_stores; - auto tokens = Lex::Lex(value_stores, *source, consumer); + auto tokens = Lex::Lex(value_stores, *source, driver_env.consumer); RawStringOstream buffer; if (Format::Format(tokens, buffer)) { diff --git a/toolchain/driver/language_server_subcommand.cpp b/toolchain/driver/language_server_subcommand.cpp index a93eefc5a88f..ef88aad89ad9 100644 --- a/toolchain/driver/language_server_subcommand.cpp +++ b/toolchain/driver/language_server_subcommand.cpp @@ -4,6 +4,7 @@ #include "toolchain/driver/language_server_subcommand.h" +#include "toolchain/diagnostics/diagnostic_consumer.h" #include "toolchain/language_server/language_server.h" namespace Carbon { @@ -20,18 +21,16 @@ LanguageServerSubcommand::LanguageServerSubcommand() auto LanguageServerSubcommand::Run(DriverEnv& driver_env) -> DriverResult { if (!driver_env.input_stream) { - *driver_env.error_stream - << "error: language-server requires input_stream\n"; + CARBON_DIAGNOSTIC(LanguageServerMissingInputStream, Error, + "language-server requires input_stream"); + driver_env.emitter.Emit(LanguageServerMissingInputStream); return {.success = false}; } - auto err = + bool success = 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"; - } - return {.success = err.ok()}; + *driver_env.error_stream, driver_env.consumer); + return {.success = success}; } } // namespace Carbon diff --git a/toolchain/driver/testdata/compile/multifile_raw_and_textual_ir.carbon b/toolchain/driver/testdata/compile/multifile_raw_and_textual_ir.carbon index e0e4830b4a3e..de213df848a3 100644 --- a/toolchain/driver/testdata/compile/multifile_raw_and_textual_ir.carbon +++ b/toolchain/driver/testdata/compile/multifile_raw_and_textual_ir.carbon @@ -2,7 +2,7 @@ // Exceptions. See /LICENSE for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // -// ARGS: compile --no-prelude-import --phase=check --dump-sem-ir --dump-raw-sem-ir %s +// ARGS: --include-diagnostic-kind compile --no-prelude-import --phase=check --dump-sem-ir --dump-raw-sem-ir %s // // Check that we can combine textual IR and raw IR dumping in one compile. // diff --git a/toolchain/driver/testdata/compile/multifile_raw_ir.carbon b/toolchain/driver/testdata/compile/multifile_raw_ir.carbon index 5a21fcba7632..8254d38a0e51 100644 --- a/toolchain/driver/testdata/compile/multifile_raw_ir.carbon +++ b/toolchain/driver/testdata/compile/multifile_raw_ir.carbon @@ -2,7 +2,7 @@ // Exceptions. See /LICENSE for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // -// ARGS: compile --no-prelude-import --phase=check --dump-raw-sem-ir %s +// ARGS: --include-diagnostic-kind compile --no-prelude-import --phase=check --dump-raw-sem-ir %s // // Check that raw IR dumping works as expected. // diff --git a/toolchain/driver/testdata/compile/raw_and_textual_ir.carbon b/toolchain/driver/testdata/compile/raw_and_textual_ir.carbon index 256c02447f9e..138073812eb5 100644 --- a/toolchain/driver/testdata/compile/raw_and_textual_ir.carbon +++ b/toolchain/driver/testdata/compile/raw_and_textual_ir.carbon @@ -2,7 +2,7 @@ // Exceptions. See /LICENSE for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // -// ARGS: compile --no-prelude-import --phase=check --dump-sem-ir --dump-raw-sem-ir %s +// ARGS: --include-diagnostic-kind compile --no-prelude-import --phase=check --dump-sem-ir --dump-raw-sem-ir %s // // Check that we can combine textual IR and raw IR dumping in one compile. // diff --git a/toolchain/driver/testdata/compile/raw_ir.carbon b/toolchain/driver/testdata/compile/raw_ir.carbon index 17d4bb2dde51..900e3d6b3c3c 100644 --- a/toolchain/driver/testdata/compile/raw_ir.carbon +++ b/toolchain/driver/testdata/compile/raw_ir.carbon @@ -2,7 +2,7 @@ // Exceptions. See /LICENSE for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // -// ARGS: compile --no-prelude-import --phase=check --dump-raw-sem-ir %s +// ARGS: --include-diagnostic-kind compile --no-prelude-import --phase=check --dump-raw-sem-ir %s // // Check that raw IR dumping works as expected. // diff --git a/toolchain/driver/testdata/compile/textual_ir.carbon b/toolchain/driver/testdata/compile/textual_ir.carbon index 1e584e27a39c..31d3a9b6eb32 100644 --- a/toolchain/driver/testdata/compile/textual_ir.carbon +++ b/toolchain/driver/testdata/compile/textual_ir.carbon @@ -2,7 +2,7 @@ // Exceptions. See /LICENSE for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // -// ARGS: compile --no-prelude-import --phase=check --dump-sem-ir %s +// ARGS: --include-diagnostic-kind compile --no-prelude-import --phase=check --dump-sem-ir %s // // Check that the command-line flag to dump textual IR works. // diff --git a/toolchain/driver/testdata/dump_mem_usage.carbon b/toolchain/driver/testdata/dump_mem_usage.carbon index caf71a21d05c..ef580bc91b04 100644 --- a/toolchain/driver/testdata/dump_mem_usage.carbon +++ b/toolchain/driver/testdata/dump_mem_usage.carbon @@ -2,7 +2,7 @@ // Exceptions. See /LICENSE for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // -// ARGS: compile --phase=check --dump-mem-usage %s +// ARGS: --include-diagnostic-kind compile --phase=check --dump-mem-usage %s // // To test this file alone, run: // bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/driver/testdata/dump_mem_usage.carbon diff --git a/toolchain/driver/testdata/dump_shared_values.carbon b/toolchain/driver/testdata/dump_shared_values.carbon index e13e938ed4b8..aa8b8062a533 100644 --- a/toolchain/driver/testdata/dump_shared_values.carbon +++ b/toolchain/driver/testdata/dump_shared_values.carbon @@ -2,7 +2,7 @@ // Exceptions. See /LICENSE for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // -// ARGS: compile --phase=lex --dump-shared-values %s +// ARGS: --include-diagnostic-kind compile --phase=lex --dump-shared-values %s // // AUTOUPDATE // TIP: To test this file alone, run: diff --git a/toolchain/driver/testdata/dump_timings.carbon b/toolchain/driver/testdata/dump_timings.carbon index b7f809c62dab..d0d4d31afea7 100644 --- a/toolchain/driver/testdata/dump_timings.carbon +++ b/toolchain/driver/testdata/dump_timings.carbon @@ -2,7 +2,7 @@ // Exceptions. See /LICENSE for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // -// ARGS: compile --dump-timings --phase=check %s +// ARGS: --include-diagnostic-kind compile --dump-timings --phase=check %s // // SET-CHECK-SUBSET // diff --git a/toolchain/driver/testdata/fail_clang_fuzzing.cpp b/toolchain/driver/testdata/fail_clang_fuzzing.cpp new file mode 100644 index 000000000000..8843dd462778 --- /dev/null +++ b/toolchain/driver/testdata/fail_clang_fuzzing.cpp @@ -0,0 +1,15 @@ +// 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: --include-diagnostic-kind --fuzzing clang foo.cpp +// +// SET-CAPTURE-CONSOLE-OUTPUT +// clang-format off +// AUTOUPDATE +// TIP: To test this file alone, run: +// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/driver/testdata/fail_clang_fuzzing.cpp +// TIP: To dump output, run: +// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/driver/testdata/fail_clang_fuzzing.cpp +// CHECK:STDERR: error: preventing fuzzing of `clang` subcommand due to library crashes [ClangFuzzingDisallowed] +// CHECK:STDERR: diff --git a/toolchain/driver/testdata/fail_clang_no_args.cpp b/toolchain/driver/testdata/fail_clang_no_args.cpp index acb1917c0046..b0608a1e1845 100644 --- a/toolchain/driver/testdata/fail_clang_no_args.cpp +++ b/toolchain/driver/testdata/fail_clang_no_args.cpp @@ -2,7 +2,7 @@ // Exceptions. See /LICENSE for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // -// ARGS: clang -- +// ARGS: --include-diagnostic-kind clang -- // // SET-CAPTURE-CONSOLE-OUTPUT // clang-format off diff --git a/toolchain/driver/testdata/fail_dump_phase_conflict.carbon b/toolchain/driver/testdata/fail_dump_phase_conflict.carbon new file mode 100644 index 000000000000..13cf153fd92d --- /dev/null +++ b/toolchain/driver/testdata/fail_dump_phase_conflict.carbon @@ -0,0 +1,13 @@ +// 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: --include-diagnostic-kind compile --phase=parse --dump-sem-ir %s +// +// AUTOUPDATE +// TIP: To test this file alone, run: +// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/driver/testdata/fail_dump_phase_conflict.carbon +// TIP: To dump output, run: +// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/driver/testdata/fail_dump_phase_conflict.carbon +// CHECK:STDERR: error: requested dumping SemIR but compile phase is limited to `parse` [CompilePhaseFlagConflict] +// CHECK:STDERR: diff --git a/toolchain/driver/testdata/fail_flag.carbon b/toolchain/driver/testdata/fail_flag.carbon index 7fb726990647..552750d30ad2 100644 --- a/toolchain/driver/testdata/fail_flag.carbon +++ b/toolchain/driver/testdata/fail_flag.carbon @@ -2,12 +2,12 @@ // Exceptions. See /LICENSE for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // -// ARGS: compile --non-existent-flag +// ARGS: --include-diagnostic-kind compile --non-existent-flag // // AUTOUPDATE // TIP: To test this file alone, run: // TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/driver/testdata/fail_flag.carbon // TIP: To dump output, run: // TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/driver/testdata/fail_flag.carbon -// CHECK:STDERR: error: unknown option `--non-existent-flag` +// CHECK:STDERR: error: unknown option `--non-existent-flag` [DriverCommandLineParseFailed] // CHECK:STDERR: diff --git a/toolchain/driver/testdata/fail_flush_errors.carbon b/toolchain/driver/testdata/fail_flush_errors.carbon index 3f9ad2ff65da..bed01311cc50 100644 --- a/toolchain/driver/testdata/fail_flush_errors.carbon +++ b/toolchain/driver/testdata/fail_flush_errors.carbon @@ -2,7 +2,7 @@ // Exceptions. See /LICENSE for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // -// ARGS: compile --no-prelude-import %s +// ARGS: --include-diagnostic-kind compile --no-prelude-import %s // // AUTOUPDATE // TIP: To test this file alone, run: @@ -13,7 +13,7 @@ fn F() { // Create diagnostics containing string references, and trigger reallocation // of the string table. - // CHECK:STDERR: fail_flush_errors.carbon:[[@LINE+4]]:3: error: name `undeclared1` not found + // CHECK:STDERR: fail_flush_errors.carbon:[[@LINE+4]]:3: error: name `undeclared1` not found [NameNotFound] // CHECK:STDERR: undeclared1; // CHECK:STDERR: ^~~~~~~~~~~ // CHECK:STDERR: @@ -23,7 +23,7 @@ fn F() { // literal storage. Use a hex escape to ensure that the tokenized buffer // allocates separate storage for the result. "undec\x6Cared2"; - // CHECK:STDERR: fail_flush_errors.carbon:[[@LINE+4]]:3: error: name `undeclared2` not found + // CHECK:STDERR: fail_flush_errors.carbon:[[@LINE+4]]:3: error: name `undeclared2` not found [NameNotFound] // CHECK:STDERR: undeclared2; // CHECK:STDERR: ^~~~~~~~~~~ // CHECK:STDERR: @@ -31,7 +31,7 @@ fn F() { // Add the name into the string table via a declaration rather than an expression. if (true) { var undeclared3: () = (); } - // CHECK:STDERR: fail_flush_errors.carbon:[[@LINE+4]]:3: error: name `undeclared3` not found + // CHECK:STDERR: fail_flush_errors.carbon:[[@LINE+4]]:3: error: name `undeclared3` not found [NameNotFound] // CHECK:STDERR: undeclared3; // CHECK:STDERR: ^~~~~~~~~~~ // CHECK:STDERR: diff --git a/toolchain/driver/testdata/fail_input_is_directory.carbon b/toolchain/driver/testdata/fail_input_is_directory.carbon index 2b9411de59db..1a699dad49c9 100644 --- a/toolchain/driver/testdata/fail_input_is_directory.carbon +++ b/toolchain/driver/testdata/fail_input_is_directory.carbon @@ -2,12 +2,12 @@ // Exceptions. See /LICENSE for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // -// ARGS: compile . +// ARGS: --include-diagnostic-kind compile . // // AUTOUPDATE // TIP: To test this file alone, run: // TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/driver/testdata/fail_input_is_directory.carbon // TIP: To dump output, run: // TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/driver/testdata/fail_input_is_directory.carbon -// CHECK:STDERR: .: error: error opening file for read: Invalid argument +// CHECK:STDERR: .: error: error opening file for read: Invalid argument [ErrorOpeningFile] // CHECK:STDERR: diff --git a/toolchain/driver/testdata/fail_missing_file.carbon b/toolchain/driver/testdata/fail_missing_file.carbon index 64891b558ee2..58b48e314243 100644 --- a/toolchain/driver/testdata/fail_missing_file.carbon +++ b/toolchain/driver/testdata/fail_missing_file.carbon @@ -2,7 +2,7 @@ // Exceptions. See /LICENSE for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // -// ARGS: compile --include-diagnostic-kind not_file.carbon +// ARGS: --include-diagnostic-kind compile not_file.carbon // // AUTOUPDATE // TIP: To test this file alone, run: diff --git a/toolchain/driver/testdata/fail_missing_stdin_output.carbon b/toolchain/driver/testdata/fail_missing_stdin_output.carbon index 0be0fcdeb2e4..b94a73a67f2a 100644 --- a/toolchain/driver/testdata/fail_missing_stdin_output.carbon +++ b/toolchain/driver/testdata/fail_missing_stdin_output.carbon @@ -2,12 +2,12 @@ // Exceptions. See /LICENSE for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // -// ARGS: compile --no-prelude-import - +// ARGS: --include-diagnostic-kind compile --no-prelude-import - // // AUTOUPDATE // TIP: To test this file alone, run: // TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/driver/testdata/fail_missing_stdin_output.carbon // TIP: To dump output, run: // TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/driver/testdata/fail_missing_stdin_output.carbon -// CHECK:STDERR: error: output file name must be specified for input `-` that is not a regular file +// CHECK:STDERR: error: output file name must be specified for input `-` that is not a regular file [CompileInputNotRegularFile] // CHECK:STDERR: diff --git a/toolchain/driver/testdata/fail_output_is_directory.carbon b/toolchain/driver/testdata/fail_output_is_directory.carbon new file mode 100644 index 000000000000..15d0641a4958 --- /dev/null +++ b/toolchain/driver/testdata/fail_output_is_directory.carbon @@ -0,0 +1,15 @@ +// 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: --include-diagnostic-kind compile --no-prelude-import --output=/ %s +// +// NOAUTOUPDATE +// TIP: To test this file alone, run: +// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/driver/testdata/fail_output_is_directory.carbon +// TIP: To dump output, run: +// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/driver/testdata/fail_output_is_directory.carbon + +// Don't depend on a specific filesystem error. +// CHECK:STDERR: error: could not open output file `/`: {{.*}} [CompileOutputFileOpenError] +// CHECK:STDERR: diff --git a/toolchain/driver/testdata/stdin.carbon b/toolchain/driver/testdata/stdin.carbon index d9173db3456e..13f19accfb30 100644 --- a/toolchain/driver/testdata/stdin.carbon +++ b/toolchain/driver/testdata/stdin.carbon @@ -2,7 +2,7 @@ // Exceptions. See /LICENSE for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // -// ARGS: compile - --no-prelude-import --phase=check --dump-sem-ir +// ARGS: --include-diagnostic-kind compile - --no-prelude-import --phase=check --dump-sem-ir // // AUTOUPDATE // TIP: To test this file alone, run: diff --git a/toolchain/format/testdata/basics/fail_invalid_comment.carbon b/toolchain/format/testdata/basics/fail_invalid_comment.carbon index 3e38551e778f..60f16dd132d6 100644 --- a/toolchain/format/testdata/basics/fail_invalid_comment.carbon +++ b/toolchain/format/testdata/basics/fail_invalid_comment.carbon @@ -14,7 +14,7 @@ // --- AUTOUPDATE-SPLIT -// CHECK:STDERR: fail_test.carbon:2:3: error: whitespace is required after '//' +// CHECK:STDERR: fail_test.carbon:2:3: error: whitespace is required after '//' [NoWhitespaceAfterCommentIntroducer] // CHECK:STDERR: //f // CHECK:STDERR: ^ // CHECK:STDERR: diff --git a/toolchain/format/testdata/basics/fail_multi_file_one_output.carbon b/toolchain/format/testdata/basics/fail_multi_file_one_output.carbon new file mode 100644 index 000000000000..e321984a0954 --- /dev/null +++ b/toolchain/format/testdata/basics/fail_multi_file_one_output.carbon @@ -0,0 +1,12 @@ +// 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: --include-diagnostic-kind format --output=foo.carbon bar.carbon baz.carbon +// AUTOUPDATE +// TIP: To test this file alone, run: +// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/format/testdata/basics/fail_multi_file_one_output.carbon +// TIP: To dump output, run: +// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/format/testdata/basics/fail_multi_file_one_output.carbon +// CHECK:STDERR: error: multiple input files are being provided; --output only works with one input [FormatMultipleFilesToOneOutput] +// CHECK:STDERR: diff --git a/toolchain/format/testdata/basics/fail_nonexistent.carbon b/toolchain/format/testdata/basics/fail_nonexistent.carbon index cc274933d14b..ffe580af4194 100644 --- a/toolchain/format/testdata/basics/fail_nonexistent.carbon +++ b/toolchain/format/testdata/basics/fail_nonexistent.carbon @@ -2,11 +2,11 @@ // Exceptions. See /LICENSE for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // -// ARGS: format fail_target_file.carbon +// ARGS: --include-diagnostic-kind format fail_target_file.carbon // AUTOUPDATE // TIP: To test this file alone, run: // TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/format/testdata/basics/fail_nonexistent.carbon // TIP: To dump output, run: // TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/format/testdata/basics/fail_nonexistent.carbon -// CHECK:STDERR: fail_target_file.carbon: error: error opening file for read: No such file or directory +// CHECK:STDERR: fail_target_file.carbon: error: error opening file for read: No such file or directory [ErrorOpeningFile] // CHECK:STDERR: diff --git a/toolchain/language_server/BUILD b/toolchain/language_server/BUILD index 24684c1014b0..0da239e43395 100644 --- a/toolchain/language_server/BUILD +++ b/toolchain/language_server/BUILD @@ -19,9 +19,9 @@ cc_library( ":context", ":incoming_messages", ":outgoing_messages", - "//common:error", "//common:ostream", "//common:raw_string_ostream", + "//toolchain/diagnostics:diagnostic_emitter", "@llvm-project//clang-tools-extra/clangd:ClangDaemon", ], ) diff --git a/toolchain/language_server/language_server.cpp b/toolchain/language_server/language_server.cpp index aff99f614f41..02e02f6dfd07 100644 --- a/toolchain/language_server/language_server.cpp +++ b/toolchain/language_server/language_server.cpp @@ -8,6 +8,7 @@ #include "clang-tools-extra/clangd/Transport.h" #include "clang-tools-extra/clangd/support/Logger.h" #include "common/raw_string_ostream.h" +#include "toolchain/diagnostics/diagnostic_emitter.h" #include "toolchain/language_server/context.h" #include "toolchain/language_server/incoming_messages.h" #include "toolchain/language_server/outgoing_messages.h" @@ -15,7 +16,8 @@ namespace Carbon::LanguageServer { auto Run(FILE* input_stream, llvm::raw_ostream& output_stream, - llvm::raw_ostream& error_stream) -> ErrorOr { + llvm::raw_ostream& error_stream, DiagnosticConsumer& consumer) + -> bool { // TODO: Consider implementing a custom logger that splits vlog to // vlog_stream when provided. For now, this disables verbose logging. clang::clangd::StreamLogger logger(error_stream, clang::clangd::Logger::Info); @@ -36,10 +38,12 @@ auto Run(FILE* input_stream, llvm::raw_ostream& output_stream, if (err) { RawStringOstream out; out << err; - return Error(out.TakeStr()); - } else { - return Success(); + CARBON_DIAGNOSTIC(LanguageServerTransportError, Error, "{0}", std::string); + NoLocDiagnosticEmitter emitter(&consumer); + emitter.Emit(LanguageServerTransportError, out.TakeStr()); + return false; } + return true; } } // namespace Carbon::LanguageServer diff --git a/toolchain/language_server/language_server.h b/toolchain/language_server/language_server.h index b48a01ce3eab..1836fdb660f3 100644 --- a/toolchain/language_server/language_server.h +++ b/toolchain/language_server/language_server.h @@ -5,15 +5,16 @@ #ifndef CARBON_TOOLCHAIN_LANGUAGE_SERVER_LANGUAGE_SERVER_H_ #define CARBON_TOOLCHAIN_LANGUAGE_SERVER_LANGUAGE_SERVER_H_ -#include "common/error.h" #include "common/ostream.h" +#include "toolchain/diagnostics/diagnostic_consumer.h" namespace Carbon::LanguageServer { // 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. +// error_stream is primarily for errors that don't fit into LSP. Returns true if +// the server cleanly exits. auto Run(FILE* input_stream, llvm::raw_ostream& output_stream, - llvm::raw_ostream& error_stream) -> ErrorOr; + llvm::raw_ostream& error_stream, DiagnosticConsumer& consumer) -> bool; } // namespace Carbon::LanguageServer diff --git a/toolchain/language_server/testdata/fail_empty_stdin.carbon b/toolchain/language_server/testdata/fail_empty_stdin.carbon index fed9f4a557ac..e538d5062f45 100644 --- a/toolchain/language_server/testdata/fail_empty_stdin.carbon +++ b/toolchain/language_server/testdata/fail_empty_stdin.carbon @@ -11,6 +11,6 @@ // --- STDIN // --- AUTOUPDATE-SPLIT -// CHECK:STDERR: error: Input/output error +// CHECK:STDERR: error: Input/output error [LanguageServerTransportError] // CHECK:STDERR: // CHECK:STDOUT: diff --git a/toolchain/language_server/testdata/fail_no_stdin.carbon b/toolchain/language_server/testdata/fail_no_stdin.carbon new file mode 100644 index 000000000000..71bc97b5f476 --- /dev/null +++ b/toolchain/language_server/testdata/fail_no_stdin.carbon @@ -0,0 +1,15 @@ +// 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 +// +// AUTOUPDATE +// TIP: To test this file alone, run: +// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/language_server/testdata/fail_no_stdin.carbon +// TIP: To dump output, run: +// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/language_server/testdata/fail_no_stdin.carbon + +// --- AUTOUPDATE-SPLIT + +// CHECK:STDERR: error: language-server requires input_stream [LanguageServerMissingInputStream] +// CHECK:STDERR: +// CHECK:STDOUT: diff --git a/toolchain/lex/tokenized_buffer_benchmark.cpp b/toolchain/lex/tokenized_buffer_benchmark.cpp index cffe8e6c394e..3ed9b1f91f1a 100644 --- a/toolchain/lex/tokenized_buffer_benchmark.cpp +++ b/toolchain/lex/tokenized_buffer_benchmark.cpp @@ -220,8 +220,7 @@ class LexerBenchHelper { auto DiagnoseErrors() -> std::string { RawStringOstream result; - StreamDiagnosticConsumer consumer(result, - /*include_diagnostic_kind=*/false); + StreamDiagnosticConsumer consumer(&result); auto buffer = Lex::Lex(value_stores_, source_, consumer); consumer.Flush(); CARBON_CHECK(buffer.has_errors(), diff --git a/toolchain/lower/testdata/debug/nodebug.carbon b/toolchain/lower/testdata/debug/nodebug.carbon index 5450be65aecd..7e9cb7f67894 100644 --- a/toolchain/lower/testdata/debug/nodebug.carbon +++ b/toolchain/lower/testdata/debug/nodebug.carbon @@ -2,7 +2,7 @@ // Exceptions. See /LICENSE for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // -// ARGS: compile --no-debug-info --phase=lower --dump-llvm-ir --output=- --exclude-dump-file-prefix=%{core_package_dir} %s +// ARGS: --include-diagnostic-kind compile --no-debug-info --phase=lower --dump-llvm-ir --output=- --exclude-dump-file-prefix=%{core_package_dir} %s // // AUTOUPDATE // TIP: To test this file alone, run: diff --git a/toolchain/source/BUILD b/toolchain/source/BUILD index e96d7fc0c4a4..db6c4cd558a6 100644 --- a/toolchain/source/BUILD +++ b/toolchain/source/BUILD @@ -13,6 +13,7 @@ cc_library( deps = [ "//common:error", "//toolchain/diagnostics:diagnostic_emitter", + "//toolchain/diagnostics:file_diagnostics", "//toolchain/diagnostics:format_providers", "@llvm-project//llvm:Support", ], diff --git a/toolchain/source/source_buffer.cpp b/toolchain/source/source_buffer.cpp index b93af3a7dfb6..59de7378f4e5 100644 --- a/toolchain/source/source_buffer.cpp +++ b/toolchain/source/source_buffer.cpp @@ -7,18 +7,10 @@ #include #include "llvm/Support/ErrorOr.h" +#include "toolchain/diagnostics/file_diagnostics.h" namespace Carbon { -namespace { -struct FilenameConverter : DiagnosticConverter { - auto ConvertLoc(llvm::StringRef filename, ContextFnT /*context_fn*/) const - -> ConvertedDiagnosticLoc override { - return {.loc = {.filename = filename}, .last_byte_offset = -1}; - } -}; -} // namespace - auto SourceBuffer::MakeFromStdin(DiagnosticConsumer& consumer) -> std::optional { return MakeFromMemoryBuffer(llvm::MemoryBuffer::getSTDIN(), "", @@ -29,8 +21,7 @@ auto SourceBuffer::MakeFromFile(llvm::vfs::FileSystem& fs, llvm::StringRef filename, DiagnosticConsumer& consumer) -> std::optional { - FilenameConverter converter; - DiagnosticEmitter emitter(converter, consumer); + FileDiagnosticEmitter emitter(&consumer); llvm::ErrorOr> file = fs.openFileForRead(filename); @@ -64,8 +55,7 @@ auto SourceBuffer::MakeFromMemoryBuffer( llvm::ErrorOr> buffer, llvm::StringRef filename, bool is_regular_file, DiagnosticConsumer& consumer) -> std::optional { - FilenameConverter converter; - DiagnosticEmitter emitter(converter, consumer); + FileDiagnosticEmitter emitter(&consumer); if (buffer.getError()) { CARBON_DIAGNOSTIC(ErrorReadingFile, Error, "error reading file: {0}", diff --git a/toolchain/testing/file_test.cpp b/toolchain/testing/file_test.cpp index 3c174d826185..3fb8a36524b8 100644 --- a/toolchain/testing/file_test.cpp +++ b/toolchain/testing/file_test.cpp @@ -104,10 +104,6 @@ auto ToolchainFileTest::Run( llvm::IntrusiveRefCntPtr& fs, FILE* input_stream, llvm::raw_pwrite_stream& output_stream, llvm::raw_pwrite_stream& error_stream) -> ErrorOr { - if (component_ == "language_server" && !input_stream) { - return Error("language_server tests must provide STDIN"); - } - CARBON_ASSIGN_OR_RETURN(auto prelude, installation_.ReadPreludeManifest()); if (!is_no_prelude()) { for (const auto& file : prelude) { @@ -147,14 +143,17 @@ auto ToolchainFileTest::Run( } auto ToolchainFileTest::GetDefaultArgs() -> llvm::SmallVector { + llvm::SmallVector args = {"--include-diagnostic-kind"}; + if (component_ == "format") { - return {"format", "%s"}; + args.insert(args.end(), {"format", "%s"}); + return args; } else if (component_ == "language_server") { - return {"language-server"}; + args.insert(args.end(), {"language-server"}); + return args; } - llvm::SmallVector args = {"compile", "--include-diagnostic-kind", - "--phase=" + component_.str()}; + args.insert(args.end(), {"compile", "--phase=" + component_.str()}); if (component_ == "lex") { args.insert(args.end(), {"--dump-tokens", "--omit-file-boundary-tokens"});