diff --git a/language_server/language_server.cpp b/language_server/language_server.cpp index 4444055f03e7..4a6e5830dc78 100644 --- a/language_server/language_server.cpp +++ b/language_server/language_server.cpp @@ -94,7 +94,7 @@ void LanguageServer::OnDocumentSymbol( vfs.addFile(file, /*mtime=*/0, llvm::MemoryBuffer::getMemBufferCopy(files_.at(file))); - auto buf = SourceBuffer::CreateFromFile(vfs, llvm::nulls(), file); + auto buf = SourceBuffer::CreateFromFile(vfs, file, NullDiagnosticConsumer()); auto lexed = Lex::TokenizedBuffer::Lex(*buf, NullDiagnosticConsumer()); auto parsed = Parse::Tree::Parse(lexed, NullDiagnosticConsumer(), nullptr); std::vector result; diff --git a/toolchain/diagnostics/diagnostic_emitter.h b/toolchain/diagnostics/diagnostic_emitter.h index a8606b19d32a..2e97b72eb27c 100644 --- a/toolchain/diagnostics/diagnostic_emitter.h +++ b/toolchain/diagnostics/diagnostic_emitter.h @@ -54,9 +54,9 @@ struct DiagnosticLocation { // A reference to the line of the error. llvm::StringRef line; // 1-based line number. - int32_t line_number; + int32_t line_number = -1; // 1-based column number. - int32_t column_number; + int32_t column_number = -1; }; // A message composing a diagnostic. This may be the main message, but can also @@ -215,8 +215,8 @@ class DiagnosticEmitter { Internal::NoTypeDeduction... args) -> DiagnosticBuilder& { CARBON_CHECK(diagnostic_base.Level == DiagnosticLevel::Note) << static_cast(diagnostic_base.Level); - diagnostic_.notes.push_back( - MakeMessage(location, diagnostic_base, std::move(args)...)); + diagnostic_.notes.push_back(MakeMessage( + emitter_, location, diagnostic_base, {llvm::Any(args)...})); return *this; } @@ -234,22 +234,23 @@ class DiagnosticEmitter { explicit DiagnosticBuilder( DiagnosticEmitter* emitter, LocationT location, const Internal::DiagnosticBase& diagnostic_base, - Internal::NoTypeDeduction... args) + llvm::SmallVector args) : emitter_(emitter), - diagnostic_({.level = diagnostic_base.Level, - .message = MakeMessage(location, diagnostic_base, - std::move(args)...)}) { + diagnostic_( + {.level = diagnostic_base.Level, + .message = MakeMessage(emitter, location, diagnostic_base, + std::move(args))}) { CARBON_CHECK(diagnostic_base.Level != DiagnosticLevel::Note); } template - auto MakeMessage(LocationT location, - const Internal::DiagnosticBase& diagnostic_base, - Internal::NoTypeDeduction... args) - -> DiagnosticMessage { + static auto MakeMessage( + DiagnosticEmitter* emitter, LocationT location, + const Internal::DiagnosticBase& diagnostic_base, + llvm::SmallVector args) -> DiagnosticMessage { return DiagnosticMessage( - diagnostic_base.Kind, emitter_->translator_->GetLocation(location), - diagnostic_base.Format, {std::move(args)...}, + diagnostic_base.Kind, emitter->translator_->GetLocation(location), + diagnostic_base.Format, std::move(args), [&diagnostic_base](const DiagnosticMessage& message) -> std::string { return diagnostic_base.FormatFn(message); }); @@ -275,7 +276,7 @@ class DiagnosticEmitter { auto Emit(LocationT location, const Internal::DiagnosticBase& diagnostic_base, Internal::NoTypeDeduction... args) -> void { - DiagnosticBuilder(this, location, diagnostic_base, std::move(args)...) + DiagnosticBuilder(this, location, diagnostic_base, {llvm::Any(args)...}) .Emit(); } @@ -290,7 +291,7 @@ class DiagnosticEmitter { const Internal::DiagnosticBase& diagnostic_base, Internal::NoTypeDeduction... args) -> DiagnosticBuilder { return DiagnosticBuilder(this, location, diagnostic_base, - std::move(args)...); + {llvm::Any(args)...}); } private: @@ -310,13 +311,19 @@ class StreamDiagnosticConsumer : public DiagnosticConsumer { } } auto Print(const DiagnosticMessage& message) -> void { - *stream_ << message.location.file_name << ":" - << message.location.line_number << ":" - << message.location.column_number << ": " - << message.format_fn(message) << "\n" - << message.location.line << "\n"; - stream_->indent(message.location.column_number - 1); - *stream_ << "^\n"; + *stream_ << message.location.file_name; + if (message.location.line_number > 0) { + *stream_ << ":" << message.location.line_number; + if (message.location.column_number > 0) { + *stream_ << ":" << message.location.column_number; + } + } + *stream_ << ": " << message.format_fn(message) << "\n"; + if (message.location.column_number > 0) { + *stream_ << message.location.line << "\n"; + stream_->indent(message.location.column_number - 1); + *stream_ << "^\n"; + } } private: diff --git a/toolchain/diagnostics/diagnostic_kind.def b/toolchain/diagnostics/diagnostic_kind.def index 7a0837faf771..ac7cbbaee3b5 100644 --- a/toolchain/diagnostics/diagnostic_kind.def +++ b/toolchain/diagnostics/diagnostic_kind.def @@ -16,6 +16,15 @@ #error "Must define the x-macro to use this file." #endif +// ============================================================================ +// SourceBuffer diagnostics +// ============================================================================ + +CARBON_DIAGNOSTIC_KIND(ErrorOpeningFile) +CARBON_DIAGNOSTIC_KIND(ErrorStattingFile) +CARBON_DIAGNOSTIC_KIND(FileTooLarge) +CARBON_DIAGNOSTIC_KIND(ErrorReadingFile) + // ============================================================================ // Lexer diagnostics // ============================================================================ diff --git a/toolchain/driver/driver.cpp b/toolchain/driver/driver.cpp index 6958d59448ea..f2f85d7925bf 100644 --- a/toolchain/driver/driver.cpp +++ b/toolchain/driver/driver.cpp @@ -401,8 +401,8 @@ class Driver::CompilationUnit { // 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_); + source_ = SourceBuffer::CreateFromFile(driver_->fs_, input_file_name_, + *consumer_); }); if (!source_) { return false; diff --git a/toolchain/driver/testdata/fail_missing_file.carbon b/toolchain/driver/testdata/fail_missing_file.carbon new file mode 100644 index 000000000000..45528c34d07c --- /dev/null +++ b/toolchain/driver/testdata/fail_missing_file.carbon @@ -0,0 +1,9 @@ +// 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 nonexistent.carbon +// +// AUTOUPDATE + +// CHECK:STDERR: nonexistent.carbon: Error opening file for read: No such file or directory diff --git a/toolchain/lex/BUILD b/toolchain/lex/BUILD index 8010d3e99f0a..d8db20ec2c61 100644 --- a/toolchain/lex/BUILD +++ b/toolchain/lex/BUILD @@ -241,6 +241,7 @@ cc_binary( ":token_kind", ":tokenized_buffer", "//common:check", + "//toolchain/diagnostics:diagnostic_emitter", "//toolchain/diagnostics:null_diagnostics", "@com_github_google_benchmark//:benchmark_main", "@com_google_absl//absl/random", diff --git a/toolchain/lex/tokenized_buffer_benchmark.cpp b/toolchain/lex/tokenized_buffer_benchmark.cpp index 79f546ed8e4f..74033c0a42f3 100644 --- a/toolchain/lex/tokenized_buffer_benchmark.cpp +++ b/toolchain/lex/tokenized_buffer_benchmark.cpp @@ -10,6 +10,7 @@ #include "common/check.h" #include "llvm/ADT/Sequence.h" #include "llvm/ADT/StringExtras.h" +#include "toolchain/diagnostics/diagnostic_emitter.h" #include "toolchain/diagnostics/null_diagnostics.h" #include "toolchain/lex/token_kind.h" #include "toolchain/lex/tokenized_buffer.h" @@ -297,8 +298,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_, llvm::errs(), filename_)); + return std::move(*SourceBuffer::CreateFromFile( + fs_, filename_, ConsoleDiagnosticConsumer())); } llvm::vfs::InMemoryFileSystem fs_; diff --git a/toolchain/lex/tokenized_buffer_fuzzer.cpp b/toolchain/lex/tokenized_buffer_fuzzer.cpp index e65dc7a6ac02..ffb220a41871 100644 --- a/toolchain/lex/tokenized_buffer_fuzzer.cpp +++ b/toolchain/lex/tokenized_buffer_fuzzer.cpp @@ -30,7 +30,8 @@ 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, llvm::nulls(), TestFileName); + auto source = + SourceBuffer::CreateFromFile(fs, TestFileName, NullDiagnosticConsumer()); auto buffer = Lex::TokenizedBuffer::Lex(*source, NullDiagnosticConsumer()); if (buffer.has_errors()) { diff --git a/toolchain/lex/tokenized_buffer_test.cpp b/toolchain/lex/tokenized_buffer_test.cpp index b98677e4fad1..6300e1d5daa9 100644 --- a/toolchain/lex/tokenized_buffer_test.cpp +++ b/toolchain/lex/tokenized_buffer_test.cpp @@ -34,8 +34,8 @@ class LexerTest : public ::testing::Test { std::string filename = llvm::formatv("test{0}.carbon", ++file_index_); CARBON_CHECK(fs_.addFile(filename, /*ModificationTime=*/0, llvm::MemoryBuffer::getMemBuffer(text))); - source_storage_.push_front( - std::move(*SourceBuffer::CreateFromFile(fs_, llvm::errs(), filename))); + source_storage_.push_front(std::move(*SourceBuffer::CreateFromFile( + fs_, filename, ConsoleDiagnosticConsumer()))); return source_storage_.front(); } diff --git a/toolchain/parse/parse_fuzzer.cpp b/toolchain/parse/parse_fuzzer.cpp index 63746a254ffb..108330992f3b 100644 --- a/toolchain/parse/parse_fuzzer.cpp +++ b/toolchain/parse/parse_fuzzer.cpp @@ -27,7 +27,8 @@ 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, llvm::nulls(), TestFileName); + auto source = + SourceBuffer::CreateFromFile(fs, TestFileName, NullDiagnosticConsumer()); // Lex the input. auto tokens = Lex::TokenizedBuffer::Lex(*source, NullDiagnosticConsumer()); diff --git a/toolchain/parse/tree_test.cpp b/toolchain/parse/tree_test.cpp index 1f7d60d25593..fe11a5548b6a 100644 --- a/toolchain/parse/tree_test.cpp +++ b/toolchain/parse/tree_test.cpp @@ -26,8 +26,8 @@ class TreeTest : public ::testing::Test { auto GetSourceBuffer(llvm::StringRef t) -> SourceBuffer& { CARBON_CHECK(fs.addFile("test.carbon", /*ModificationTime=*/0, llvm::MemoryBuffer::getMemBuffer(t))); - source_storage.push_front(std::move( - *SourceBuffer::CreateFromFile(fs, llvm::errs(), "test.carbon"))); + source_storage.push_front( + std::move(*SourceBuffer::CreateFromFile(fs, "test.carbon", consumer))); return source_storage.front(); } diff --git a/toolchain/source/BUILD b/toolchain/source/BUILD index 2ba2adb6292d..6f413799055e 100644 --- a/toolchain/source/BUILD +++ b/toolchain/source/BUILD @@ -12,6 +12,7 @@ cc_library( hdrs = ["source_buffer.h"], deps = [ "//common:error", + "//toolchain/diagnostics:diagnostic_emitter", "@llvm-project//llvm:Support", ], ) @@ -24,6 +25,7 @@ cc_test( ":source_buffer", "//common:check", "//testing/base:gtest_main", + "//toolchain/diagnostics:diagnostic_emitter", "@com_google_googletest//:gtest", "@llvm-project//llvm:Support", ], diff --git a/toolchain/source/source_buffer.cpp b/toolchain/source/source_buffer.cpp index b38e0d06b6c0..1204a3083a7b 100644 --- a/toolchain/source/source_buffer.cpp +++ b/toolchain/source/source_buffer.cpp @@ -10,36 +10,52 @@ namespace Carbon { +namespace { +struct FilenameTranslator : DiagnosticLocationTranslator { + auto GetLocation(llvm::StringRef filename) -> DiagnosticLocation override { + return {.file_name = filename}; + } +}; +} // namespace + auto SourceBuffer::CreateFromFile(llvm::vfs::FileSystem& fs, - llvm::raw_ostream& error_stream, - llvm::StringRef filename) + llvm::StringRef filename, + DiagnosticConsumer& consumer) -> std::optional { + FilenameTranslator translator; + DiagnosticEmitter emitter(translator, consumer); + llvm::ErrorOr> file = fs.openFileForRead(filename); if (file.getError()) { - error_stream << "Error opening `" << filename - << "`: " << file.getError().message(); + CARBON_DIAGNOSTIC(ErrorOpeningFile, Error, + "Error opening file for read: {0}", std::string); + emitter.Emit(filename, ErrorOpeningFile, file.getError().message()); return std::nullopt; } llvm::ErrorOr status = (*file)->status(); if (status.getError()) { - error_stream << "Error getting status for `" << filename - << "`: " << file.getError().message(); + CARBON_DIAGNOSTIC(ErrorStattingFile, Error, "Error statting file: {0}", + std::string); + emitter.Emit(filename, ErrorStattingFile, file.getError().message()); return std::nullopt; } - auto size = status->getSize(); + int64_t size = status->getSize(); if (size >= std::numeric_limits::max()) { - error_stream << "Cannot load `" << filename - << "`: file is over the 2GiB input limit."; + CARBON_DIAGNOSTIC(FileTooLarge, Error, + "File is over the 2GiB input limit; size is {0} bytes.", + int64_t); + emitter.Emit(filename, FileTooLarge, size); return std::nullopt; } llvm::ErrorOr> buffer = (*file)->getBuffer(filename, size, /*RequiresNullTerminator=*/false); if (buffer.getError()) { - error_stream << "Error reading `" << filename - << "`: " << file.getError().message(); + CARBON_DIAGNOSTIC(ErrorReadingFile, Error, "Error reading file: {0}", + std::string); + emitter.Emit(filename, ErrorReadingFile, file.getError().message()); return std::nullopt; } diff --git a/toolchain/source/source_buffer.h b/toolchain/source/source_buffer.h index 52d97b3d6725..9eb715536481 100644 --- a/toolchain/source/source_buffer.h +++ b/toolchain/source/source_buffer.h @@ -11,6 +11,7 @@ #include "llvm/ADT/StringRef.h" #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/VirtualFileSystem.h" +#include "toolchain/diagnostics/diagnostic_emitter.h" namespace Carbon { @@ -35,10 +36,9 @@ 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::raw_ostream& error_stream, - llvm::StringRef filename) + llvm::StringRef filename, + DiagnosticConsumer& consumer) -> std::optional; // Use one of the factory functions above to create a source buffer. diff --git a/toolchain/source/source_buffer_test.cpp b/toolchain/source/source_buffer_test.cpp index 927243478e99..ceea62a5e30a 100644 --- a/toolchain/source/source_buffer_test.cpp +++ b/toolchain/source/source_buffer_test.cpp @@ -8,6 +8,7 @@ #include "common/check.h" #include "llvm/Support/VirtualFileSystem.h" +#include "toolchain/diagnostics/diagnostic_emitter.h" namespace Carbon::Testing { namespace { @@ -16,7 +17,8 @@ static constexpr llvm::StringLiteral TestFileName = "test.carbon"; TEST(SourceBufferTest, MissingFile) { llvm::vfs::InMemoryFileSystem fs; - auto buffer = SourceBuffer::CreateFromFile(fs, llvm::errs(), TestFileName); + auto buffer = SourceBuffer::CreateFromFile(fs, TestFileName, + ConsoleDiagnosticConsumer()); EXPECT_FALSE(buffer); } @@ -25,7 +27,8 @@ TEST(SourceBufferTest, SimpleFile) { CARBON_CHECK(fs.addFile(TestFileName, /*ModificationTime=*/0, llvm::MemoryBuffer::getMemBuffer("Hello World"))); - auto buffer = SourceBuffer::CreateFromFile(fs, llvm::errs(), TestFileName); + auto buffer = SourceBuffer::CreateFromFile(fs, TestFileName, + ConsoleDiagnosticConsumer()); ASSERT_TRUE(buffer); EXPECT_EQ(TestFileName, buffer->filename()); @@ -41,7 +44,8 @@ TEST(SourceBufferTest, NoNull) { /*BufferName=*/"", /*RequiresNullTerminator=*/false))); - auto buffer = SourceBuffer::CreateFromFile(fs, llvm::errs(), TestFileName); + auto buffer = SourceBuffer::CreateFromFile(fs, TestFileName, + ConsoleDiagnosticConsumer()); ASSERT_TRUE(buffer); EXPECT_EQ(TestFileName, buffer->filename()); @@ -53,7 +57,8 @@ TEST(SourceBufferTest, EmptyFile) { CARBON_CHECK(fs.addFile(TestFileName, /*ModificationTime=*/0, llvm::MemoryBuffer::getMemBuffer(""))); - auto buffer = SourceBuffer::CreateFromFile(fs, llvm::errs(), TestFileName); + auto buffer = SourceBuffer::CreateFromFile(fs, TestFileName, + ConsoleDiagnosticConsumer()); ASSERT_TRUE(buffer); EXPECT_EQ(TestFileName, buffer->filename());