From e79d3be5bde208718d28027fd0a132fa03d83bea Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Thu, 6 Feb 2025 12:27:57 -0800 Subject: [PATCH] Combine DiagnosticConverter into DiagnosticEmitter (#4878) At present, we typically define a DiagnosticConverter, then store an instance of it and a DiagnosticEmitter that wraps it. This is relatively minor in general, but I've been trying to create more self-contained DiagnosticEmitter classes (which hold their own DiagnosticConverter, similar to NullDiagnosticEmitter), and there it just gets in the way. Since we don't reuse DiagnosticConverter instances, this combines the definition into DiagnosticEmitter. Mainly this means we don't have a separate object in play, and less to carry around. The most impact is probably to SemIRDiagnosticConverter, which was also the most complex. Now `SemIRLocDiagnosticEmitter`, this gets some different construction flow. Note in the PR I've split the file rename to its own commit, to try to help delta views. However, the most substantial parts of the refactoring are split into #4876, which this depends upon. --- toolchain/check/BUILD | 8 +- toolchain/check/check.cpp | 9 ++- toolchain/check/check.h | 7 +- toolchain/check/check_unit.cpp | 25 +++--- toolchain/check/check_unit.h | 54 +++++++------ toolchain/check/context.cpp | 4 +- toolchain/check/context.h | 6 +- toolchain/check/diagnostic_helpers.h | 2 +- ....cpp => sem_ir_loc_diagnostic_emitter.cpp} | 18 ++--- ...rter.h => sem_ir_loc_diagnostic_emitter.h} | 37 +++++---- toolchain/diagnostics/diagnostic_emitter.h | 75 +++++++---------- .../diagnostics/diagnostic_emitter_test.cpp | 11 ++- toolchain/diagnostics/file_diagnostics.h | 17 ++-- toolchain/diagnostics/null_diagnostics.h | 31 +++---- .../sorting_diagnostic_consumer_test.cpp | 9 ++- toolchain/driver/compile_subcommand.cpp | 80 +++++++------------ toolchain/lex/BUILD | 1 + toolchain/lex/lex.cpp | 15 ++-- toolchain/lex/numeric_literal_benchmark.cpp | 4 +- toolchain/lex/numeric_literal_test.cpp | 3 +- toolchain/lex/string_literal_test.cpp | 3 +- toolchain/lex/test_helpers.h | 8 +- toolchain/lex/tokenized_buffer.cpp | 12 --- toolchain/lex/tokenized_buffer.h | 59 ++++++-------- toolchain/lower/BUILD | 4 +- toolchain/lower/file_context.cpp | 11 +-- toolchain/lower/file_context.h | 6 +- toolchain/lower/lower.cpp | 4 +- toolchain/lower/lower.h | 2 +- toolchain/parse/context.cpp | 4 +- toolchain/parse/context.h | 51 ++++++------ 31 files changed, 264 insertions(+), 316 deletions(-) rename toolchain/check/{sem_ir_diagnostic_converter.cpp => sem_ir_loc_diagnostic_emitter.cpp} (87%) rename toolchain/check/{sem_ir_diagnostic_converter.h => sem_ir_loc_diagnostic_emitter.h} (77%) diff --git a/toolchain/check/BUILD b/toolchain/check/BUILD index 2043007a7f86..cf10e10b3abe 100644 --- a/toolchain/check/BUILD +++ b/toolchain/check/BUILD @@ -139,7 +139,7 @@ cc_library( ":dump", ":impl", ":pointer_dereference", - ":sem_ir_diagnostic_converter", + ":sem_ir_loc_diagnostic_emitter", "//common:check", "//common:error", "//common:map", @@ -263,9 +263,9 @@ cc_library( ) cc_library( - name = "sem_ir_diagnostic_converter", - srcs = ["sem_ir_diagnostic_converter.cpp"], - hdrs = ["sem_ir_diagnostic_converter.h"], + name = "sem_ir_loc_diagnostic_emitter", + srcs = ["sem_ir_loc_diagnostic_emitter.cpp"], + hdrs = ["sem_ir_loc_diagnostic_emitter.h"], deps = [ ":context", "//common:raw_string_ostream", diff --git a/toolchain/check/check.cpp b/toolchain/check/check.cpp index 05ea9500396d..4501c3821b3e 100644 --- a/toolchain/check/check.cpp +++ b/toolchain/check/check.cpp @@ -9,7 +9,7 @@ #include "toolchain/check/check_unit.h" #include "toolchain/check/context.h" #include "toolchain/check/diagnostic_helpers.h" -#include "toolchain/check/sem_ir_diagnostic_converter.h" +#include "toolchain/check/sem_ir_loc_diagnostic_emitter.h" #include "toolchain/diagnostics/diagnostic.h" #include "toolchain/diagnostics/format_providers.h" #include "toolchain/lex/token_kind.h" @@ -320,9 +320,12 @@ auto CheckParseTrees(llvm::MutableArrayRef units, bool prelude_import, // UnitAndImports is big due to its SmallVectors, so we default to 0 on the // stack. llvm::SmallVector unit_infos; + llvm::SmallVector tree_and_subtrees_getters; unit_infos.reserve(units.size()); + tree_and_subtrees_getters.reserve(units.size()); for (auto [i, unit] : llvm::enumerate(units)) { unit_infos.emplace_back(SemIR::CheckIRId(i), unit); + tree_and_subtrees_getters.push_back(unit.tree_and_subtrees_getter); } Map api_map = @@ -372,7 +375,7 @@ auto CheckParseTrees(llvm::MutableArrayRef units, bool prelude_import, for (int check_index = 0; check_index < static_cast(ready_to_check.size()); ++check_index) { auto* unit_info = ready_to_check[check_index]; - CheckUnit(unit_info, units.size(), fs, vlog_stream).Run(); + CheckUnit(unit_info, tree_and_subtrees_getters, fs, vlog_stream).Run(); for (auto* incoming_import : unit_info->incoming_imports) { --incoming_import->imports_remaining; if (incoming_import->imports_remaining == 0) { @@ -419,7 +422,7 @@ auto CheckParseTrees(llvm::MutableArrayRef units, bool prelude_import, // incomplete imports. for (auto& unit_info : unit_infos) { if (unit_info.imports_remaining > 0) { - CheckUnit(&unit_info, units.size(), fs, vlog_stream).Run(); + CheckUnit(&unit_info, tree_and_subtrees_getters, fs, vlog_stream).Run(); } } } diff --git a/toolchain/check/check.h b/toolchain/check/check.h index 1440fe7bbad9..f876c681ae45 100644 --- a/toolchain/check/check.h +++ b/toolchain/check/check.h @@ -8,7 +8,7 @@ #include "common/ostream.h" #include "toolchain/base/shared_value_stores.h" #include "toolchain/base/timings.h" -#include "toolchain/check/sem_ir_diagnostic_converter.h" +#include "toolchain/check/sem_ir_loc_diagnostic_emitter.h" #include "toolchain/diagnostics/diagnostic_emitter.h" #include "toolchain/parse/tree_and_subtrees.h" #include "toolchain/sem_ir/file.h" @@ -23,13 +23,10 @@ struct Unit { Timings* timings; // Returns a lazily constructed TreeAndSubtrees. - Parse::GetTreeAndSubtreesFn get_parse_tree_and_subtrees; + Parse::GetTreeAndSubtreesFn tree_and_subtrees_getter; // The unit's SemIR, provided as empty and filled in by CheckParseTrees. SemIR::File* sem_ir; - - // Diagnostic converters. - SemIRDiagnosticConverter* sem_ir_converter; }; // Checks a group of parse trees. This will use imports to decide the order of diff --git a/toolchain/check/check_unit.cpp b/toolchain/check/check_unit.cpp index bc05d5d60a7c..8db88ac81bff 100644 --- a/toolchain/check/check_unit.cpp +++ b/toolchain/check/check_unit.cpp @@ -34,19 +34,21 @@ static auto GetImportedIRCount(UnitAndImports* unit_and_imports) -> int { return count; } -CheckUnit::CheckUnit(UnitAndImports* unit_and_imports, int total_ir_count, - llvm::IntrusiveRefCntPtr fs, - llvm::raw_ostream* vlog_stream) +CheckUnit::CheckUnit( + UnitAndImports* unit_and_imports, + llvm::ArrayRef tree_and_subtrees_getters, + llvm::IntrusiveRefCntPtr fs, + llvm::raw_ostream* vlog_stream) : unit_and_imports_(unit_and_imports), - total_ir_count_(total_ir_count), + total_ir_count_(tree_and_subtrees_getters.size()), fs_(std::move(fs)), vlog_stream_(vlog_stream), - emitter_(*unit_and_imports_->unit->sem_ir_converter, - unit_and_imports_->err_tracker), - context_(&emitter_, unit_and_imports_->unit->get_parse_tree_and_subtrees, + emitter_(&unit_and_imports_->err_tracker, tree_and_subtrees_getters, + unit_and_imports_->unit->sem_ir), + context_(&emitter_, unit_and_imports_->unit->tree_and_subtrees_getter, unit_and_imports_->unit->sem_ir, - GetImportedIRCount(unit_and_imports), total_ir_count, - vlog_stream) {} + GetImportedIRCount(unit_and_imports), + tree_and_subtrees_getters.size(), vlog_stream) {} auto CheckUnit::Run() -> void { Timings::ScopedTiming timing(unit_and_imports_->unit->timings, "check"); @@ -363,7 +365,7 @@ auto CheckUnit::ProcessNodeIds() -> bool { // On crash, report which token we were handling. PrettyStackTraceFunction node_dumper([&](llvm::raw_ostream& output) { - const auto& tree = unit_and_imports_->unit->get_parse_tree_and_subtrees(); + const auto& tree = unit_and_imports_->unit->tree_and_subtrees_getter(); auto converted = tree.NodeToDiagnosticLoc(node_id, /*token_only=*/false); converted.loc.FormatLocation(output); output << "checking " << context_.parse_tree().node_kind(node_id) << "\n"; @@ -374,8 +376,7 @@ auto CheckUnit::ProcessNodeIds() -> bool { while (auto maybe_node_id = traversal.Next()) { node_id = *maybe_node_id; - unit_and_imports_->unit->sem_ir_converter->AdvanceToken( - context_.parse_tree().node_token(node_id)); + emitter_.AdvanceToken(context_.parse_tree().node_token(node_id)); if (context_.parse_tree().node_has_error(node_id)) { context_.TODO(node_id, "handle invalid parse trees in `check`"); diff --git a/toolchain/check/check_unit.h b/toolchain/check/check_unit.h index dcca71f1f00d..52d456cd678e 100644 --- a/toolchain/check/check_unit.h +++ b/toolchain/check/check_unit.h @@ -9,6 +9,7 @@ #include "llvm/ADT/SmallVector.h" #include "toolchain/check/check.h" #include "toolchain/check/context.h" +#include "toolchain/check/sem_ir_loc_diagnostic_emitter.h" #include "toolchain/sem_ir/ids.h" namespace Carbon::Check { @@ -43,33 +44,33 @@ struct PackageImports { llvm::SmallVector imports; }; -// Converts a `NodeId` to a diagnostic location for `UnitAndImports`. -class UnitAndImportsDiagnosticConverter - : public DiagnosticConverter { - public: - explicit UnitAndImportsDiagnosticConverter( - Parse::GetTreeAndSubtreesFn get_parse_tree_and_subtrees) - : get_parse_tree_and_subtrees_(get_parse_tree_and_subtrees) {} - - auto ConvertLoc(Parse::NodeId node_id, ContextFnT /*context_fn*/) const - -> ConvertedDiagnosticLoc override { - return get_parse_tree_and_subtrees_().NodeToDiagnosticLoc( - node_id, /*token_only=*/false); - } - - private: - Parse::GetTreeAndSubtreesFn get_parse_tree_and_subtrees_; -}; - // Contains information accumulated while checking a `Unit` (primarily import // information), in addition to the `Unit` itself. struct UnitAndImports { + // Converts a `NodeId` to a diagnostic location for `UnitAndImports`. + class Emitter : public DiagnosticEmitter { + public: + explicit Emitter(DiagnosticConsumer* consumer, + Parse::GetTreeAndSubtreesFn tree_and_subtrees_getter) + : DiagnosticEmitter(consumer), + tree_and_subtrees_getter_(tree_and_subtrees_getter) {} + + protected: + auto ConvertLoc(Parse::NodeId node_id, ContextFnT /*context_fn*/) const + -> ConvertedDiagnosticLoc override { + return tree_and_subtrees_getter_().NodeToDiagnosticLoc( + node_id, /*token_only=*/false); + } + + private: + Parse::GetTreeAndSubtreesFn tree_and_subtrees_getter_; + }; + explicit UnitAndImports(SemIR::CheckIRId check_ir_id, Unit& unit) : check_ir_id(check_ir_id), unit(&unit), err_tracker(*unit.consumer), - converter(unit.get_parse_tree_and_subtrees), - emitter(converter, err_tracker) {} + emitter(&err_tracker, unit.tree_and_subtrees_getter) {} auto parse_tree() -> const Parse::Tree& { return unit->sem_ir->parse_tree(); } auto source() -> const SourceBuffer& { @@ -81,8 +82,7 @@ struct UnitAndImports { // Emitter information. ErrorTrackingDiagnosticConsumer err_tracker; - UnitAndImportsDiagnosticConverter converter; - DiagnosticEmitter emitter; + Emitter emitter; // List of the outgoing imports. If a package includes unavailable library // imports, it has an entry with has_load_error set. Invalid imports (for @@ -119,9 +119,11 @@ struct UnitAndImports { // logic in check.cpp. class CheckUnit { public: - explicit CheckUnit(UnitAndImports* unit_and_imports, int total_ir_count, - llvm::IntrusiveRefCntPtr fs, - llvm::raw_ostream* vlog_stream); + explicit CheckUnit( + UnitAndImports* unit_and_imports, + llvm::ArrayRef tree_and_subtrees_getters, + llvm::IntrusiveRefCntPtr fs, + llvm::raw_ostream* vlog_stream); // Produces and checks the IR for the provided unit. auto Run() -> void; @@ -171,7 +173,7 @@ class CheckUnit { llvm::IntrusiveRefCntPtr fs_; llvm::raw_ostream* vlog_stream_; - Context::DiagnosticEmitter emitter_; + SemIRLocDiagnosticEmitter emitter_; Context context_; }; diff --git a/toolchain/check/context.cpp b/toolchain/check/context.cpp index 5eba636ee82d..bd1eb879f6c5 100644 --- a/toolchain/check/context.cpp +++ b/toolchain/check/context.cpp @@ -39,11 +39,11 @@ namespace Carbon::Check { Context::Context(DiagnosticEmitter* emitter, - Parse::GetTreeAndSubtreesFn get_parse_tree_and_subtrees, + Parse::GetTreeAndSubtreesFn tree_and_subtrees_getter, SemIR::File* sem_ir, int imported_ir_count, int total_ir_count, llvm::raw_ostream* vlog_stream) : emitter_(emitter), - get_parse_tree_and_subtrees_(get_parse_tree_and_subtrees), + tree_and_subtrees_getter_(tree_and_subtrees_getter), sem_ir_(sem_ir), vlog_stream_(vlog_stream), node_stack_(sem_ir->parse_tree(), vlog_stream), diff --git a/toolchain/check/context.h b/toolchain/check/context.h index db095387b1d9..01d7a0cbb40c 100644 --- a/toolchain/check/context.h +++ b/toolchain/check/context.h @@ -73,7 +73,7 @@ class Context { // Stores references for work. explicit Context(DiagnosticEmitter* emitter, - Parse::GetTreeAndSubtreesFn get_parse_tree_and_subtrees, + Parse::GetTreeAndSubtreesFn tree_and_subtrees_getter, SemIR::File* sem_ir, int imported_ir_count, int total_ir_count, llvm::raw_ostream* vlog_stream); @@ -540,7 +540,7 @@ class Context { auto emitter() -> DiagnosticEmitter& { return *emitter_; } auto parse_tree_and_subtrees() -> const Parse::TreeAndSubtrees& { - return get_parse_tree_and_subtrees_(); + return tree_and_subtrees_getter_(); } auto sem_ir() -> SemIR::File& { return *sem_ir_; } @@ -747,7 +747,7 @@ class Context { DiagnosticEmitter* emitter_; // Returns a lazily constructed TreeAndSubtrees. - Parse::GetTreeAndSubtreesFn get_parse_tree_and_subtrees_; + Parse::GetTreeAndSubtreesFn tree_and_subtrees_getter_; // The SemIR::File being added to. SemIR::File* sem_ir_; diff --git a/toolchain/check/diagnostic_helpers.h b/toolchain/check/diagnostic_helpers.h index 61aa23d803a5..0c1973d0d52f 100644 --- a/toolchain/check/diagnostic_helpers.h +++ b/toolchain/check/diagnostic_helpers.h @@ -33,7 +33,7 @@ class SemIRLoc { private: // Only allow member access for diagnostics. - friend class SemIRDiagnosticConverter; + friend class SemIRLocDiagnosticEmitter; union { SemIR::InstId inst_id_; diff --git a/toolchain/check/sem_ir_diagnostic_converter.cpp b/toolchain/check/sem_ir_loc_diagnostic_emitter.cpp similarity index 87% rename from toolchain/check/sem_ir_diagnostic_converter.cpp rename to toolchain/check/sem_ir_loc_diagnostic_emitter.cpp index 14a336bd6dd9..dc24be71112b 100644 --- a/toolchain/check/sem_ir_diagnostic_converter.cpp +++ b/toolchain/check/sem_ir_loc_diagnostic_emitter.cpp @@ -2,7 +2,7 @@ // Exceptions. See /LICENSE for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -#include "toolchain/check/sem_ir_diagnostic_converter.h" +#include "toolchain/check/sem_ir_loc_diagnostic_emitter.h" #include "common/raw_string_ostream.h" #include "toolchain/sem_ir/absolute_node_id.h" @@ -10,8 +10,8 @@ namespace Carbon::Check { -auto SemIRDiagnosticConverter::ConvertLoc(SemIRLoc loc, - ContextFnT context_fn) const +auto SemIRLocDiagnosticEmitter::ConvertLoc(SemIRLoc loc, + ContextFnT context_fn) const -> ConvertedDiagnosticLoc { auto converted = ConvertLocImpl(loc, context_fn); @@ -33,8 +33,8 @@ auto SemIRDiagnosticConverter::ConvertLoc(SemIRLoc loc, return converted; } -auto SemIRDiagnosticConverter::ConvertLocImpl(SemIRLoc loc, - ContextFnT context_fn) const +auto SemIRLocDiagnosticEmitter::ConvertLocImpl(SemIRLoc loc, + ContextFnT context_fn) const -> ConvertedDiagnosticLoc { llvm::SmallVector absolute_node_ids = loc.is_inst_id_ ? SemIR::GetAbsoluteNodeId(sem_ir_, loc.inst_id_) @@ -57,16 +57,16 @@ auto SemIRDiagnosticConverter::ConvertLocImpl(SemIRLoc loc, return ConvertLocInFile(final_node_id, loc.token_only_, context_fn); } -auto SemIRDiagnosticConverter::ConvertLocInFile( +auto SemIRLocDiagnosticEmitter::ConvertLocInFile( SemIR::AbsoluteNodeId absolute_node_id, bool token_only, ContextFnT /*context_fn*/) const -> ConvertedDiagnosticLoc { const auto& tree_and_subtrees = - imported_trees_and_subtrees_[absolute_node_id.check_ir_id.index](); + tree_and_subtrees_getters_[absolute_node_id.check_ir_id.index](); return tree_and_subtrees.NodeToDiagnosticLoc(absolute_node_id.node_id, token_only); } -auto SemIRDiagnosticConverter::ConvertArg(llvm::Any arg) const -> llvm::Any { +auto SemIRLocDiagnosticEmitter::ConvertArg(llvm::Any arg) const -> llvm::Any { if (auto* library_name_id = llvm::any_cast(&arg)) { std::string library_name; if (*library_name_id == SemIR::LibraryNameId::Default) { @@ -118,7 +118,7 @@ auto SemIRDiagnosticConverter::ConvertArg(llvm::Any arg) const -> llvm::Any { return llvm::APSInt(typed_int->value, !sem_ir_->types().IsSignedInt(typed_int->type)); } - return DiagnosticConverter::ConvertArg(arg); + return DiagnosticEmitter::ConvertArg(arg); } } // namespace Carbon::Check diff --git a/toolchain/check/sem_ir_diagnostic_converter.h b/toolchain/check/sem_ir_loc_diagnostic_emitter.h similarity index 77% rename from toolchain/check/sem_ir_diagnostic_converter.h rename to toolchain/check/sem_ir_loc_diagnostic_emitter.h index 1f540167cfe8..a09ecc4565ab 100644 --- a/toolchain/check/sem_ir_diagnostic_converter.h +++ b/toolchain/check/sem_ir_loc_diagnostic_emitter.h @@ -2,8 +2,8 @@ // Exceptions. See /LICENSE for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -#ifndef CARBON_TOOLCHAIN_CHECK_SEM_IR_DIAGNOSTIC_CONVERTER_H_ -#define CARBON_TOOLCHAIN_CHECK_SEM_IR_DIAGNOSTIC_CONVERTER_H_ +#ifndef CARBON_TOOLCHAIN_CHECK_SEM_IR_LOC_DIAGNOSTIC_EMITTER_H_ +#define CARBON_TOOLCHAIN_CHECK_SEM_IR_LOC_DIAGNOSTIC_EMITTER_H_ #include "llvm/ADT/ArrayRef.h" #include "toolchain/check/diagnostic_helpers.h" @@ -17,14 +17,26 @@ namespace Carbon::Check { // Handles the transformation of a SemIRLoc to a DiagnosticLoc. -class SemIRDiagnosticConverter : public DiagnosticConverter { +class SemIRLocDiagnosticEmitter : public DiagnosticEmitter { public: - explicit SemIRDiagnosticConverter( - llvm::ArrayRef imported_trees_and_subtrees, + explicit SemIRLocDiagnosticEmitter( + DiagnosticConsumer* consumer, + llvm::ArrayRef tree_and_subtrees_getters, const SemIR::File* sem_ir) - : imported_trees_and_subtrees_(imported_trees_and_subtrees), + : DiagnosticEmitter(consumer), + tree_and_subtrees_getters_(tree_and_subtrees_getters), sem_ir_(sem_ir) {} + // If a byte offset is past the current last byte offset, advances forward. + // Earlier offsets are ignored. + auto AdvanceToken(Lex::TokenIndex token) -> void { + last_token_ = std::max(last_token_, token); + } + + protected: + // Implements argument conversions for supported check-phase arguments. + auto ConvertArg(llvm::Any arg) const -> llvm::Any override; + // Implements `DiagnosticConverter::ConvertLoc`. Adds context for any imports // used in the current SemIR to get to the underlying code. // @@ -34,15 +46,6 @@ class SemIRDiagnosticConverter : public DiagnosticConverter { auto ConvertLoc(SemIRLoc loc, ContextFnT context_fn) const -> ConvertedDiagnosticLoc override; - // Implements argument conversions for supported check-phase arguments. - auto ConvertArg(llvm::Any arg) const -> llvm::Any override; - - // If a byte offset is past the current last byte offset, advances forward. - // Earlier offsets are ignored. - auto AdvanceToken(Lex::TokenIndex token) -> void { - last_token_ = std::max(last_token_, token); - } - private: // Implements `ConvertLoc`, but without `last_token_` applied. auto ConvertLocImpl(SemIRLoc loc, ContextFnT context_fn) const @@ -54,7 +57,7 @@ class SemIRDiagnosticConverter : public DiagnosticConverter { ContextFnT context_fn) const -> ConvertedDiagnosticLoc; // Converters for each SemIR. - llvm::ArrayRef imported_trees_and_subtrees_; + llvm::ArrayRef tree_and_subtrees_getters_; // The current SemIR being processed. const SemIR::File* sem_ir_; @@ -65,4 +68,4 @@ class SemIRDiagnosticConverter : public DiagnosticConverter { } // namespace Carbon::Check -#endif // CARBON_TOOLCHAIN_CHECK_SEM_IR_DIAGNOSTIC_CONVERTER_H_ +#endif // CARBON_TOOLCHAIN_CHECK_SEM_IR_LOC_DIAGNOSTIC_EMITTER_H_ diff --git a/toolchain/diagnostics/diagnostic_emitter.h b/toolchain/diagnostics/diagnostic_emitter.h index 15023dc94ee8..38080fa1dc5b 100644 --- a/toolchain/diagnostics/diagnostic_emitter.h +++ b/toolchain/diagnostics/diagnostic_emitter.h @@ -41,30 +41,6 @@ struct ConvertedDiagnosticLoc { int32_t last_byte_offset; }; -// An interface that can convert some representation of a location into a -// diagnostic location. -template -class DiagnosticConverter { - public: - // Callback type used to report context messages from ConvertLoc. - // Note that the first parameter type is DiagnosticLoc rather than - // LocT, because ConvertLoc must not recurse. - using ContextFnT = - llvm::function_ref&)>; - - virtual ~DiagnosticConverter() = default; - - // Converts a LocT to a DiagnosticLoc and its `last_byte_offset` (see - // `DiagnosticMessage`). ConvertLoc may invoke context_fn to provide context - // messages. - virtual auto ConvertLoc(LocT loc, ContextFnT context_fn) const - -> ConvertedDiagnosticLoc = 0; - - // Converts arg types as needed. Not all uses require conversion, so the - // default returns the argument unchanged. - virtual auto ConvertArg(llvm::Any arg) const -> llvm::Any { return arg; } -}; - // Used by types to indicate a diagnostic type conversion that results in the // provided StorageType. For example, to convert NameId to a std::string, we // write: @@ -155,15 +131,11 @@ class DiagnosticEmitter { Diagnostic diagnostic_; }; - // 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) {} + // `consumer` is required to outlive the diagnostic emitter. + explicit DiagnosticEmitter(DiagnosticConsumer* consumer) + : consumer_(consumer) {} - ~DiagnosticEmitter() = default; + virtual ~DiagnosticEmitter() = default; // Emits an error. // @@ -187,6 +159,23 @@ class DiagnosticEmitter { // be silently ignored. auto BuildSuppressed() -> DiagnosticBuilder { return DiagnosticBuilder(); } + protected: + // Callback type used to report context messages from ConvertLoc. + // Note that the first parameter type is DiagnosticLoc rather than + // LocT, because ConvertLoc must not recurse. + using ContextFnT = + llvm::function_ref&)>; + + // Converts a LocT to a DiagnosticLoc and its `last_byte_offset` (see + // `DiagnosticMessage`). ConvertLoc may invoke context_fn to provide context + // messages. + virtual auto ConvertLoc(LocT loc, ContextFnT context_fn) const + -> ConvertedDiagnosticLoc = 0; + + // Converts arg types as needed. Most children don't customize conversion, so + // the default returns the argument unchanged. + virtual auto ConvertArg(llvm::Any arg) const -> llvm::Any { return arg; } + private: // Converts an argument to llvm::Any for storage, handling input to storage // type conversion when needed. @@ -196,7 +185,6 @@ class DiagnosticEmitter { template friend class DiagnosticAnnotationScope; - DiagnosticConverter* converter_; DiagnosticConsumer* consumer_; llvm::SmallVectorvoid>> annotate_fns_; @@ -211,9 +199,7 @@ class DiagnosticEmitter { // 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) {} + using DiagnosticEmitter::DiagnosticEmitter; // Emits an error. This specialization only applies to // `NoLocDiagnosticEmitter`. @@ -223,14 +209,11 @@ class NoLocDiagnosticEmitter : public DiagnosticEmitter { 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_; + protected: + auto ConvertLoc(void* /*loc*/, ContextFnT /*context_fn*/) const + -> ConvertedDiagnosticLoc override { + return {.loc = {.filename = ""}, .last_byte_offset = -1}; + } }; // An RAII object that denotes a scope in which any diagnostic produced should @@ -328,7 +311,7 @@ auto DiagnosticEmitter::DiagnosticBuilder::AddMessage( if (!emitter_) { return; } - auto converted = emitter_->converter_->ConvertLoc( + auto converted = emitter_->ConvertLoc( loc, [&](DiagnosticLoc context_loc, const DiagnosticBase<>& context_diagnostic_base) { AddMessageWithDiagnosticLoc(context_loc, context_diagnostic_base, {}); @@ -397,7 +380,7 @@ auto DiagnosticEmitter::Build( template template auto DiagnosticEmitter::MakeAny(Arg arg) -> llvm::Any { - llvm::Any converted = converter_->ConvertArg(arg); + llvm::Any converted = ConvertArg(arg); using Storage = Internal::DiagnosticTypeForArg::StorageType; CARBON_CHECK(llvm::any_cast(&converted), "Failed to convert argument of type {0} to its storage type {1}", diff --git a/toolchain/diagnostics/diagnostic_emitter_test.cpp b/toolchain/diagnostics/diagnostic_emitter_test.cpp index f0d6c86ee6bf..0387b0634a98 100644 --- a/toolchain/diagnostics/diagnostic_emitter_test.cpp +++ b/toolchain/diagnostics/diagnostic_emitter_test.cpp @@ -17,7 +17,11 @@ using ::Carbon::Testing::IsDiagnostic; using ::Carbon::Testing::IsSingleDiagnostic; using testing::ElementsAre; -struct FakeDiagnosticConverter : DiagnosticConverter { +class FakeDiagnosticEmitter : public DiagnosticEmitter { + public: + using DiagnosticEmitter::DiagnosticEmitter; + + protected: auto ConvertLoc(int n, ContextFnT /*context_fn*/) const -> ConvertedDiagnosticLoc override { return {.loc = {.line_number = 1, .column_number = n}, @@ -27,11 +31,10 @@ struct FakeDiagnosticConverter : DiagnosticConverter { class DiagnosticEmitterTest : public ::testing::Test { public: - DiagnosticEmitterTest() : emitter_(converter_, consumer_) {} + DiagnosticEmitterTest() : emitter_(&consumer_) {} - FakeDiagnosticConverter converter_; Testing::MockDiagnosticConsumer consumer_; - DiagnosticEmitter emitter_; + FakeDiagnosticEmitter emitter_; }; TEST_F(DiagnosticEmitterTest, EmitSimpleError) { diff --git a/toolchain/diagnostics/file_diagnostics.h b/toolchain/diagnostics/file_diagnostics.h index 45d75dccd560..08a1956adb74 100644 --- a/toolchain/diagnostics/file_diagnostics.h +++ b/toolchain/diagnostics/file_diagnostics.h @@ -17,19 +17,14 @@ namespace Carbon { // specific emitters must be used for that. class FileDiagnosticEmitter : public DiagnosticEmitter { public: - explicit FileDiagnosticEmitter(DiagnosticConsumer* consumer) - : DiagnosticEmitter(converter_, *consumer) {} + using DiagnosticEmitter::DiagnosticEmitter; - private: + protected: // 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_; + auto ConvertLoc(llvm::StringRef filename, ContextFnT /*context_fn*/) const + -> ConvertedDiagnosticLoc override { + return {.loc = {.filename = filename}, .last_byte_offset = -1}; + } }; } // namespace Carbon diff --git a/toolchain/diagnostics/null_diagnostics.h b/toolchain/diagnostics/null_diagnostics.h index 625883190965..04755c0633e9 100644 --- a/toolchain/diagnostics/null_diagnostics.h +++ b/toolchain/diagnostics/null_diagnostics.h @@ -9,19 +9,7 @@ namespace Carbon { -template -inline auto NullDiagnosticConverter() -> DiagnosticConverter& { - struct Converter : public DiagnosticConverter { - auto ConvertLoc(LocT /*loc*/, - DiagnosticConverter::ContextFnT /*context_fn*/) const - -> ConvertedDiagnosticLoc override { - return {.loc = {}, .last_byte_offset = -1}; - } - }; - static auto* converter = new Converter; - return *converter; -} - +// Returns a singleton consumer that doesn't print its diagnostics. inline auto NullDiagnosticConsumer() -> DiagnosticConsumer& { struct Consumer : DiagnosticConsumer { auto HandleDiagnostic(Diagnostic /*d*/) -> void override {} @@ -30,10 +18,23 @@ inline auto NullDiagnosticConsumer() -> DiagnosticConsumer& { return *consumer; } +// Returns a singleton emitter that doesn't print its diagnostics. template inline auto NullDiagnosticEmitter() -> DiagnosticEmitter& { - static auto* emitter = new DiagnosticEmitter( - NullDiagnosticConverter(), NullDiagnosticConsumer()); + class Emitter : public DiagnosticEmitter { + public: + using DiagnosticEmitter::DiagnosticEmitter; + + protected: + // Converts a filename directly to the diagnostic location. + auto ConvertLoc(LocT /*loc*/, + DiagnosticEmitter::ContextFnT /*context_fn*/) const + -> ConvertedDiagnosticLoc override { + return {.loc = {}, .last_byte_offset = -1}; + } + }; + + static auto* emitter = new Emitter(&NullDiagnosticConsumer()); return *emitter; } diff --git a/toolchain/diagnostics/sorting_diagnostic_consumer_test.cpp b/toolchain/diagnostics/sorting_diagnostic_consumer_test.cpp index 084c503b06f2..3ed4856220e8 100644 --- a/toolchain/diagnostics/sorting_diagnostic_consumer_test.cpp +++ b/toolchain/diagnostics/sorting_diagnostic_consumer_test.cpp @@ -20,7 +20,11 @@ using ::testing::InSequence; CARBON_DIAGNOSTIC(TestDiagnostic, Error, "M{0}", int); -struct FakeDiagnosticConverter : DiagnosticConverter { +class FakeDiagnosticEmitter : public DiagnosticEmitter { + public: + using DiagnosticEmitter::DiagnosticEmitter; + + protected: auto ConvertLoc(int32_t last_byte_offset, ContextFnT /*context_fn*/) const -> ConvertedDiagnosticLoc override { return {.loc = {}, .last_byte_offset = last_byte_offset}; @@ -28,10 +32,9 @@ struct FakeDiagnosticConverter : DiagnosticConverter { }; TEST(SortedDiagnosticEmitterTest, SortErrors) { - FakeDiagnosticConverter converter; Testing::MockDiagnosticConsumer consumer; SortingDiagnosticConsumer sorting_consumer(consumer); - DiagnosticEmitter emitter(converter, sorting_consumer); + FakeDiagnosticEmitter emitter(&sorting_consumer); emitter.Emit(1, TestDiagnostic, 1); emitter.Emit(-1, TestDiagnostic, 2); diff --git a/toolchain/driver/compile_subcommand.cpp b/toolchain/driver/compile_subcommand.cpp index 7775168183a6..044ae7dbcc25 100644 --- a/toolchain/driver/compile_subcommand.cpp +++ b/toolchain/driver/compile_subcommand.cpp @@ -333,22 +333,15 @@ class CompilationUnit { // Parses tokens. Returns true on success. auto RunParse() -> void; - // Prepares per-IR lazy fetch functions which may come up in cross-IR - // diagnostics. - auto PreCheck() -> Parse::GetTreeAndSubtreesFn; - // Returns information needed to check this unit. - auto GetCheckUnit( - SemIR::CheckIRId check_ir_id, - llvm::ArrayRef all_trees_and_subtrees) - -> Check::Unit; + auto GetCheckUnit(SemIR::CheckIRId check_ir_id) -> Check::Unit; // Runs post-check logic. Returns true if checking succeeded for the IR. auto PostCheck() -> void; // Lower SemIR to LLVM IR. auto RunLower(std::optional> - all_trees_and_subtrees_for_debug_info) -> void; + tree_and_subtrees_getters_for_debug_info) -> void; auto RunCodeGen() -> void; @@ -363,6 +356,9 @@ class CompilationUnit { auto input_filename() -> llvm::StringRef { return input_filename_; } auto success() -> bool { return success_; } auto has_source() -> bool { return source_.has_value(); } + auto get_trees_and_subtrees() -> Parse::GetTreeAndSubtreesFn { + return *tree_and_subtrees_getter_; + } private: // Do codegen. Returns true on success. @@ -414,8 +410,7 @@ class CompilationUnit { std::optional parse_tree_; std::optional parse_tree_and_subtrees_; std::optional> - get_parse_tree_and_subtrees_; - std::optional sem_ir_converter_; + tree_and_subtrees_getter_; std::optional sem_ir_; std::unique_ptr llvm_context_; std::unique_ptr module_; @@ -500,36 +495,25 @@ auto CompilationUnit::RunParse() -> void { } } -auto CompilationUnit::PreCheck() -> Parse::GetTreeAndSubtreesFn { +auto CompilationUnit::GetCheckUnit(SemIR::CheckIRId check_ir_id) + -> Check::Unit { CARBON_CHECK(parse_tree_, "Must call RunParse first"); - CARBON_CHECK(!get_parse_tree_and_subtrees_, "Called PreCheck twice"); + CARBON_CHECK(!sem_ir_, "Called GetCheckUnit twice"); - get_parse_tree_and_subtrees_ = [this]() -> const Parse::TreeAndSubtrees& { + tree_and_subtrees_getter_ = [this]() -> const Parse::TreeAndSubtrees& { return this->GetParseTreeAndSubtrees(); }; - return *get_parse_tree_and_subtrees_; -} - -auto CompilationUnit::GetCheckUnit( - SemIR::CheckIRId check_ir_id, - llvm::ArrayRef all_trees_and_subtrees) - -> Check::Unit { - CARBON_CHECK(get_parse_tree_and_subtrees_, "Must call PreCheck first"); - CARBON_CHECK(!sem_ir_converter_, "Called GetCheckUnit twice"); - sem_ir_.emplace(&*parse_tree_, check_ir_id, parse_tree_->packaging_decl(), value_stores_, input_filename_); - sem_ir_converter_.emplace(all_trees_and_subtrees, &*sem_ir_); return {.consumer = consumer_, .value_stores = &value_stores_, .timings = timings_ ? &*timings_ : nullptr, - .get_parse_tree_and_subtrees = *get_parse_tree_and_subtrees_, - .sem_ir = &*sem_ir_, - .sem_ir_converter = &*sem_ir_converter_}; + .tree_and_subtrees_getter = *tree_and_subtrees_getter_, + .sem_ir = &*sem_ir_}; } auto CompilationUnit::PostCheck() -> void { - CARBON_CHECK(sem_ir_converter_, "Must call GetCheckUnit first"); + CARBON_CHECK(sem_ir_, "Must call GetCheckUnit first"); // We've finished all steps that can produce diagnostics. Emit the // diagnostics now, so that the developer sees them sooner and doesn't need @@ -589,15 +573,15 @@ auto CompilationUnit::PostCheck() -> void { auto CompilationUnit::RunLower( std::optional> - all_trees_and_subtrees_for_debug_info) -> void { + tree_and_subtrees_getters_for_debug_info) -> void { LogCall("Lower::LowerToLLVM", "lower", [&] { llvm_context_ = std::make_unique(); // TODO: Consider disabling instruction naming by default if we're not // producing textual LLVM IR. SemIR::InstNamer inst_namer(&*sem_ir_); module_ = Lower::LowerToLLVM( - *llvm_context_, all_trees_and_subtrees_for_debug_info, input_filename_, - *sem_ir_, &inst_namer, vlog_stream_); + *llvm_context_, tree_and_subtrees_getters_for_debug_info, + input_filename_, *sem_ir_, &inst_namer, vlog_stream_); }); if (vlog_stream_) { CARBON_VLOG("*** llvm::Module ***\n"); @@ -843,25 +827,13 @@ auto CompileSubcommand::Run(DriverEnv& driver_env) -> DriverResult { return make_result(); } - // Pre-check assigns IR IDs and constructs node converters. - llvm::SmallVector all_trees_and_subtrees; - // This size may not match due to units that are missing source, but that's an - // error case and not worth extra work. - all_trees_and_subtrees.reserve(units.size()); - for (auto& unit : units) { - if (unit->has_source()) { - all_trees_and_subtrees.push_back(unit->PreCheck()); - } - } - // Gather Check::Units. llvm::SmallVector check_units; - check_units.reserve(all_trees_and_subtrees.size()); + check_units.reserve(units.size()); for (auto& unit : units) { if (unit->has_source()) { SemIR::CheckIRId check_ir_id(check_units.size()); - check_units.push_back( - unit->GetCheckUnit(check_ir_id, all_trees_and_subtrees)); + check_units.push_back(unit->GetCheckUnit(check_ir_id)); } } @@ -888,13 +860,23 @@ auto CompileSubcommand::Run(DriverEnv& driver_env) -> DriverResult { } // Lower. + llvm::SmallVector tree_and_subtrees_getters; std::optional> - all_trees_and_subtrees_for_debug_info; + tree_and_subtrees_getters_for_debug_info; if (options_.include_debug_info) { - all_trees_and_subtrees_for_debug_info = all_trees_and_subtrees; + // This size may not match due to units that are missing source, but that's + // an error case and not worth extra work. + tree_and_subtrees_getters.reserve(units.size()); + for (auto& unit : units) { + if (unit->has_source()) { + tree_and_subtrees_getters.push_back(unit->get_trees_and_subtrees()); + } + } + tree_and_subtrees_getters_for_debug_info = {}; + tree_and_subtrees_getters_for_debug_info = tree_and_subtrees_getters; } for (const auto& unit : units) { - unit->RunLower(all_trees_and_subtrees_for_debug_info); + unit->RunLower(tree_and_subtrees_getters_for_debug_info); } if (options_.phase == CompileOptions::Phase::Lower) { return make_result(); diff --git a/toolchain/lex/BUILD b/toolchain/lex/BUILD index 6e4e850221ba..5b2f57da508a 100644 --- a/toolchain/lex/BUILD +++ b/toolchain/lex/BUILD @@ -188,6 +188,7 @@ cc_library( ":helpers", ":numeric_literal", ":string_literal", + ":token_index", ":token_kind", ":tokenized_buffer", "//common:check", diff --git a/toolchain/lex/lex.cpp b/toolchain/lex/lex.cpp index 257a5a327cf0..eb73ddddd5f0 100644 --- a/toolchain/lex/lex.cpp +++ b/toolchain/lex/lex.cpp @@ -17,6 +17,7 @@ #include "toolchain/lex/helpers.h" #include "toolchain/lex/numeric_literal.h" #include "toolchain/lex/string_literal.h" +#include "toolchain/lex/token_index.h" #include "toolchain/lex/token_kind.h" #include "toolchain/lex/tokenized_buffer.h" @@ -83,10 +84,8 @@ class [[clang::internal_linkage]] Lexer { DiagnosticConsumer& consumer) : buffer_(value_stores, source), consumer_(consumer), - converter_(&buffer_), - emitter_(converter_, consumer_), - token_converter_(&buffer_), - token_emitter_(token_converter_, consumer_) {} + emitter_(&consumer_, &buffer_), + token_emitter_(&consumer_, &buffer_) {} // Find all line endings and create the line data structures. // @@ -223,11 +222,9 @@ class [[clang::internal_linkage]] Lexer { ErrorTrackingDiagnosticConsumer consumer_; - TokenizedBuffer::SourceBufferDiagnosticConverter converter_; - LexerDiagnosticEmitter emitter_; + TokenizedBuffer::SourcePointerDiagnosticEmitter emitter_; - TokenDiagnosticConverter token_converter_; - TokenDiagnosticEmitter token_emitter_; + TokenizedBuffer::TokenDiagnosticEmitter token_emitter_; }; #if CARBON_USE_SIMD @@ -1529,7 +1526,7 @@ class Lexer::ErrorRecoveryBuffer { }; // Issue an UnmatchedOpening diagnostic. -static auto DiagnoseUnmatchedOpening(TokenDiagnosticEmitter& emitter, +static auto DiagnoseUnmatchedOpening(DiagnosticEmitter& emitter, TokenIndex opening_token) -> void { CARBON_DIAGNOSTIC(UnmatchedOpening, Error, "opening symbol without a corresponding closing symbol"); diff --git a/toolchain/lex/numeric_literal_benchmark.cpp b/toolchain/lex/numeric_literal_benchmark.cpp index 5f1e77e8959b..847557a43ca2 100644 --- a/toolchain/lex/numeric_literal_benchmark.cpp +++ b/toolchain/lex/numeric_literal_benchmark.cpp @@ -26,7 +26,7 @@ static void BM_Lex_Int(benchmark::State& state) { static void BM_ComputeValue_Float(benchmark::State& state) { auto val = NumericLiteral::Lex("0.000001", true); CARBON_CHECK(val); - auto emitter = NullDiagnosticEmitter(); + auto& emitter = NullDiagnosticEmitter(); for (auto _ : state) { val->ComputeValue(emitter); } @@ -34,7 +34,7 @@ static void BM_ComputeValue_Float(benchmark::State& state) { static void BM_ComputeValue_Int(benchmark::State& state) { auto val = NumericLiteral::Lex("1_234_567_890", true); - auto emitter = NullDiagnosticEmitter(); + auto& emitter = NullDiagnosticEmitter(); CARBON_CHECK(val); for (auto _ : state) { val->ComputeValue(emitter); diff --git a/toolchain/lex/numeric_literal_test.cpp b/toolchain/lex/numeric_literal_test.cpp index 18bf4df92031..e07ba045b7c4 100644 --- a/toolchain/lex/numeric_literal_test.cpp +++ b/toolchain/lex/numeric_literal_test.cpp @@ -37,8 +37,7 @@ class NumericLiteralTest : public ::testing::Test { auto Parse(llvm::StringRef text, bool can_form_real_literal = true) -> NumericLiteral::Value { - Testing::SingleTokenDiagnosticConverter converter(text); - DiagnosticEmitter emitter(converter, error_tracker); + Testing::SingleTokenDiagnosticEmitter emitter(&error_tracker, text); return Lex(text, can_form_real_literal).ComputeValue(emitter); } diff --git a/toolchain/lex/string_literal_test.cpp b/toolchain/lex/string_literal_test.cpp index 32819568c238..96749f202040 100644 --- a/toolchain/lex/string_literal_test.cpp +++ b/toolchain/lex/string_literal_test.cpp @@ -27,8 +27,7 @@ class StringLiteralTest : public ::testing::Test { auto Parse(llvm::StringRef text) -> llvm::StringRef { StringLiteral token = Lex(text); - Testing::SingleTokenDiagnosticConverter converter(text); - DiagnosticEmitter emitter(converter, error_tracker_); + Testing::SingleTokenDiagnosticEmitter emitter(&error_tracker_, text); return token.ComputeValue(allocator_, emitter); } diff --git a/toolchain/lex/test_helpers.h b/toolchain/lex/test_helpers.h index d1632b8099b6..1e62100f5c33 100644 --- a/toolchain/lex/test_helpers.h +++ b/toolchain/lex/test_helpers.h @@ -17,13 +17,15 @@ namespace Carbon::Testing { // A diagnostic converter for tests that lex a single token. Produces // locations such as "`12.5`:1:3" to refer to the third character in the token. -class SingleTokenDiagnosticConverter : public DiagnosticConverter { +class SingleTokenDiagnosticEmitter : public DiagnosticEmitter { public: // Form a converter for a given token. The string provided here must refer // to the same character array that we are going to lex. - explicit SingleTokenDiagnosticConverter(llvm::StringRef token) - : token_(token) {} + explicit SingleTokenDiagnosticEmitter(DiagnosticConsumer* consumer, + llvm::StringRef token) + : DiagnosticEmitter(consumer), token_(token) {} + protected: // Implements `DiagnosticConverter::ConvertLoc`. auto ConvertLoc(const char* pos, ContextFnT /*context_fn*/) const -> ConvertedDiagnosticLoc override { diff --git a/toolchain/lex/tokenized_buffer.cpp b/toolchain/lex/tokenized_buffer.cpp index 43e4c1241aff..f8d9ebbf372a 100644 --- a/toolchain/lex/tokenized_buffer.cpp +++ b/toolchain/lex/tokenized_buffer.cpp @@ -415,12 +415,6 @@ auto TokenizedBuffer::SourcePointerToDiagnosticLoc(const char* loc) const .last_byte_offset = offset}; } -auto TokenizedBuffer::SourceBufferDiagnosticConverter::ConvertLoc( - const char* loc, ContextFnT /*context_fn*/) const - -> ConvertedDiagnosticLoc { - return tokens_->SourcePointerToDiagnosticLoc(loc); -} - auto TokenizedBuffer::TokenToDiagnosticLoc(TokenIndex token) const -> ConvertedDiagnosticLoc { // Map the token location into a position within the source buffer. @@ -435,10 +429,4 @@ auto TokenizedBuffer::TokenToDiagnosticLoc(TokenIndex token) const return converted; } -auto TokenDiagnosticConverter::ConvertLoc(TokenIndex token, - ContextFnT /*context_fn*/) const - -> ConvertedDiagnosticLoc { - return tokens_->TokenToDiagnosticLoc(token); -} - } // namespace Carbon::Lex diff --git a/toolchain/lex/tokenized_buffer.h b/toolchain/lex/tokenized_buffer.h index 81d0c2db2da1..73ef4b62990e 100644 --- a/toolchain/lex/tokenized_buffer.h +++ b/toolchain/lex/tokenized_buffer.h @@ -60,24 +60,6 @@ using CommentIterator = IndexIterator; // Random-access iterator over tokens within the buffer. using TokenIterator = IndexIterator; -// A diagnostic location converter that maps token locations into source -// buffer locations. -class TokenDiagnosticConverter : public DiagnosticConverter { - public: - explicit TokenDiagnosticConverter(const TokenizedBuffer* tokens) - : tokens_(tokens) {} - - // Implements `DiagnosticConverter::ConvertLoc`. - auto ConvertLoc(TokenIndex token, ContextFnT context_fn) const - -> ConvertedDiagnosticLoc override; - - protected: - auto tokens() const -> const TokenizedBuffer& { return *tokens_; } - - private: - const TokenizedBuffer* tokens_; -}; - // A buffer of tokenized Carbon source code. // // This is constructed by lexing the source code text into a series of tokens. @@ -225,17 +207,33 @@ class TokenizedBuffer : public Printable { private: friend class Lexer; - // A diagnostic location converter that maps token locations into source - // buffer locations. - class SourceBufferDiagnosticConverter - : public DiagnosticConverter { + class SourcePointerDiagnosticEmitter : public DiagnosticEmitter { public: - explicit SourceBufferDiagnosticConverter(const TokenizedBuffer* tokens) - : tokens_(tokens) {} + explicit SourcePointerDiagnosticEmitter(DiagnosticConsumer* consumer, + const TokenizedBuffer* tokens) + : DiagnosticEmitter(consumer), tokens_(tokens) {} - // Implements `DiagnosticConverter::ConvertLoc`. - auto ConvertLoc(const char* loc, ContextFnT context_fn) const - -> ConvertedDiagnosticLoc override; + protected: + auto ConvertLoc(const char* loc, ContextFnT /*context_fn*/) const + -> ConvertedDiagnosticLoc override { + return tokens_->SourcePointerToDiagnosticLoc(loc); + } + + private: + const TokenizedBuffer* tokens_; + }; + + class TokenDiagnosticEmitter : public DiagnosticEmitter { + public: + explicit TokenDiagnosticEmitter(DiagnosticConsumer* consumer, + const TokenizedBuffer* tokens) + : DiagnosticEmitter(consumer), tokens_(tokens) {} + + protected: + auto ConvertLoc(TokenIndex token, ContextFnT /*context_fn*/) const + -> ConvertedDiagnosticLoc override { + return tokens_->TokenToDiagnosticLoc(token); + } private: const TokenizedBuffer* tokens_; @@ -493,13 +491,6 @@ class TokenizedBuffer : public Printable { llvm::BitVector recovery_tokens_; }; -// A diagnostic emitter that uses positions within a source buffer's text as -// its source of location information. -using LexerDiagnosticEmitter = DiagnosticEmitter; - -// A diagnostic emitter that uses tokens as its source of location information. -using TokenDiagnosticEmitter = DiagnosticEmitter; - inline auto TokenizedBuffer::GetKind(TokenIndex token) const -> TokenKind { return GetTokenInfo(token).kind(); } diff --git a/toolchain/lower/BUILD b/toolchain/lower/BUILD index 285513b13793..049d9c3aa870 100644 --- a/toolchain/lower/BUILD +++ b/toolchain/lower/BUILD @@ -17,7 +17,7 @@ cc_library( hdrs = ["lower.h"], deps = [ ":context", - "//toolchain/check:sem_ir_diagnostic_converter", + "//toolchain/check:sem_ir_loc_diagnostic_emitter", "//toolchain/parse:tree", "//toolchain/sem_ir:file", "//toolchain/sem_ir:inst_namer", @@ -50,7 +50,7 @@ cc_library( "//common:raw_string_ostream", "//common:vlog", "//toolchain/base:kind_switch", - "//toolchain/check:sem_ir_diagnostic_converter", + "//toolchain/check:sem_ir_loc_diagnostic_emitter", "//toolchain/sem_ir:absolute_node_id", "//toolchain/sem_ir:entry_point", "//toolchain/sem_ir:file", diff --git a/toolchain/lower/file_context.cpp b/toolchain/lower/file_context.cpp index 22e79e19e83d..efa72b028ef9 100644 --- a/toolchain/lower/file_context.cpp +++ b/toolchain/lower/file_context.cpp @@ -26,18 +26,18 @@ namespace Carbon::Lower { FileContext::FileContext( llvm::LLVMContext& llvm_context, std::optional> - all_trees_and_subtrees_for_debug_info, + tree_and_subtrees_getters_for_debug_info, llvm::StringRef module_name, const SemIR::File& sem_ir, const SemIR::InstNamer* inst_namer, llvm::raw_ostream* vlog_stream) : llvm_context_(&llvm_context), llvm_module_(std::make_unique(module_name, llvm_context)), di_builder_(*llvm_module_), di_compile_unit_( - all_trees_and_subtrees_for_debug_info + tree_and_subtrees_getters_for_debug_info ? BuildDICompileUnit(module_name, *llvm_module_, di_builder_) : nullptr), - all_trees_and_subtrees_for_debug_info_( - all_trees_and_subtrees_for_debug_info), + tree_and_subtrees_getters_for_debug_info_( + tree_and_subtrees_getters_for_debug_info), sem_ir_(&sem_ir), inst_namer_(inst_namer), vlog_stream_(vlog_stream) { @@ -621,7 +621,8 @@ auto FileContext::BuildGlobalVariableDecl(SemIR::VarStorage var_storage) auto FileContext::GetLocForDI(SemIR::InstId inst_id) -> LocForDI { SemIR::AbsoluteNodeId resolved = GetAbsoluteNodeId(sem_ir_, inst_id).back(); const auto& tree_and_subtrees = - (*all_trees_and_subtrees_for_debug_info_)[resolved.check_ir_id.index](); + (*tree_and_subtrees_getters_for_debug_info_)[resolved.check_ir_id + .index](); const auto& tokens = tree_and_subtrees.tree().tokens(); if (resolved.node_id.has_value()) { diff --git a/toolchain/lower/file_context.h b/toolchain/lower/file_context.h index 79f9bae95011..5cd68da6917f 100644 --- a/toolchain/lower/file_context.h +++ b/toolchain/lower/file_context.h @@ -9,7 +9,7 @@ #include "llvm/IR/DIBuilder.h" #include "llvm/IR/LLVMContext.h" #include "llvm/IR/Module.h" -#include "toolchain/check/sem_ir_diagnostic_converter.h" +#include "toolchain/check/sem_ir_loc_diagnostic_emitter.h" #include "toolchain/sem_ir/file.h" #include "toolchain/sem_ir/inst_namer.h" @@ -30,7 +30,7 @@ class FileContext { explicit FileContext( llvm::LLVMContext& llvm_context, std::optional> - all_trees_and_subtrees_for_debug_info, + tree_and_subtrees_getters_for_debug_info, llvm::StringRef module_name, const SemIR::File& sem_ir, const SemIR::InstNamer* inst_namer, llvm::raw_ostream* vlog_stream); @@ -131,7 +131,7 @@ class FileContext { // The trees are only provided when debug info should be emitted. std::optional> - all_trees_and_subtrees_for_debug_info_; + tree_and_subtrees_getters_for_debug_info_; // The input SemIR. const SemIR::File* const sem_ir_; diff --git a/toolchain/lower/lower.cpp b/toolchain/lower/lower.cpp index d4d5accb6ee6..8dec5936967d 100644 --- a/toolchain/lower/lower.cpp +++ b/toolchain/lower/lower.cpp @@ -10,12 +10,12 @@ namespace Carbon::Lower { auto LowerToLLVM(llvm::LLVMContext& llvm_context, std::optional> - all_trees_and_subtrees_for_debug_info, + tree_and_subtrees_getters_for_debug_info, llvm::StringRef module_name, const SemIR::File& sem_ir, const SemIR::InstNamer* inst_namer, llvm::raw_ostream* vlog_stream) -> std::unique_ptr { - FileContext context(llvm_context, all_trees_and_subtrees_for_debug_info, + FileContext context(llvm_context, tree_and_subtrees_getters_for_debug_info, module_name, sem_ir, inst_namer, vlog_stream); return context.Run(); } diff --git a/toolchain/lower/lower.h b/toolchain/lower/lower.h index b651528e00fb..b1c628c4a867 100644 --- a/toolchain/lower/lower.h +++ b/toolchain/lower/lower.h @@ -17,7 +17,7 @@ namespace Carbon::Lower { // Lowers SemIR to LLVM IR. auto LowerToLLVM(llvm::LLVMContext& llvm_context, std::optional> - all_trees_and_subtrees_for_debug_info, + tree_and_subtrees_getters_for_debug_info, llvm::StringRef module_name, const SemIR::File& sem_ir, const SemIR::InstNamer* inst_namer, llvm::raw_ostream* vlog_stream) diff --git a/toolchain/parse/context.cpp b/toolchain/parse/context.cpp index 98fe8960c42e..55ffda244524 100644 --- a/toolchain/parse/context.cpp +++ b/toolchain/parse/context.cpp @@ -9,6 +9,7 @@ #include "common/check.h" #include "common/ostream.h" #include "llvm/ADT/STLExtras.h" +#include "toolchain/diagnostics/diagnostic_emitter.h" #include "toolchain/diagnostics/format_providers.h" #include "toolchain/lex/token_kind.h" #include "toolchain/lex/tokenized_buffer.h" @@ -24,9 +25,8 @@ Context::Context(Tree* tree, Lex::TokenizedBuffer* tokens, DiagnosticConsumer* consumer, llvm::raw_ostream* vlog_stream) : tree_(tree), tokens_(tokens), - converter_(tokens, &position_), err_tracker_(*consumer), - emitter_(converter_, err_tracker_), + emitter_(&err_tracker_, this), vlog_stream_(vlog_stream), position_(tokens_->tokens().begin()), end_(tokens_->tokens().end()) { diff --git a/toolchain/parse/context.h b/toolchain/parse/context.h index 3fbdf69f1ae7..1f92c9ccacb6 100644 --- a/toolchain/parse/context.h +++ b/toolchain/parse/context.h @@ -33,6 +33,28 @@ enum class Lookahead : int32_t { // documentation. class Context { public: + // A token-based emitter for use during parse. + class Emitter : public DiagnosticEmitter { + public: + explicit Emitter(DiagnosticConsumer* consumer, Context* context) + : DiagnosticEmitter(consumer), context_(context) {} + + protected: + // Applies the `position_` to the `last_byte_offset` returned by + // `TokenToDiagnosticLoc`. + auto ConvertLoc(Lex::TokenIndex token, ContextFnT /*context_fn*/) const + -> ConvertedDiagnosticLoc override { + auto converted = context_->tokens().TokenToDiagnosticLoc(token); + converted.last_byte_offset = + std::max(converted.last_byte_offset, + context_->tokens().GetByteOffset(*context_->position())); + return converted; + } + + private: + Context* context_; + }; + // Possible operator fixities for errors. enum class OperatorFixity : int8_t { Prefix, Infix, Postfix }; @@ -376,7 +398,7 @@ class Context { auto has_errors() const -> bool { return err_tracker_.seen_error(); } - auto emitter() -> Lex::TokenDiagnosticEmitter& { return emitter_; } + auto emitter() -> Emitter& { return emitter_; } auto position() -> Lex::TokenIterator& { return position_; } auto position() const -> Lex::TokenIterator { return position_; } @@ -402,30 +424,6 @@ class Context { } private: - // Applies the `position_` to the `last_byte_offset` returned by - // `TokenToDiagnosticLoc`. - class TokenDiagnosticConverterForParse - : public DiagnosticConverter { - public: - explicit TokenDiagnosticConverterForParse(Lex::TokenizedBuffer* tokens, - Lex::TokenIterator* position) - : tokens_(tokens), position_(position) {} - - auto ConvertLoc(Lex::TokenIndex token, ContextFnT /*context_fn*/) const - -> ConvertedDiagnosticLoc override { - auto converted = tokens_->TokenToDiagnosticLoc(token); - converted.last_byte_offset = std::max( - converted.last_byte_offset, tokens_->GetByteOffset(**position_)); - return converted; - } - - private: - Lex::TokenizedBuffer* tokens_; - - // The position in `Parse()`. - Lex::TokenIterator* position_; - }; - // Prints a single token for a stack dump. Used by PrintForStackDump. auto PrintTokenForStackDump(llvm::raw_ostream& output, Lex::TokenIndex token) const -> void; @@ -433,9 +431,8 @@ class Context { Tree* tree_; Lex::TokenizedBuffer* tokens_; - TokenDiagnosticConverterForParse converter_; ErrorTrackingDiagnosticConsumer err_tracker_; - Lex::TokenDiagnosticEmitter emitter_; + Emitter emitter_; // Whether to print verbose output. llvm::raw_ostream* vlog_stream_;