diff --git a/toolchain/check/BUILD b/toolchain/check/BUILD index 8c55c261933a..48ee3d8687db 100644 --- a/toolchain/check/BUILD +++ b/toolchain/check/BUILD @@ -276,6 +276,7 @@ cc_library( deps = [ ":context", "//toolchain/diagnostics:diagnostic_emitter", + "//toolchain/lex:token_index", "//toolchain/parse:tree_node_diagnostic_converter", "//toolchain/sem_ir:file", "//toolchain/sem_ir:stringify_type", diff --git a/toolchain/check/check_unit.cpp b/toolchain/check/check_unit.cpp index e4efef0f7a69..6f7e9189d177 100644 --- a/toolchain/check/check_unit.cpp +++ b/toolchain/check/check_unit.cpp @@ -335,17 +335,19 @@ auto CheckUnit::ProcessNodeIds() -> bool { // On crash, report which token we were handling. PrettyStackTraceFunction node_dumper([&](llvm::raw_ostream& output) { - auto loc = unit_and_imports_->unit->node_converter->ConvertLoc( + auto converted = unit_and_imports_->unit->node_converter->ConvertLoc( node_id, [](DiagnosticLoc, const DiagnosticBase<>&) {}); - loc.FormatLocation(output); + converted.loc.FormatLocation(output); output << ": checking " << context_.parse_tree().node_kind(node_id) << "\n"; // Crash output has a tab indent; try to indent slightly past that. - loc.FormatSnippet(output, /*indent=*/10); + converted.loc.FormatSnippet(output, /*indent=*/10); }); while (auto maybe_node_id = traversal.Next()) { node_id = *maybe_node_id; - auto parse_kind = context_.parse_tree().node_kind(node_id); + + unit_and_imports_->unit->sem_ir_converter->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`"); @@ -353,6 +355,7 @@ auto CheckUnit::ProcessNodeIds() -> bool { } bool result; + auto parse_kind = context_.parse_tree().node_kind(node_id); switch (parse_kind) { #define CARBON_PARSE_NODE_KIND(Name) \ case Parse::NodeKind::Name: { \ diff --git a/toolchain/check/sem_ir_diagnostic_converter.cpp b/toolchain/check/sem_ir_diagnostic_converter.cpp index b4acabdb948f..2cf0e846dbd8 100644 --- a/toolchain/check/sem_ir_diagnostic_converter.cpp +++ b/toolchain/check/sem_ir_diagnostic_converter.cpp @@ -9,7 +9,30 @@ namespace Carbon::Check { auto SemIRDiagnosticConverter::ConvertLoc(SemIRLoc loc, ContextFnT context_fn) const - -> DiagnosticLoc { + -> ConvertedDiagnosticLoc { + auto converted = ConvertLocImpl(loc, context_fn); + + // Use the token when possible, but -1 is the default value. + auto last_offset = -1; + if (last_token_.is_valid()) { + last_offset = sem_ir_->parse_tree().tokens().GetByteOffset(last_token_); + } + + // When the diagnostic is in the same file, we use the last possible offset; + // otherwise, we ignore the offset because it's probably in that file. + if (converted.loc.filename == sem_ir_->filename()) { + converted.last_byte_offset = + std::max(converted.last_byte_offset, last_offset); + } else { + converted.last_byte_offset = last_offset; + } + + return converted; +} + +auto SemIRDiagnosticConverter::ConvertLocImpl(SemIRLoc loc, + ContextFnT context_fn) const + -> ConvertedDiagnosticLoc { // Cursors for the current IR and instruction in that IR. const auto* cursor_ir = sem_ir_; auto cursor_inst_id = SemIR::InstId::Invalid; @@ -23,7 +46,7 @@ auto SemIRDiagnosticConverter::ConvertLoc(SemIRLoc loc, "If we get invalid locations here, we may need to more " "thoroughly track ImportDecls."); - DiagnosticLoc in_import_loc; + ConvertedDiagnosticLoc in_import_loc; auto import_loc_id = cursor_ir->insts().GetLocId(import_ir.decl_id); if (import_loc_id.is_node_id()) { // For imports in the current file, the location is simple. @@ -50,7 +73,7 @@ auto SemIRDiagnosticConverter::ConvertLoc(SemIRLoc loc, if (import_loc_id.is_valid()) { // TODO: Include the name of the imported library in the diagnostic. CARBON_DIAGNOSTIC(InImport, LocationInfo, "in import"); - context_fn(in_import_loc, InImport); + context_fn(in_import_loc.loc, InImport); } cursor_ir = import_ir.sem_ir; @@ -59,7 +82,8 @@ auto SemIRDiagnosticConverter::ConvertLoc(SemIRLoc loc, // If the location is is an import, follows it and returns nullopt. // Otherwise, it's a parse node, so return the final location. - auto handle_loc = [&](SemIR::LocId loc_id) -> std::optional { + auto handle_loc = + [&](SemIR::LocId loc_id) -> std::optional { if (loc_id.is_import_ir_inst_id()) { follow_import_ref(loc_id.import_ir_inst_id()); return std::nullopt; @@ -172,7 +196,7 @@ auto SemIRDiagnosticConverter::ConvertLocInFile(const SemIR::File* sem_ir, Parse::NodeId node_id, bool token_only, ContextFnT context_fn) const - -> DiagnosticLoc { + -> ConvertedDiagnosticLoc { return node_converters_[sem_ir->check_ir_id().index]->ConvertLoc( Parse::NodeLoc(node_id, token_only), context_fn); } diff --git a/toolchain/check/sem_ir_diagnostic_converter.h b/toolchain/check/sem_ir_diagnostic_converter.h index 34743ebd0781..810ff02dbc3c 100644 --- a/toolchain/check/sem_ir_diagnostic_converter.h +++ b/toolchain/check/sem_ir_diagnostic_converter.h @@ -8,6 +8,7 @@ #include "llvm/ADT/ArrayRef.h" #include "toolchain/check/diagnostic_helpers.h" #include "toolchain/diagnostics/diagnostic_converter.h" +#include "toolchain/lex/token_index.h" #include "toolchain/parse/tree_node_diagnostic_converter.h" #include "toolchain/sem_ir/file.h" @@ -21,27 +22,43 @@ class SemIRDiagnosticConverter : public DiagnosticConverter { const SemIR::File* sem_ir) : node_converters_(node_converters), sem_ir_(sem_ir) {} - // Converts an instruction's location to a diagnostic location, which will be - // the underlying line of code. Adds context for any imports used in the - // current SemIR to get to the underlying code. + // Implements `DiagnosticConverter::ConvertLoc`. Adds context for any imports + // used in the current SemIR to get to the underlying code. + // + // For the last byte offset, this uses `last_token_` exclusively for imported + // locations, or `loc` if it's in the same file and (for whatever reason) + // later. auto ConvertLoc(SemIRLoc loc, ContextFnT context_fn) const - -> DiagnosticLoc override; + -> 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 + -> ConvertedDiagnosticLoc; + // Converts a node_id corresponding to a specific sem_ir to a diagnostic // location. auto ConvertLocInFile(const SemIR::File* sem_ir, Parse::NodeId node_id, bool token_only, ContextFnT context_fn) const - -> DiagnosticLoc; + -> ConvertedDiagnosticLoc; // Converters for each SemIR. llvm::ArrayRef node_converters_; // The current SemIR being processed. const SemIR::File* sem_ir_; + + // The last token encountered during processing. + Lex::TokenIndex last_token_ = Lex::TokenIndex::Invalid; }; } // namespace Carbon::Check diff --git a/toolchain/check/testdata/alias/no_prelude/fail_modifiers.carbon b/toolchain/check/testdata/alias/no_prelude/fail_modifiers.carbon index 7d0821edd161..7498b7f41d77 100644 --- a/toolchain/check/testdata/alias/no_prelude/fail_modifiers.carbon +++ b/toolchain/check/testdata/alias/no_prelude/fail_modifiers.carbon @@ -10,28 +10,28 @@ class Class {} -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+25]]:1: error: `abstract` not allowed on `alias` declaration [ModifierNotAllowedOnDeclaration] -// CHECK:STDERR: abstract base default final alias A = Class; -// CHECK:STDERR: ^~~~~~~~ -// CHECK:STDERR: -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+21]]:10: error: `base` not allowed on declaration with `abstract` [ModifierNotAllowedWith] +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+25]]:10: error: `base` not allowed on declaration with `abstract` [ModifierNotAllowedWith] // CHECK:STDERR: abstract base default final alias A = Class; // CHECK:STDERR: ^~~~ -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+18]]:1: note: `abstract` previously appeared here [ModifierPrevious] +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+22]]:1: note: `abstract` previously appeared here [ModifierPrevious] // CHECK:STDERR: abstract base default final alias A = Class; // CHECK:STDERR: ^~~~~~~~ // CHECK:STDERR: -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+14]]:15: error: `default` not allowed on declaration with `abstract` [ModifierNotAllowedWith] +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+18]]:15: error: `default` not allowed on declaration with `abstract` [ModifierNotAllowedWith] // CHECK:STDERR: abstract base default final alias A = Class; // CHECK:STDERR: ^~~~~~~ -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:1: note: `abstract` previously appeared here [ModifierPrevious] +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+15]]:1: note: `abstract` previously appeared here [ModifierPrevious] // CHECK:STDERR: abstract base default final alias A = Class; // CHECK:STDERR: ^~~~~~~~ // CHECK:STDERR: -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+7]]:23: error: `final` not allowed on declaration with `abstract` [ModifierNotAllowedWith] +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:23: error: `final` not allowed on declaration with `abstract` [ModifierNotAllowedWith] // CHECK:STDERR: abstract base default final alias A = Class; // CHECK:STDERR: ^~~~~ -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:1: note: `abstract` previously appeared here [ModifierPrevious] +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+8]]:1: note: `abstract` previously appeared here [ModifierPrevious] +// CHECK:STDERR: abstract base default final alias A = Class; +// CHECK:STDERR: ^~~~~~~~ +// CHECK:STDERR: +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:1: error: `abstract` not allowed on `alias` declaration [ModifierNotAllowedOnDeclaration] // CHECK:STDERR: abstract base default final alias A = Class; // CHECK:STDERR: ^~~~~~~~ // CHECK:STDERR: diff --git a/toolchain/check/testdata/class/fail_modifiers.carbon b/toolchain/check/testdata/class/fail_modifiers.carbon index fa8b96559a53..6431dbfa1b14 100644 --- a/toolchain/check/testdata/class/fail_modifiers.carbon +++ b/toolchain/check/testdata/class/fail_modifiers.carbon @@ -47,18 +47,18 @@ base class BaseDecl; // CHECK:STDERR: abstract abstract class TwoAbstract { } -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+15]]:1: error: `protected` not allowed; requires class scope [ModifierProtectedNotAllowed] +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+15]]:19: error: `base` not allowed on declaration with `virtual` [ModifierNotAllowedWith] // CHECK:STDERR: protected virtual base class Virtual {} -// CHECK:STDERR: ^~~~~~~~~ -// CHECK:STDERR: -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:11: error: `virtual` not allowed on `class` declaration [ModifierNotAllowedOnDeclaration] +// CHECK:STDERR: ^~~~ +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+12]]:11: note: `virtual` previously appeared here [ModifierPrevious] // CHECK:STDERR: protected virtual base class Virtual {} // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+7]]:19: error: `base` not allowed on declaration with `virtual` [ModifierNotAllowedWith] +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+8]]:1: error: `protected` not allowed; requires class scope [ModifierProtectedNotAllowed] // CHECK:STDERR: protected virtual base class Virtual {} -// CHECK:STDERR: ^~~~ -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:11: note: `virtual` previously appeared here [ModifierPrevious] +// CHECK:STDERR: ^~~~~~~~~ +// CHECK:STDERR: +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:11: error: `virtual` not allowed on `class` declaration [ModifierNotAllowedOnDeclaration] // CHECK:STDERR: protected virtual base class Virtual {} // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: diff --git a/toolchain/check/testdata/function/declaration/no_prelude/extern_library.carbon b/toolchain/check/testdata/function/declaration/no_prelude/extern_library.carbon index 793c9359b721..db1639504099 100644 --- a/toolchain/check/testdata/function/declaration/no_prelude/extern_library.carbon +++ b/toolchain/check/testdata/function/declaration/no_prelude/extern_library.carbon @@ -102,16 +102,16 @@ extern library "extern_library_owner" fn F(); library "[[@TEST_NAME]]"; -// CHECK:STDERR: fail_extern_library_mismatch_owner.carbon:[[@LINE+4]]:1: error: semantics TODO: `handle invalid parse trees in `check`` [SemanticsTodo] -// CHECK:STDERR: import library "extern_library_mismatch" -// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -// CHECK:STDERR: import library "extern_library_mismatch" -// CHECK:STDERR: fail_extern_library_mismatch_owner.carbon:[[@LINE+4]]:1: error: `import` declarations must end with a `;` [ExpectedDeclSemi] +// CHECK:STDERR: fail_extern_library_mismatch_owner.carbon:[[@LINE+8]]:1: error: `import` declarations must end with a `;` [ExpectedDeclSemi] // CHECK:STDERR: extern fn F(); // CHECK:STDERR: ^~~~~~ // CHECK:STDERR: +// CHECK:STDERR: fail_extern_library_mismatch_owner.carbon:[[@LINE-6]]:1: error: semantics TODO: `handle invalid parse trees in `check`` [SemanticsTodo] +// CHECK:STDERR: import library "extern_library_mismatch" +// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +// CHECK:STDERR: extern fn F(); // --- fail_extern_self_library.carbon diff --git a/toolchain/check/testdata/function/declaration/no_prelude/fail_modifiers.carbon b/toolchain/check/testdata/function/declaration/no_prelude/fail_modifiers.carbon index c4564bde8b13..d8ffb2d2969f 100644 --- a/toolchain/check/testdata/function/declaration/no_prelude/fail_modifiers.carbon +++ b/toolchain/check/testdata/function/declaration/no_prelude/fail_modifiers.carbon @@ -8,27 +8,27 @@ // TIP: To dump output, run: // TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/function/declaration/no_prelude/fail_modifiers.carbon -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:1: error: `default` not allowed; requires interface scope [ModifierRequiresInterface] +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:9: error: `protected` must appear before `default` [ModifierMustAppearBefore] +// CHECK:STDERR: default protected fn WrongOrder(); +// CHECK:STDERR: ^~~~~~~~~ +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+8]]:1: note: `default` previously appeared here [ModifierPrevious] // CHECK:STDERR: default protected fn WrongOrder(); // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+7]]:9: error: `protected` must appear before `default` [ModifierMustAppearBefore] -// CHECK:STDERR: default protected fn WrongOrder(); -// CHECK:STDERR: ^~~~~~~~~ -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:1: note: `default` previously appeared here [ModifierPrevious] +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:1: error: `default` not allowed; requires interface scope [ModifierRequiresInterface] // CHECK:STDERR: default protected fn WrongOrder(); // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: default protected fn WrongOrder(); -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:1: error: `virtual` not allowed; requires class scope [ModifierRequiresClass] +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:9: error: `virtual` repeated on declaration [ModifierRepeated] +// CHECK:STDERR: virtual virtual fn DuplicateVirtual() {} +// CHECK:STDERR: ^~~~~~~ +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+8]]:1: note: `virtual` previously appeared here [ModifierPrevious] // CHECK:STDERR: virtual virtual fn DuplicateVirtual() {} // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+7]]:9: error: `virtual` repeated on declaration [ModifierRepeated] -// CHECK:STDERR: virtual virtual fn DuplicateVirtual() {} -// CHECK:STDERR: ^~~~~~~ -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:1: note: `virtual` previously appeared here [ModifierPrevious] +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:1: error: `virtual` not allowed; requires class scope [ModifierRequiresClass] // CHECK:STDERR: virtual virtual fn DuplicateVirtual() {} // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: @@ -43,14 +43,14 @@ virtual virtual fn DuplicateVirtual() {} // CHECK:STDERR: private protected fn TwoAccess(); -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:1: error: `abstract` not allowed; requires class scope [ModifierRequiresClass] +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:10: error: `virtual` not allowed on declaration with `abstract` [ModifierNotAllowedWith] +// CHECK:STDERR: abstract virtual fn ModifiersConflict() {} +// CHECK:STDERR: ^~~~~~~ +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+8]]:1: note: `abstract` previously appeared here [ModifierPrevious] // CHECK:STDERR: abstract virtual fn ModifiersConflict() {} // CHECK:STDERR: ^~~~~~~~ // CHECK:STDERR: -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+7]]:10: error: `virtual` not allowed on declaration with `abstract` [ModifierNotAllowedWith] -// CHECK:STDERR: abstract virtual fn ModifiersConflict() {} -// CHECK:STDERR: ^~~~~~~ -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:1: note: `abstract` previously appeared here [ModifierPrevious] +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:1: error: `abstract` not allowed; requires class scope [ModifierRequiresClass] // CHECK:STDERR: abstract virtual fn ModifiersConflict() {} // CHECK:STDERR: ^~~~~~~~ // CHECK:STDERR: @@ -62,21 +62,21 @@ abstract virtual fn ModifiersConflict() {} // CHECK:STDERR: base fn InvalidModifier(); -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+18]]:1: error: `default` not allowed; requires interface scope [ModifierRequiresInterface] -// CHECK:STDERR: default final virtual fn ModifiersConflict2() {} -// CHECK:STDERR: ^~~~~~~ -// CHECK:STDERR: -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+14]]:9: error: `final` not allowed on declaration with `default` [ModifierNotAllowedWith] +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+18]]:9: error: `final` not allowed on declaration with `default` [ModifierNotAllowedWith] // CHECK:STDERR: default final virtual fn ModifiersConflict2() {} // CHECK:STDERR: ^~~~~ -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:1: note: `default` previously appeared here [ModifierPrevious] +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+15]]:1: note: `default` previously appeared here [ModifierPrevious] // CHECK:STDERR: default final virtual fn ModifiersConflict2() {} // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+7]]:15: error: `virtual` not allowed on declaration with `default` [ModifierNotAllowedWith] +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:15: error: `virtual` not allowed on declaration with `default` [ModifierNotAllowedWith] // CHECK:STDERR: default final virtual fn ModifiersConflict2() {} // CHECK:STDERR: ^~~~~~~ -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:1: note: `default` previously appeared here [ModifierPrevious] +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+8]]:1: note: `default` previously appeared here [ModifierPrevious] +// CHECK:STDERR: default final virtual fn ModifiersConflict2() {} +// CHECK:STDERR: ^~~~~~~ +// CHECK:STDERR: +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:1: error: `default` not allowed; requires interface scope [ModifierRequiresInterface] // CHECK:STDERR: default final virtual fn ModifiersConflict2() {} // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: diff --git a/toolchain/check/testdata/function/declaration/no_prelude/fail_todo_no_params.carbon b/toolchain/check/testdata/function/declaration/no_prelude/fail_todo_no_params.carbon index 6dc933b664e0..b9908112d0d2 100644 --- a/toolchain/check/testdata/function/declaration/no_prelude/fail_todo_no_params.carbon +++ b/toolchain/check/testdata/function/declaration/no_prelude/fail_todo_no_params.carbon @@ -31,14 +31,14 @@ fn A { // TODO: We don't have parsing support for this yet. library "[[@TEST_NAME]]"; -// CHECK:STDERR: fail_todo_arrow_body.carbon:[[@LINE+8]]:1: error: semantics TODO: `handle invalid parse trees in `check`` [SemanticsTodo] -// CHECK:STDERR: fn A => 0; -// CHECK:STDERR: ^~~~~~~~~~ -// CHECK:STDERR: -// CHECK:STDERR: fail_todo_arrow_body.carbon:[[@LINE+4]]:6: error: `fn` declarations must either end with a `;` or have a `{ ... }` block for a definition [ExpectedDeclSemiOrDefinition] +// CHECK:STDERR: fail_todo_arrow_body.carbon:[[@LINE+8]]:6: error: `fn` declarations must either end with a `;` or have a `{ ... }` block for a definition [ExpectedDeclSemiOrDefinition] // CHECK:STDERR: fn A => 0; // CHECK:STDERR: ^~ // CHECK:STDERR: +// CHECK:STDERR: fail_todo_arrow_body.carbon:[[@LINE+4]]:1: error: semantics TODO: `handle invalid parse trees in `check`` [SemanticsTodo] +// CHECK:STDERR: fn A => 0; +// CHECK:STDERR: ^~~~~~~~~~ +// CHECK:STDERR: fn A => 0; // --- fail_invalid_file_generic_regression_test.carbon diff --git a/toolchain/check/testdata/if_expr/fail_not_in_function.carbon b/toolchain/check/testdata/if_expr/fail_not_in_function.carbon index 2de3a33260bf..e854acfd161a 100644 --- a/toolchain/check/testdata/if_expr/fail_not_in_function.carbon +++ b/toolchain/check/testdata/if_expr/fail_not_in_function.carbon @@ -12,17 +12,17 @@ // CHECK:STDERR: let x: i32 = if true then 1 else 0; // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: -// CHECK:STDERR: fail_not_in_function.carbon:[[@LINE+12]]:14: error: semantics TODO: `Control flow expressions are currently only supported inside functions.` [SemanticsTodo] +// CHECK:STDERR: fail_not_in_function.carbon:[[@LINE+12]]:22: error: semantics TODO: `Control flow expressions are currently only supported inside functions.` [SemanticsTodo] // CHECK:STDERR: let x: i32 = if true then 1 else 0; -// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~ +// CHECK:STDERR: ^~~~~~ // CHECK:STDERR: // CHECK:STDERR: fail_not_in_function.carbon:[[@LINE+8]]:14: error: semantics TODO: `Control flow expressions are currently only supported inside functions.` [SemanticsTodo] // CHECK:STDERR: let x: i32 = if true then 1 else 0; -// CHECK:STDERR: ^~~~~~~ +// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~ // CHECK:STDERR: -// CHECK:STDERR: fail_not_in_function.carbon:[[@LINE+4]]:22: error: semantics TODO: `Control flow expressions are currently only supported inside functions.` [SemanticsTodo] +// CHECK:STDERR: fail_not_in_function.carbon:[[@LINE+4]]:14: error: semantics TODO: `Control flow expressions are currently only supported inside functions.` [SemanticsTodo] // CHECK:STDERR: let x: i32 = if true then 1 else 0; -// CHECK:STDERR: ^~~~~~ +// CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: let x: i32 = if true then 1 else 0; diff --git a/toolchain/check/testdata/impl/fail_extend_impl_type_as.carbon b/toolchain/check/testdata/impl/fail_extend_impl_type_as.carbon index 8a9a73108ae4..c2b7e130107c 100644 --- a/toolchain/check/testdata/impl/fail_extend_impl_type_as.carbon +++ b/toolchain/check/testdata/impl/fail_extend_impl_type_as.carbon @@ -19,27 +19,27 @@ class C { } class D { - // CHECK:STDERR: fail_extend_impl_type_as.carbon:[[@LINE+11]]:3: error: cannot `extend` an `impl` with an explicit self type [ExtendImplSelfAs] + // CHECK:STDERR: fail_extend_impl_type_as.carbon:[[@LINE+7]]:3: error: cannot `extend` an `impl` with an explicit self type [ExtendImplSelfAs] // CHECK:STDERR: extend impl D as I; // CHECK:STDERR: ^~~~~~ - // CHECK:STDERR: fail_extend_impl_type_as.carbon:[[@LINE+8]]:15: note: remove the explicit `Self` type here [ExtendImplSelfAsDefault] + // CHECK:STDERR: fail_extend_impl_type_as.carbon:[[@LINE+4]]:15: note: remove the explicit `Self` type here [ExtendImplSelfAsDefault] // CHECK:STDERR: extend impl D as I; // CHECK:STDERR: ^ // CHECK:STDERR: - // CHECK:STDERR: fail_extend_impl_type_as.carbon:[[@LINE+4]]:3: error: impl declared but not defined [MissingImplDefinition] - // CHECK:STDERR: extend impl D as I; - // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~ - // CHECK:STDERR: extend impl D as I; } class E { - // CHECK:STDERR: fail_extend_impl_type_as.carbon:[[@LINE+6]]:3: error: cannot `extend` an `impl` with an explicit self type [ExtendImplSelfAs] + // CHECK:STDERR: fail_extend_impl_type_as.carbon:[[@LINE+10]]:3: error: cannot `extend` an `impl` with an explicit self type [ExtendImplSelfAs] // CHECK:STDERR: extend impl Self as I {} // CHECK:STDERR: ^~~~~~ - // CHECK:STDERR: fail_extend_impl_type_as.carbon:[[@LINE+3]]:15: note: remove the explicit `Self` type here [ExtendImplSelfAsDefault] + // CHECK:STDERR: fail_extend_impl_type_as.carbon:[[@LINE+7]]:15: note: remove the explicit `Self` type here [ExtendImplSelfAsDefault] // CHECK:STDERR: extend impl Self as I {} // CHECK:STDERR: ^~~~ + // CHECK:STDERR: + // CHECK:STDERR: fail_extend_impl_type_as.carbon:[[@LINE-11]]:3: error: impl declared but not defined [MissingImplDefinition] + // CHECK:STDERR: extend impl D as I; + // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~ extend impl Self as I {} } diff --git a/toolchain/check/testdata/impl/fail_extend_undefined_interface.carbon b/toolchain/check/testdata/impl/fail_extend_undefined_interface.carbon index f54451d025a4..babf9ea582fb 100644 --- a/toolchain/check/testdata/impl/fail_extend_undefined_interface.carbon +++ b/toolchain/check/testdata/impl/fail_extend_undefined_interface.carbon @@ -11,17 +11,13 @@ interface I; class C { - // CHECK:STDERR: fail_extend_undefined_interface.carbon:[[@LINE+11]]:3: error: implementation of undefined interface I [ImplOfUndefinedInterface] + // CHECK:STDERR: fail_extend_undefined_interface.carbon:[[@LINE+7]]:3: error: implementation of undefined interface I [ImplOfUndefinedInterface] // CHECK:STDERR: extend impl as I; // CHECK:STDERR: ^~~~~~~~~~~~~~~~~ // CHECK:STDERR: fail_extend_undefined_interface.carbon:[[@LINE-6]]:1: note: interface was forward declared here [InterfaceForwardDeclaredHere] // CHECK:STDERR: interface I; // CHECK:STDERR: ^~~~~~~~~~~~ // CHECK:STDERR: - // CHECK:STDERR: fail_extend_undefined_interface.carbon:[[@LINE+4]]:3: error: impl declared but not defined [MissingImplDefinition] - // CHECK:STDERR: extend impl as I; - // CHECK:STDERR: ^~~~~~~~~~~~~~~~~ - // CHECK:STDERR: extend impl as I; } @@ -29,7 +25,7 @@ fn F(c: C) { // CHECK:STDERR: fail_extend_undefined_interface.carbon:[[@LINE+10]]:3: error: member access into undefined interface `I` [QualifiedExprInUndefinedInterfaceScope] // CHECK:STDERR: C.F(); // CHECK:STDERR: ^~~ - // CHECK:STDERR: fail_extend_undefined_interface.carbon:[[@LINE-21]]:1: note: interface was forward declared here [InterfaceForwardDeclaredHere] + // CHECK:STDERR: fail_extend_undefined_interface.carbon:[[@LINE-17]]:1: note: interface was forward declared here [InterfaceForwardDeclaredHere] // CHECK:STDERR: interface I; // CHECK:STDERR: ^~~~~~~~~~~~ // CHECK:STDERR: fail_extend_undefined_interface.carbon:[[@LINE-10]]:18: note: declared as an extended scope here [FromExtendHere] @@ -37,15 +33,19 @@ fn F(c: C) { // CHECK:STDERR: ^ // CHECK:STDERR: C.F(); - // CHECK:STDERR: fail_extend_undefined_interface.carbon:[[@LINE+9]]:3: error: member access into undefined interface `I` [QualifiedExprInUndefinedInterfaceScope] + // CHECK:STDERR: fail_extend_undefined_interface.carbon:[[@LINE+13]]:3: error: member access into undefined interface `I` [QualifiedExprInUndefinedInterfaceScope] // CHECK:STDERR: c.F(); // CHECK:STDERR: ^~~ - // CHECK:STDERR: fail_extend_undefined_interface.carbon:[[@LINE-32]]:1: note: interface was forward declared here [InterfaceForwardDeclaredHere] + // CHECK:STDERR: fail_extend_undefined_interface.carbon:[[@LINE-28]]:1: note: interface was forward declared here [InterfaceForwardDeclaredHere] // CHECK:STDERR: interface I; // CHECK:STDERR: ^~~~~~~~~~~~ // CHECK:STDERR: fail_extend_undefined_interface.carbon:[[@LINE-21]]:18: note: declared as an extended scope here [FromExtendHere] // CHECK:STDERR: extend impl as I; // CHECK:STDERR: ^ + // CHECK:STDERR: + // CHECK:STDERR: fail_extend_undefined_interface.carbon:[[@LINE-25]]:3: error: impl declared but not defined [MissingImplDefinition] + // CHECK:STDERR: extend impl as I; + // CHECK:STDERR: ^~~~~~~~~~~~~~~~~ c.F(); } @@ -82,7 +82,7 @@ fn F(c: C) { // CHECK:STDOUT: %c.param_patt: %C = value_param_pattern %c.patt, runtime_param0 // CHECK:STDOUT: } { // CHECK:STDOUT: %c.param: %C = value_param runtime_param0 -// CHECK:STDOUT: %C.ref.loc28: type = name_ref C, file.%C.decl [template = constants.%C] +// CHECK:STDOUT: %C.ref.loc24: type = name_ref C, file.%C.decl [template = constants.%C] // CHECK:STDOUT: %c: %C = bind_name c, %c.param // CHECK:STDOUT: } // CHECK:STDOUT: } @@ -107,8 +107,8 @@ fn F(c: C) { // CHECK:STDOUT: // CHECK:STDOUT: fn @F(%c.param_patt: %C) { // CHECK:STDOUT: !entry: -// CHECK:STDOUT: %C.ref.loc39: type = name_ref C, file.%C.decl [template = constants.%C] -// CHECK:STDOUT: %F.ref.loc39: = name_ref F, [template = ] +// CHECK:STDOUT: %C.ref.loc35: type = name_ref C, file.%C.decl [template = constants.%C] +// CHECK:STDOUT: %F.ref.loc35: = name_ref F, [template = ] // CHECK:STDOUT: %c.ref: %C = name_ref c, %c // CHECK:STDOUT: %F.ref.loc49: = name_ref F, [template = ] // CHECK:STDOUT: return diff --git a/toolchain/check/testdata/impl/lookup/fail_todo_undefined_impl.carbon b/toolchain/check/testdata/impl/lookup/fail_todo_undefined_impl.carbon index feda471e9f4c..81297fdf2977 100644 --- a/toolchain/check/testdata/impl/lookup/fail_todo_undefined_impl.carbon +++ b/toolchain/check/testdata/impl/lookup/fail_todo_undefined_impl.carbon @@ -15,17 +15,17 @@ interface I { class C { // This doesn't match `impl C as I` below under proposal #3763. - // CHECK:STDERR: fail_todo_undefined_impl.carbon:[[@LINE+4]]:3: error: impl declared but not defined [MissingImplDefinition] - // CHECK:STDERR: extend impl as I; - // CHECK:STDERR: ^~~~~~~~~~~~~~~~~ - // CHECK:STDERR: extend impl as I; } fn F() -> i32 { - // CHECK:STDERR: fail_todo_undefined_impl.carbon:[[@LINE+3]]:10: error: accessing member from impl before the end of its definition [ImplAccessMemberBeforeComplete] + // CHECK:STDERR: fail_todo_undefined_impl.carbon:[[@LINE+7]]:10: error: accessing member from impl before the end of its definition [ImplAccessMemberBeforeComplete] // CHECK:STDERR: return C.F(); // CHECK:STDERR: ^~~ + // CHECK:STDERR: + // CHECK:STDERR: fail_todo_undefined_impl.carbon:[[@LINE-8]]:3: error: impl declared but not defined [MissingImplDefinition] + // CHECK:STDERR: extend impl as I; + // CHECK:STDERR: ^~~~~~~~~~~~~~~~~ return C.F(); } diff --git a/toolchain/check/testdata/interface/no_prelude/fail_assoc_const_not_constant.carbon b/toolchain/check/testdata/interface/no_prelude/fail_assoc_const_not_constant.carbon index 1927430c5632..30f3ed7a5e2b 100644 --- a/toolchain/check/testdata/interface/no_prelude/fail_assoc_const_not_constant.carbon +++ b/toolchain/check/testdata/interface/no_prelude/fail_assoc_const_not_constant.carbon @@ -9,13 +9,13 @@ // TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/interface/no_prelude/fail_assoc_const_not_constant.carbon interface I { - // CHECK:STDERR: fail_assoc_const_not_constant.carbon:[[@LINE+7]]:7: error: pattern in associated constant declaration must be a single `:!` binding [ExpectedSymbolicBindingInAssociatedConstant] - // CHECK:STDERR: let a: i32; - // CHECK:STDERR: ^ - // CHECK:STDERR: - // CHECK:STDERR: fail_assoc_const_not_constant.carbon:[[@LINE+3]]:10: error: `Core.Int` implicitly referenced here, but package `Core` not found [CoreNotFound] + // CHECK:STDERR: fail_assoc_const_not_constant.carbon:[[@LINE+7]]:10: error: `Core.Int` implicitly referenced here, but package `Core` not found [CoreNotFound] // CHECK:STDERR: let a: i32; // CHECK:STDERR: ^~~ + // CHECK:STDERR: + // CHECK:STDERR: fail_assoc_const_not_constant.carbon:[[@LINE+3]]:7: error: pattern in associated constant declaration must be a single `:!` binding [ExpectedSymbolicBindingInAssociatedConstant] + // CHECK:STDERR: let a: i32; + // CHECK:STDERR: ^ let a: i32; } diff --git a/toolchain/check/testdata/let/fail_modifiers.carbon b/toolchain/check/testdata/let/fail_modifiers.carbon index 2c4059070098..f69d22eed109 100644 --- a/toolchain/check/testdata/let/fail_modifiers.carbon +++ b/toolchain/check/testdata/let/fail_modifiers.carbon @@ -32,53 +32,53 @@ final let d: i32 = 1; // CHECK:STDERR: virtual let e: i32 = 1; -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:1: error: `default` not allowed; requires interface scope [ModifierRequiresInterface] +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:9: error: `final` not allowed on declaration with `default` [ModifierNotAllowedWith] +// CHECK:STDERR: default final let f: i32 = 1; +// CHECK:STDERR: ^~~~~ +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+8]]:1: note: `default` previously appeared here [ModifierPrevious] // CHECK:STDERR: default final let f: i32 = 1; // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+7]]:9: error: `final` not allowed on declaration with `default` [ModifierNotAllowedWith] -// CHECK:STDERR: default final let f: i32 = 1; -// CHECK:STDERR: ^~~~~ -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:1: note: `default` previously appeared here [ModifierPrevious] +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:1: error: `default` not allowed; requires interface scope [ModifierRequiresInterface] // CHECK:STDERR: default final let f: i32 = 1; // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: default final let f: i32 = 1; -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:1: error: `default` not allowed; requires interface scope [ModifierRequiresInterface] +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:9: error: `default` repeated on declaration [ModifierRepeated] +// CHECK:STDERR: default default let g: i32 = 1; +// CHECK:STDERR: ^~~~~~~ +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+8]]:1: note: `default` previously appeared here [ModifierPrevious] // CHECK:STDERR: default default let g: i32 = 1; // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+7]]:9: error: `default` repeated on declaration [ModifierRepeated] -// CHECK:STDERR: default default let g: i32 = 1; -// CHECK:STDERR: ^~~~~~~ -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:1: note: `default` previously appeared here [ModifierPrevious] +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:1: error: `default` not allowed; requires interface scope [ModifierRequiresInterface] // CHECK:STDERR: default default let g: i32 = 1; // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: default default let g: i32 = 1; -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:1: error: `protected` not allowed; requires class scope [ModifierProtectedNotAllowed] +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:11: error: `private` not allowed on declaration with `protected` [ModifierNotAllowedWith] +// CHECK:STDERR: protected private let h: i32 = 1; +// CHECK:STDERR: ^~~~~~~ +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+8]]:1: note: `protected` previously appeared here [ModifierPrevious] // CHECK:STDERR: protected private let h: i32 = 1; // CHECK:STDERR: ^~~~~~~~~ // CHECK:STDERR: -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+7]]:11: error: `private` not allowed on declaration with `protected` [ModifierNotAllowedWith] -// CHECK:STDERR: protected private let h: i32 = 1; -// CHECK:STDERR: ^~~~~~~ -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:1: note: `protected` previously appeared here [ModifierPrevious] +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:1: error: `protected` not allowed; requires class scope [ModifierProtectedNotAllowed] // CHECK:STDERR: protected private let h: i32 = 1; // CHECK:STDERR: ^~~~~~~~~ // CHECK:STDERR: protected private let h: i32 = 1; -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+10]]:1: error: `protected` not allowed; requires class scope [ModifierProtectedNotAllowed] +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+10]]:11: error: `protected` repeated on declaration [ModifierRepeated] +// CHECK:STDERR: protected protected let i: i32 = 1; +// CHECK:STDERR: ^~~~~~~~~ +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+7]]:1: note: `protected` previously appeared here [ModifierPrevious] // CHECK:STDERR: protected protected let i: i32 = 1; // CHECK:STDERR: ^~~~~~~~~ // CHECK:STDERR: -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+6]]:11: error: `protected` repeated on declaration [ModifierRepeated] -// CHECK:STDERR: protected protected let i: i32 = 1; -// CHECK:STDERR: ^~~~~~~~~ -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+3]]:1: note: `protected` previously appeared here [ModifierPrevious] +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+3]]:1: error: `protected` not allowed; requires class scope [ModifierProtectedNotAllowed] // CHECK:STDERR: protected protected let i: i32 = 1; // CHECK:STDERR: ^~~~~~~~~ protected protected let i: i32 = 1; diff --git a/toolchain/check/testdata/namespace/fail_modifiers.carbon b/toolchain/check/testdata/namespace/fail_modifiers.carbon index 966f8f6a03c4..6002488c900c 100644 --- a/toolchain/check/testdata/namespace/fail_modifiers.carbon +++ b/toolchain/check/testdata/namespace/fail_modifiers.carbon @@ -8,36 +8,36 @@ // TIP: To dump output, run: // TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/namespace/fail_modifiers.carbon -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+33]]:1: error: `private` not allowed on `namespace` declaration [ModifierNotAllowedOnDeclaration] +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+33]]:25: error: `base` not allowed on declaration with `abstract` [ModifierNotAllowedWith] +// CHECK:STDERR: private extern abstract base default final namespace A; +// CHECK:STDERR: ^~~~ +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+30]]:16: note: `abstract` previously appeared here [ModifierPrevious] +// CHECK:STDERR: private extern abstract base default final namespace A; +// CHECK:STDERR: ^~~~~~~~ +// CHECK:STDERR: +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+26]]:30: error: `default` not allowed on declaration with `abstract` [ModifierNotAllowedWith] +// CHECK:STDERR: private extern abstract base default final namespace A; +// CHECK:STDERR: ^~~~~~~ +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+23]]:16: note: `abstract` previously appeared here [ModifierPrevious] +// CHECK:STDERR: private extern abstract base default final namespace A; +// CHECK:STDERR: ^~~~~~~~ +// CHECK:STDERR: +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+19]]:38: error: `final` not allowed on declaration with `abstract` [ModifierNotAllowedWith] +// CHECK:STDERR: private extern abstract base default final namespace A; +// CHECK:STDERR: ^~~~~ +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+16]]:16: note: `abstract` previously appeared here [ModifierPrevious] +// CHECK:STDERR: private extern abstract base default final namespace A; +// CHECK:STDERR: ^~~~~~~~ +// CHECK:STDERR: +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+12]]:1: error: `private` not allowed on `namespace` declaration [ModifierNotAllowedOnDeclaration] // CHECK:STDERR: private extern abstract base default final namespace A; // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+29]]:9: error: `extern` not allowed on `namespace` declaration [ModifierNotAllowedOnDeclaration] +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+8]]:9: error: `extern` not allowed on `namespace` declaration [ModifierNotAllowedOnDeclaration] // CHECK:STDERR: private extern abstract base default final namespace A; // CHECK:STDERR: ^~~~~~ // CHECK:STDERR: -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+25]]:16: error: `abstract` not allowed on `namespace` declaration [ModifierNotAllowedOnDeclaration] -// CHECK:STDERR: private extern abstract base default final namespace A; -// CHECK:STDERR: ^~~~~~~~ -// CHECK:STDERR: -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+21]]:25: error: `base` not allowed on declaration with `abstract` [ModifierNotAllowedWith] -// CHECK:STDERR: private extern abstract base default final namespace A; -// CHECK:STDERR: ^~~~ -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+18]]:16: note: `abstract` previously appeared here [ModifierPrevious] -// CHECK:STDERR: private extern abstract base default final namespace A; -// CHECK:STDERR: ^~~~~~~~ -// CHECK:STDERR: -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+14]]:30: error: `default` not allowed on declaration with `abstract` [ModifierNotAllowedWith] -// CHECK:STDERR: private extern abstract base default final namespace A; -// CHECK:STDERR: ^~~~~~~ -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:16: note: `abstract` previously appeared here [ModifierPrevious] -// CHECK:STDERR: private extern abstract base default final namespace A; -// CHECK:STDERR: ^~~~~~~~ -// CHECK:STDERR: -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+7]]:38: error: `final` not allowed on declaration with `abstract` [ModifierNotAllowedWith] -// CHECK:STDERR: private extern abstract base default final namespace A; -// CHECK:STDERR: ^~~~~ -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:16: note: `abstract` previously appeared here [ModifierPrevious] +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:16: error: `abstract` not allowed on `namespace` declaration [ModifierNotAllowedOnDeclaration] // CHECK:STDERR: private extern abstract base default final namespace A; // CHECK:STDERR: ^~~~~~~~ // CHECK:STDERR: diff --git a/toolchain/check/testdata/operators/builtin/fail_and_or_not_in_function.carbon b/toolchain/check/testdata/operators/builtin/fail_and_or_not_in_function.carbon index b34adde44183..ea565bc71e68 100644 --- a/toolchain/check/testdata/operators/builtin/fail_and_or_not_in_function.carbon +++ b/toolchain/check/testdata/operators/builtin/fail_and_or_not_in_function.carbon @@ -14,14 +14,14 @@ fn F(b: bool) -> type { } // TODO: Short-circuit operators should be permitted outside functions. -// CHECK:STDERR: fail_and_or_not_in_function.carbon:[[@LINE+8]]:11: error: cannot evaluate type expression [TypeExprEvaluationFailure] -// CHECK:STDERR: var and_: F(true and true); -// CHECK:STDERR: ^~~~~~~~~~~~~~~~ -// CHECK:STDERR: -// CHECK:STDERR: fail_and_or_not_in_function.carbon:[[@LINE+4]]:13: error: semantics TODO: `Control flow expressions are currently only supported inside functions.` [SemanticsTodo] +// CHECK:STDERR: fail_and_or_not_in_function.carbon:[[@LINE+8]]:13: error: semantics TODO: `Control flow expressions are currently only supported inside functions.` [SemanticsTodo] // CHECK:STDERR: var and_: F(true and true); // CHECK:STDERR: ^~~~~~~~~~~~~ // CHECK:STDERR: +// CHECK:STDERR: fail_and_or_not_in_function.carbon:[[@LINE+4]]:11: error: cannot evaluate type expression [TypeExprEvaluationFailure] +// CHECK:STDERR: var and_: F(true and true); +// CHECK:STDERR: ^~~~~~~~~~~~~~~~ +// CHECK:STDERR: var and_: F(true and true); // CHECK:STDERR: fail_and_or_not_in_function.carbon:[[@LINE+12]]:21: error: semantics TODO: `Control flow expressions are currently only supported inside functions.` [SemanticsTodo] @@ -38,14 +38,14 @@ var and_: F(true and true); // CHECK:STDERR: var and_val: bool = true and true; -// CHECK:STDERR: fail_and_or_not_in_function.carbon:[[@LINE+8]]:10: error: cannot evaluate type expression [TypeExprEvaluationFailure] -// CHECK:STDERR: var or_: F(true or true); -// CHECK:STDERR: ^~~~~~~~~~~~~~~ -// CHECK:STDERR: -// CHECK:STDERR: fail_and_or_not_in_function.carbon:[[@LINE+4]]:12: error: semantics TODO: `Control flow expressions are currently only supported inside functions.` [SemanticsTodo] +// CHECK:STDERR: fail_and_or_not_in_function.carbon:[[@LINE+8]]:12: error: semantics TODO: `Control flow expressions are currently only supported inside functions.` [SemanticsTodo] // CHECK:STDERR: var or_: F(true or true); // CHECK:STDERR: ^~~~~~~~~~~~ // CHECK:STDERR: +// CHECK:STDERR: fail_and_or_not_in_function.carbon:[[@LINE+4]]:10: error: cannot evaluate type expression [TypeExprEvaluationFailure] +// CHECK:STDERR: var or_: F(true or true); +// CHECK:STDERR: ^~~~~~~~~~~~~~~ +// CHECK:STDERR: var or_: F(true or true); // CHECK:STDERR: fail_and_or_not_in_function.carbon:[[@LINE+11]]:20: error: semantics TODO: `Control flow expressions are currently only supported inside functions.` [SemanticsTodo] diff --git a/toolchain/check/testdata/packages/no_prelude/fail_modifiers.carbon b/toolchain/check/testdata/packages/no_prelude/fail_modifiers.carbon index 7acaeb69674d..a069d2c40efa 100644 --- a/toolchain/check/testdata/packages/no_prelude/fail_modifiers.carbon +++ b/toolchain/check/testdata/packages/no_prelude/fail_modifiers.carbon @@ -50,53 +50,53 @@ export library "export_library"; // --- fail_import_modifiers.carbon -// CHECK:STDERR: fail_import_modifiers.carbon:[[@LINE+8]]:1: error: `impl` not allowed on `import` declaration [ModifierNotAllowedOnDeclaration] -// CHECK:STDERR: impl import ImplImport; -// CHECK:STDERR: ^~~~ -// CHECK:STDERR: -// CHECK:STDERR: fail_import_modifiers.carbon:[[@LINE+4]]:6: error: imported API 'ImplImport' not found [ImportNotFound] +// CHECK:STDERR: fail_import_modifiers.carbon:[[@LINE+8]]:6: error: imported API 'ImplImport' not found [ImportNotFound] // CHECK:STDERR: impl import ImplImport; // CHECK:STDERR: ^~~~~~ // CHECK:STDERR: +// CHECK:STDERR: fail_import_modifiers.carbon:[[@LINE+4]]:1: error: `impl` not allowed on `import` declaration [ModifierNotAllowedOnDeclaration] +// CHECK:STDERR: impl import ImplImport; +// CHECK:STDERR: ^~~~ +// CHECK:STDERR: impl import ImplImport; -// CHECK:STDERR: fail_import_modifiers.carbon:[[@LINE+8]]:1: error: `extend` not allowed on `import` declaration [ModifierNotAllowedOnDeclaration] -// CHECK:STDERR: extend import ExtendImport; -// CHECK:STDERR: ^~~~~~ -// CHECK:STDERR: -// CHECK:STDERR: fail_import_modifiers.carbon:[[@LINE+4]]:8: error: imported API 'ExtendImport' not found [ImportNotFound] +// CHECK:STDERR: fail_import_modifiers.carbon:[[@LINE+8]]:8: error: imported API 'ExtendImport' not found [ImportNotFound] // CHECK:STDERR: extend import ExtendImport; // CHECK:STDERR: ^~~~~~ // CHECK:STDERR: +// CHECK:STDERR: fail_import_modifiers.carbon:[[@LINE+4]]:1: error: `extend` not allowed on `import` declaration [ModifierNotAllowedOnDeclaration] +// CHECK:STDERR: extend import ExtendImport; +// CHECK:STDERR: ^~~~~~ +// CHECK:STDERR: extend import ExtendImport; -// CHECK:STDERR: fail_import_modifiers.carbon:[[@LINE+8]]:1: error: `virtual` not allowed on `import` declaration [ModifierNotAllowedOnDeclaration] -// CHECK:STDERR: virtual import VirtualImport; -// CHECK:STDERR: ^~~~~~~ -// CHECK:STDERR: -// CHECK:STDERR: fail_import_modifiers.carbon:[[@LINE+4]]:9: error: imported API 'VirtualImport' not found [ImportNotFound] +// CHECK:STDERR: fail_import_modifiers.carbon:[[@LINE+8]]:9: error: imported API 'VirtualImport' not found [ImportNotFound] // CHECK:STDERR: virtual import VirtualImport; // CHECK:STDERR: ^~~~~~ // CHECK:STDERR: +// CHECK:STDERR: fail_import_modifiers.carbon:[[@LINE+4]]:1: error: `virtual` not allowed on `import` declaration [ModifierNotAllowedOnDeclaration] +// CHECK:STDERR: virtual import VirtualImport; +// CHECK:STDERR: ^~~~~~~ +// CHECK:STDERR: virtual import VirtualImport; -// CHECK:STDERR: fail_import_modifiers.carbon:[[@LINE+8]]:1: error: `base` not allowed on `import` declaration [ModifierNotAllowedOnDeclaration] -// CHECK:STDERR: base import BaseImport; -// CHECK:STDERR: ^~~~ -// CHECK:STDERR: -// CHECK:STDERR: fail_import_modifiers.carbon:[[@LINE+4]]:6: error: imported API 'BaseImport' not found [ImportNotFound] +// CHECK:STDERR: fail_import_modifiers.carbon:[[@LINE+8]]:6: error: imported API 'BaseImport' not found [ImportNotFound] // CHECK:STDERR: base import BaseImport; // CHECK:STDERR: ^~~~~~ // CHECK:STDERR: +// CHECK:STDERR: fail_import_modifiers.carbon:[[@LINE+4]]:1: error: `base` not allowed on `import` declaration [ModifierNotAllowedOnDeclaration] +// CHECK:STDERR: base import BaseImport; +// CHECK:STDERR: ^~~~ +// CHECK:STDERR: base import BaseImport; -// CHECK:STDERR: fail_import_modifiers.carbon:[[@LINE+7]]:1: error: `private` not allowed on `import` declaration [ModifierNotAllowedOnDeclaration] -// CHECK:STDERR: private import PrivateImport; -// CHECK:STDERR: ^~~~~~~ -// CHECK:STDERR: -// CHECK:STDERR: fail_import_modifiers.carbon:[[@LINE+3]]:9: error: imported API 'PrivateImport' not found [ImportNotFound] +// CHECK:STDERR: fail_import_modifiers.carbon:[[@LINE+7]]:9: error: imported API 'PrivateImport' not found [ImportNotFound] // CHECK:STDERR: private import PrivateImport; // CHECK:STDERR: ^~~~~~ +// CHECK:STDERR: +// CHECK:STDERR: fail_import_modifiers.carbon:[[@LINE+3]]:1: error: `private` not allowed on `import` declaration [ModifierNotAllowedOnDeclaration] +// CHECK:STDERR: private import PrivateImport; +// CHECK:STDERR: ^~~~~~~ private import PrivateImport; // CHECK:STDOUT: --- fail_export_package.carbon diff --git a/toolchain/check/testdata/pointer/fail_address_of_error.carbon b/toolchain/check/testdata/pointer/fail_address_of_error.carbon index 451fc85f87d6..a21209140f08 100644 --- a/toolchain/check/testdata/pointer/fail_address_of_error.carbon +++ b/toolchain/check/testdata/pointer/fail_address_of_error.carbon @@ -14,13 +14,13 @@ fn Test() { // CHECK:STDERR: ^~~~~~~~~~ // CHECK:STDERR: &undeclared; - // CHECK:STDERR: fail_address_of_error.carbon:[[@LINE+7]]:3: error: cannot take the address of non-reference expression [AddrOfNonRef] - // CHECK:STDERR: &(&undeclared); - // CHECK:STDERR: ^ - // CHECK:STDERR: - // CHECK:STDERR: fail_address_of_error.carbon:[[@LINE+3]]:6: error: name `undeclared` not found [NameNotFound] + // CHECK:STDERR: fail_address_of_error.carbon:[[@LINE+7]]:6: error: name `undeclared` not found [NameNotFound] // CHECK:STDERR: &(&undeclared); // CHECK:STDERR: ^~~~~~~~~~ + // CHECK:STDERR: + // CHECK:STDERR: fail_address_of_error.carbon:[[@LINE+3]]:3: error: cannot take the address of non-reference expression [AddrOfNonRef] + // CHECK:STDERR: &(&undeclared); + // CHECK:STDERR: ^ &(&undeclared); } diff --git a/toolchain/check/testdata/return/fail_let_in_type.carbon b/toolchain/check/testdata/return/fail_let_in_type.carbon index ce79241f2a0d..1a40e50f9e77 100644 --- a/toolchain/check/testdata/return/fail_let_in_type.carbon +++ b/toolchain/check/testdata/return/fail_let_in_type.carbon @@ -28,13 +28,13 @@ let y:! type = i32; fn HalfDozen() -> y { return 6; } // TODO: This should work. -// CHECK:STDERR: fail_let_in_type.carbon:[[@LINE+7]]:5: error: semantics TODO: `HandleTemplate` [SemanticsTodo] -// CHECK:STDERR: let template z:! type = i32; -// CHECK:STDERR: ^~~~~~~~~~~~~~~~~ -// CHECK:STDERR: -// CHECK:STDERR: fail_let_in_type.carbon:[[@LINE+3]]:14: error: semantics TODO: ``let` compile time binding outside function or interface` [SemanticsTodo] +// CHECK:STDERR: fail_let_in_type.carbon:[[@LINE+7]]:14: error: semantics TODO: ``let` compile time binding outside function or interface` [SemanticsTodo] // CHECK:STDERR: let template z:! type = i32; // CHECK:STDERR: ^~~~~~~~ +// CHECK:STDERR: +// CHECK:STDERR: fail_let_in_type.carbon:[[@LINE+3]]:5: error: semantics TODO: `HandleTemplate` [SemanticsTodo] +// CHECK:STDERR: let template z:! type = i32; +// CHECK:STDERR: ^~~~~~~~~~~~~~~~~ let template z:! type = i32; fn FirstPerfectNumber() -> z { return 6; } diff --git a/toolchain/check/testdata/var/fail_todo_control_flow_init.carbon b/toolchain/check/testdata/var/fail_todo_control_flow_init.carbon index a0417fc2768e..1ff104e55f72 100644 --- a/toolchain/check/testdata/var/fail_todo_control_flow_init.carbon +++ b/toolchain/check/testdata/var/fail_todo_control_flow_init.carbon @@ -12,17 +12,17 @@ // CHECK:STDERR: var x: () = if true then () else (); // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: -// CHECK:STDERR: fail_todo_control_flow_init.carbon:[[@LINE+12]]:13: error: semantics TODO: `Control flow expressions are currently only supported inside functions.` [SemanticsTodo] +// CHECK:STDERR: fail_todo_control_flow_init.carbon:[[@LINE+12]]:21: error: semantics TODO: `Control flow expressions are currently only supported inside functions.` [SemanticsTodo] // CHECK:STDERR: var x: () = if true then () else (); -// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~ +// CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: // CHECK:STDERR: fail_todo_control_flow_init.carbon:[[@LINE+8]]:13: error: semantics TODO: `Control flow expressions are currently only supported inside functions.` [SemanticsTodo] // CHECK:STDERR: var x: () = if true then () else (); -// CHECK:STDERR: ^~~~~~~ +// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~ // CHECK:STDERR: -// CHECK:STDERR: fail_todo_control_flow_init.carbon:[[@LINE+4]]:21: error: semantics TODO: `Control flow expressions are currently only supported inside functions.` [SemanticsTodo] +// CHECK:STDERR: fail_todo_control_flow_init.carbon:[[@LINE+4]]:13: error: semantics TODO: `Control flow expressions are currently only supported inside functions.` [SemanticsTodo] // CHECK:STDERR: var x: () = if true then () else (); -// CHECK:STDERR: ^~~~~~~ +// CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: var x: () = if true then () else (); @@ -30,17 +30,17 @@ var x: () = if true then () else (); // CHECK:STDERR: var x2: () = if false then () else (); // CHECK:STDERR: ^~~~~~~~ // CHECK:STDERR: -// CHECK:STDERR: fail_todo_control_flow_init.carbon:[[@LINE+12]]:14: error: semantics TODO: `Control flow expressions are currently only supported inside functions.` [SemanticsTodo] +// CHECK:STDERR: fail_todo_control_flow_init.carbon:[[@LINE+12]]:23: error: semantics TODO: `Control flow expressions are currently only supported inside functions.` [SemanticsTodo] // CHECK:STDERR: var x2: () = if false then () else (); -// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~ +// CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: // CHECK:STDERR: fail_todo_control_flow_init.carbon:[[@LINE+8]]:14: error: semantics TODO: `Control flow expressions are currently only supported inside functions.` [SemanticsTodo] // CHECK:STDERR: var x2: () = if false then () else (); -// CHECK:STDERR: ^~~~~~~~ +// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~ // CHECK:STDERR: -// CHECK:STDERR: fail_todo_control_flow_init.carbon:[[@LINE+4]]:23: error: semantics TODO: `Control flow expressions are currently only supported inside functions.` [SemanticsTodo] +// CHECK:STDERR: fail_todo_control_flow_init.carbon:[[@LINE+4]]:14: error: semantics TODO: `Control flow expressions are currently only supported inside functions.` [SemanticsTodo] // CHECK:STDERR: var x2: () = if false then () else (); -// CHECK:STDERR: ^~~~~~~ +// CHECK:STDERR: ^~~~~~~~ // CHECK:STDERR: var x2: () = if false then () else (); diff --git a/toolchain/check/testdata/var/no_prelude/fail_modifiers.carbon b/toolchain/check/testdata/var/no_prelude/fail_modifiers.carbon index 8646141c3322..510e6b0b7c7f 100644 --- a/toolchain/check/testdata/var/no_prelude/fail_modifiers.carbon +++ b/toolchain/check/testdata/var/no_prelude/fail_modifiers.carbon @@ -23,14 +23,14 @@ protected var b: (); // CHECK:STDERR: private protected var c: (); -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:1: error: `protected` not allowed; requires class scope [ModifierProtectedNotAllowed] +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:11: error: `protected` repeated on declaration [ModifierRepeated] +// CHECK:STDERR: protected protected var d: (); +// CHECK:STDERR: ^~~~~~~~~ +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+8]]:1: note: `protected` previously appeared here [ModifierPrevious] // CHECK:STDERR: protected protected var d: (); // CHECK:STDERR: ^~~~~~~~~ // CHECK:STDERR: -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+7]]:11: error: `protected` repeated on declaration [ModifierRepeated] -// CHECK:STDERR: protected protected var d: (); -// CHECK:STDERR: ^~~~~~~~~ -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:1: note: `protected` previously appeared here [ModifierPrevious] +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:1: error: `protected` not allowed; requires class scope [ModifierProtectedNotAllowed] // CHECK:STDERR: protected protected var d: (); // CHECK:STDERR: ^~~~~~~~~ // CHECK:STDERR: diff --git a/toolchain/diagnostics/diagnostic.h b/toolchain/diagnostics/diagnostic.h index 975476432420..80861df82ced 100644 --- a/toolchain/diagnostics/diagnostic.h +++ b/toolchain/diagnostics/diagnostic.h @@ -60,12 +60,16 @@ struct DiagnosticLoc { // Name of the file or buffer that this diagnostic refers to. llvm::StringRef filename; + // A reference to the line of the error. llvm::StringRef line; + // 1-based line number. -1 indicates unknown; other values are unused. int32_t line_number = -1; + // 1-based column number. -1 indicates unknown; other values are unused. int32_t column_number = -1; + // The number of characters corresponding to the location in the line, // starting at column_number. Should always be at least 1. int32_t length = 1; @@ -105,6 +109,18 @@ struct Diagnostic { // The diagnostic's level. DiagnosticLevel level; + // The byte offset of the final token which is associated with the diagnostic. + // This is used by `SortingDiagnosticConsumer`. This is separate from the + // `DiagnosticLoc` because it must refer to a position in the primary file + // being processed by a consumer, and has no use cross-file or in notes. + // + // This will usually be the start position (not end) of the last lexed token + // processed before the diagnostic; it could also be `-1` when no source code + // needs to be processed for a diagnostic, or an appropriate byte offset when + // we specifically want a different diagnostic ordering than when a diagnostic + // is issued. + int32_t last_byte_offset = -1; + // Messages related to the diagnostic. Only one should be a warning or error; // other messages provide context. llvm::SmallVector messages; diff --git a/toolchain/diagnostics/diagnostic_converter.h b/toolchain/diagnostics/diagnostic_converter.h index 9c92837ef1e2..e710bd4913c4 100644 --- a/toolchain/diagnostics/diagnostic_converter.h +++ b/toolchain/diagnostics/diagnostic_converter.h @@ -10,6 +10,15 @@ namespace Carbon { +// The result of `DiagnosticConvert::ConvertLoc`. This is non-templated to allow +// sharing across converters. +struct ConvertedDiagnosticLoc { + // Becomes DiagnosticMessage::loc. + DiagnosticLoc loc; + // Becomes Diagnostic::last_byte_offset. + int32_t last_byte_offset; +}; + // An interface that can convert some representation of a location into a // diagnostic location. template @@ -23,10 +32,11 @@ class DiagnosticConverter { virtual ~DiagnosticConverter() = default; - // Converts a LocT to a DiagnosticLoc. ConvertLoc may invoke - // context_fn to provide context messages. + // 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 - -> DiagnosticLoc = 0; + -> ConvertedDiagnosticLoc = 0; // Converts arg types as needed. Not all uses require conversion, so the // default returns the argument unchanged. diff --git a/toolchain/diagnostics/diagnostic_emitter.h b/toolchain/diagnostics/diagnostic_emitter.h index 8c5342e38758..792508e007e9 100644 --- a/toolchain/diagnostics/diagnostic_emitter.h +++ b/toolchain/diagnostics/diagnostic_emitter.h @@ -114,15 +114,17 @@ class DiagnosticEmitter { if (!emitter_) { return; } - AddMessageWithDiagnosticLoc( - emitter_->converter_->ConvertLoc( - loc, - [&](DiagnosticLoc context_loc, - const DiagnosticBase<>& context_diagnostic_base) { - AddMessageWithDiagnosticLoc(context_loc, - context_diagnostic_base, {}); - }), - diagnostic_base, args); + auto converted = emitter_->converter_->ConvertLoc( + loc, [&](DiagnosticLoc context_loc, + const DiagnosticBase<>& context_diagnostic_base) { + AddMessageWithDiagnosticLoc(context_loc, context_diagnostic_base, + {}); + }); + // Use the last byte offset from the first message. + if (diagnostic_.messages.empty()) { + diagnostic_.last_byte_offset = converted.last_byte_offset; + } + AddMessageWithDiagnosticLoc(converted.loc, diagnostic_base, args); } // Adds a message to the diagnostic, handling conversion of the arguments. A diff --git a/toolchain/diagnostics/diagnostic_emitter_test.cpp b/toolchain/diagnostics/diagnostic_emitter_test.cpp index 043d5d708aeb..f0d6c86ee6bf 100644 --- a/toolchain/diagnostics/diagnostic_emitter_test.cpp +++ b/toolchain/diagnostics/diagnostic_emitter_test.cpp @@ -19,8 +19,9 @@ using testing::ElementsAre; struct FakeDiagnosticConverter : DiagnosticConverter { auto ConvertLoc(int n, ContextFnT /*context_fn*/) const - -> DiagnosticLoc override { - return {.line_number = 1, .column_number = n}; + -> ConvertedDiagnosticLoc override { + return {.loc = {.line_number = 1, .column_number = n}, + .last_byte_offset = -1}; } }; diff --git a/toolchain/diagnostics/null_diagnostics.h b/toolchain/diagnostics/null_diagnostics.h index 4f73ec8a6f71..625883190965 100644 --- a/toolchain/diagnostics/null_diagnostics.h +++ b/toolchain/diagnostics/null_diagnostics.h @@ -14,8 +14,8 @@ inline auto NullDiagnosticConverter() -> DiagnosticConverter& { struct Converter : public DiagnosticConverter { auto ConvertLoc(LocT /*loc*/, DiagnosticConverter::ContextFnT /*context_fn*/) const - -> DiagnosticLoc override { - return {}; + -> ConvertedDiagnosticLoc override { + return {.loc = {}, .last_byte_offset = -1}; } }; static auto* converter = new Converter; diff --git a/toolchain/diagnostics/sorting_diagnostic_consumer.h b/toolchain/diagnostics/sorting_diagnostic_consumer.h index 9aa6c00c42ef..3406690deced 100644 --- a/toolchain/diagnostics/sorting_diagnostic_consumer.h +++ b/toolchain/diagnostics/sorting_diagnostic_consumer.h @@ -12,6 +12,13 @@ namespace Carbon { // Buffers incoming diagnostics for printing and sorting. +// +// Sorting is based on `last_byte_offset` without taking the filename into +// account. When processing multiple files, it's expected that separate +// consumers will be used in order to keep diagnostics distinct. Typically +// `Diagnostic::messages[0]` will always be a location in the consumer's primary +// file, but if it needs to correspond to a different file, the +// `last_byte_offset` must still indicate an offset within the primary file. class SortingDiagnosticConsumer : public DiagnosticConsumer { public: explicit SortingDiagnosticConsumer(DiagnosticConsumer& next_consumer) @@ -35,12 +42,7 @@ class SortingDiagnosticConsumer : public DiagnosticConsumer { void Flush() override { llvm::stable_sort(diagnostics_, [](const Diagnostic& lhs, const Diagnostic& rhs) { - const auto& lhs_loc = lhs.messages[0].loc; - const auto& rhs_loc = rhs.messages[0].loc; - return std::tie(lhs_loc.filename, lhs_loc.line_number, - lhs_loc.column_number) < - std::tie(rhs_loc.filename, rhs_loc.line_number, - rhs_loc.column_number); + return lhs.last_byte_offset < rhs.last_byte_offset; }); for (auto& diag : diagnostics_) { next_consumer_->HandleDiagnostic(std::move(diag)); diff --git a/toolchain/diagnostics/sorting_diagnostic_consumer_test.cpp b/toolchain/diagnostics/sorting_diagnostic_consumer_test.cpp index 46e53deb61f9..084c503b06f2 100644 --- a/toolchain/diagnostics/sorting_diagnostic_consumer_test.cpp +++ b/toolchain/diagnostics/sorting_diagnostic_consumer_test.cpp @@ -15,14 +15,15 @@ namespace Carbon { namespace { using ::Carbon::Testing::IsSingleDiagnostic; +using ::testing::_; using ::testing::InSequence; CARBON_DIAGNOSTIC(TestDiagnostic, Error, "M{0}", int); -struct FakeDiagnosticConverter : DiagnosticConverter { - auto ConvertLoc(DiagnosticLoc loc, ContextFnT /*context_fn*/) const - -> DiagnosticLoc override { - return loc; +struct FakeDiagnosticConverter : DiagnosticConverter { + auto ConvertLoc(int32_t last_byte_offset, ContextFnT /*context_fn*/) const + -> ConvertedDiagnosticLoc override { + return {.loc = {}, .last_byte_offset = last_byte_offset}; } }; @@ -30,34 +31,34 @@ TEST(SortedDiagnosticEmitterTest, SortErrors) { FakeDiagnosticConverter converter; Testing::MockDiagnosticConsumer consumer; SortingDiagnosticConsumer sorting_consumer(consumer); - DiagnosticEmitter emitter(converter, sorting_consumer); + DiagnosticEmitter emitter(converter, sorting_consumer); - emitter.Emit({"f", "line", 2, 1}, TestDiagnostic, 1); - emitter.Emit({"f", "line", 1, 1}, TestDiagnostic, 2); - emitter.Emit({"f", "line", 1, 3}, TestDiagnostic, 3); - emitter.Emit({"f", "line", 3, 4}, TestDiagnostic, 4); - emitter.Emit({"f", "line", 3, 2}, TestDiagnostic, 5); - emitter.Emit({"f", "line", 3, 2}, TestDiagnostic, 6); + emitter.Emit(1, TestDiagnostic, 1); + emitter.Emit(-1, TestDiagnostic, 2); + emitter.Emit(0, TestDiagnostic, 3); + emitter.Emit(4, TestDiagnostic, 4); + emitter.Emit(3, TestDiagnostic, 5); + emitter.Emit(3, TestDiagnostic, 6); InSequence s; EXPECT_CALL(consumer, HandleDiagnostic(IsSingleDiagnostic( DiagnosticKind::TestDiagnostic, - DiagnosticLevel::Error, 1, 1, "M2"))); + DiagnosticLevel::Error, _, _, "M2"))); EXPECT_CALL(consumer, HandleDiagnostic(IsSingleDiagnostic( DiagnosticKind::TestDiagnostic, - DiagnosticLevel::Error, 1, 3, "M3"))); + DiagnosticLevel::Error, _, _, "M3"))); EXPECT_CALL(consumer, HandleDiagnostic(IsSingleDiagnostic( DiagnosticKind::TestDiagnostic, - DiagnosticLevel::Error, 2, 1, "M1"))); + DiagnosticLevel::Error, _, _, "M1"))); EXPECT_CALL(consumer, HandleDiagnostic(IsSingleDiagnostic( DiagnosticKind::TestDiagnostic, - DiagnosticLevel::Error, 3, 2, "M5"))); + DiagnosticLevel::Error, _, _, "M5"))); EXPECT_CALL(consumer, HandleDiagnostic(IsSingleDiagnostic( DiagnosticKind::TestDiagnostic, - DiagnosticLevel::Error, 3, 2, "M6"))); + DiagnosticLevel::Error, _, _, "M6"))); EXPECT_CALL(consumer, HandleDiagnostic(IsSingleDiagnostic( DiagnosticKind::TestDiagnostic, - DiagnosticLevel::Error, 3, 4, "M4"))); + DiagnosticLevel::Error, _, _, "M4"))); sorting_consumer.Flush(); } diff --git a/toolchain/lex/test_helpers.h b/toolchain/lex/test_helpers.h index 88bfa4f9d18c..d1632b8099b6 100644 --- a/toolchain/lex/test_helpers.h +++ b/toolchain/lex/test_helpers.h @@ -24,23 +24,28 @@ class SingleTokenDiagnosticConverter : public DiagnosticConverter { explicit SingleTokenDiagnosticConverter(llvm::StringRef token) : token_(token) {} + // Implements `DiagnosticConverter::ConvertLoc`. auto ConvertLoc(const char* pos, ContextFnT /*context_fn*/) const - -> DiagnosticLoc override { + -> ConvertedDiagnosticLoc override { CARBON_CHECK(StringRefContainsPointer(token_, pos), "invalid diagnostic location"); llvm::StringRef prefix = token_.take_front(pos - token_.begin()); auto [before_last_newline, this_line] = prefix.rsplit('\n'); if (before_last_newline.size() == prefix.size()) { // On first line. - return {.line_number = 1, - .column_number = static_cast(pos - token_.begin() + 1)}; + return {.loc = {.line_number = 1, + .column_number = + static_cast(pos - token_.begin() + 1)}, + .last_byte_offset = -1}; } else { // On second or subsequent lines. Note that the line number here is 2 // more than the number of newlines because `rsplit` removed one newline // and `line_number` is 1-based. - return {.line_number = - static_cast(before_last_newline.count('\n') + 2), - .column_number = static_cast(this_line.size() + 1)}; + return { + .loc = {.line_number = + static_cast(before_last_newline.count('\n') + 2), + .column_number = static_cast(this_line.size() + 1)}, + .last_byte_offset = -1}; } } diff --git a/toolchain/lex/tokenized_buffer.cpp b/toolchain/lex/tokenized_buffer.cpp index 5601cb7d28ba..e8c7039e8861 100644 --- a/toolchain/lex/tokenized_buffer.cpp +++ b/toolchain/lex/tokenized_buffer.cpp @@ -376,28 +376,29 @@ auto TokenizedBuffer::CollectMemUsage(MemUsage& mem_usage, } auto TokenizedBuffer::SourceBufferDiagnosticConverter::ConvertLoc( - const char* loc, ContextFnT /*context_fn*/) const -> DiagnosticLoc { - CARBON_CHECK(StringRefContainsPointer(buffer_->source_->text(), loc), + const char* loc, ContextFnT /*context_fn*/) const + -> ConvertedDiagnosticLoc { + CARBON_CHECK(StringRefContainsPointer(tokens_->source_->text(), loc), "location not within buffer"); - int32_t offset = loc - buffer_->source_->text().begin(); + int32_t offset = loc - tokens_->source_->text().begin(); // Find the first line starting after the given location. const auto* next_line_it = llvm::partition_point( - buffer_->line_infos_, + tokens_->line_infos_, [offset](const LineInfo& line) { return line.start <= offset; }); // Step back one line to find the line containing the given position. - CARBON_CHECK(next_line_it != buffer_->line_infos_.begin(), + CARBON_CHECK(next_line_it != tokens_->line_infos_.begin(), "location precedes the start of the first line"); const auto* line_it = std::prev(next_line_it); - int line_number = line_it - buffer_->line_infos_.begin(); + int line_number = line_it - tokens_->line_infos_.begin(); int column_number = offset - line_it->start; // Grab the line from the buffer by slicing from this line to the next // minus the newline. When on the last line, instead use the start to the end // of the buffer. - llvm::StringRef text = buffer_->source_->text(); - llvm::StringRef line = next_line_it != buffer_->line_infos_.end() + llvm::StringRef text = tokens_->source_->text(); + llvm::StringRef line = next_line_it != tokens_->line_infos_.end() ? text.slice(line_it->start, next_line_it->start) : text.substr(line_it->start); @@ -406,28 +407,29 @@ auto TokenizedBuffer::SourceBufferDiagnosticConverter::ConvertLoc( // tail of the line such as CR+LF, etc. line.consume_back("\n"); - return {.filename = buffer_->source_->filename(), - .line = line, - .line_number = line_number + 1, - .column_number = column_number + 1}; + return {.loc = {.filename = tokens_->source_->filename(), + .line = line, + .line_number = line_number + 1, + .column_number = column_number + 1}, + .last_byte_offset = offset}; } auto TokenDiagnosticConverter::ConvertLoc(TokenIndex token, ContextFnT context_fn) const - -> DiagnosticLoc { + -> ConvertedDiagnosticLoc { // Map the token location into a position within the source buffer. - const auto& token_info = buffer_->GetTokenInfo(token); + const auto& token_info = tokens_->GetTokenInfo(token); const char* token_start = - buffer_->source_->text().begin() + token_info.byte_offset(); + tokens_->source_->text().begin() + token_info.byte_offset(); // Find the corresponding file location. // TODO: Should we somehow indicate in the diagnostic location if this token // is a recovery token that doesn't correspond to the original source? - DiagnosticLoc loc = - TokenizedBuffer::SourceBufferDiagnosticConverter(buffer_).ConvertLoc( + auto converted = + TokenizedBuffer::SourceBufferDiagnosticConverter(tokens_).ConvertLoc( token_start, context_fn); - loc.length = buffer_->GetTokenText(token).size(); - return loc; + converted.loc.length = tokens_->GetTokenText(token).size(); + return converted; } } // namespace Carbon::Lex diff --git a/toolchain/lex/tokenized_buffer.h b/toolchain/lex/tokenized_buffer.h index 0a9f5b542aae..3ba39714b02c 100644 --- a/toolchain/lex/tokenized_buffer.h +++ b/toolchain/lex/tokenized_buffer.h @@ -64,15 +64,18 @@ using TokenIterator = IndexIterator; // buffer locations. class TokenDiagnosticConverter : public DiagnosticConverter { public: - explicit TokenDiagnosticConverter(const TokenizedBuffer* buffer) - : buffer_(buffer) {} + explicit TokenDiagnosticConverter(const TokenizedBuffer* tokens) + : tokens_(tokens) {} - // Map the given token into a diagnostic location. + // Implements `DiagnosticConverter::ConvertLoc`. auto ConvertLoc(TokenIndex token, ContextFnT context_fn) const - -> DiagnosticLoc override; + -> ConvertedDiagnosticLoc override; + + protected: + auto tokens() const -> const TokenizedBuffer& { return *tokens_; } private: - const TokenizedBuffer* buffer_; + const TokenizedBuffer* tokens_; }; // A buffer of tokenized Carbon source code. @@ -161,6 +164,10 @@ class TokenizedBuffer : public Printable { // Returns the previous line handle. auto GetPrevLine(LineIndex line) const -> LineIndex; + auto GetByteOffset(TokenIndex token) const -> int32_t { + return GetTokenInfo(token).byte_offset(); + } + // Returns true if the token comes after the comment. auto IsAfterComment(TokenIndex token, CommentIndex comment_index) const -> bool; @@ -221,16 +228,15 @@ class TokenizedBuffer : public Printable { class SourceBufferDiagnosticConverter : public DiagnosticConverter { public: - explicit SourceBufferDiagnosticConverter(const TokenizedBuffer* buffer) - : buffer_(buffer) {} + explicit SourceBufferDiagnosticConverter(const TokenizedBuffer* tokens) + : tokens_(tokens) {} - // Map the given position within the source buffer into a diagnostic - // location. + // Implements `DiagnosticConverter::ConvertLoc`. auto ConvertLoc(const char* loc, ContextFnT context_fn) const - -> DiagnosticLoc override; + -> ConvertedDiagnosticLoc override; private: - const TokenizedBuffer* buffer_; + const TokenizedBuffer* tokens_; }; // Specifies minimum widths to use when printing a token's fields via diff --git a/toolchain/lower/file_context.cpp b/toolchain/lower/file_context.cpp index f6b409a5d9ff..dcc0930e41b2 100644 --- a/toolchain/lower/file_context.cpp +++ b/toolchain/lower/file_context.cpp @@ -616,13 +616,13 @@ auto FileContext::BuildGlobalVariableDecl(SemIR::VarStorage var_storage) } auto FileContext::GetLocForDI(SemIR::InstId inst_id) -> LocForDI { - auto diag_loc = converter_.ConvertLoc( + auto converted = converter_.ConvertLoc( inst_id, [&](DiagnosticLoc /*context_loc*/, const DiagnosticBase<>& /*context_diagnostic_base*/) {}); - return {.filename = diag_loc.filename, - .line_number = diag_loc.line_number == -1 ? 0 : diag_loc.line_number, - .column_number = - diag_loc.column_number == -1 ? 0 : diag_loc.column_number}; + const auto& loc = converted.loc; + return {.filename = loc.filename, + .line_number = loc.line_number == -1 ? 0 : loc.line_number, + .column_number = loc.column_number == -1 ? 0 : loc.column_number}; } } // namespace Carbon::Lower diff --git a/toolchain/parse/BUILD b/toolchain/parse/BUILD index 24ee38f00837..748f83d30e4e 100644 --- a/toolchain/parse/BUILD +++ b/toolchain/parse/BUILD @@ -68,6 +68,7 @@ cc_library( "//common:check", "//common:ostream", "//common:vlog", + "//toolchain/diagnostics:diagnostic_emitter", "//toolchain/diagnostics:format_providers", "//toolchain/lex:token_kind", "//toolchain/lex:tokenized_buffer", diff --git a/toolchain/parse/context.cpp b/toolchain/parse/context.cpp index 92fa9cea184e..a298a23e1659 100644 --- a/toolchain/parse/context.cpp +++ b/toolchain/parse/context.cpp @@ -20,12 +20,13 @@ namespace Carbon::Parse { -Context::Context(Tree& tree, Lex::TokenizedBuffer& tokens, - Lex::TokenDiagnosticEmitter& emitter, - llvm::raw_ostream* vlog_stream) - : tree_(&tree), - tokens_(&tokens), - emitter_(&emitter), +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_), vlog_stream_(vlog_stream), position_(tokens_->tokens().begin()), end_(tokens_->tokens().end()) { @@ -58,8 +59,8 @@ auto Context::ConsumeAndAddOpenParen(Lex::TokenIndex default_token, } else { CARBON_DIAGNOSTIC(ExpectedParenAfter, Error, "expected `(` after `{0}`", Lex::TokenKind); - emitter_->Emit(*position_, ExpectedParenAfter, - tokens().GetKind(default_token)); + emitter_.Emit(*position_, ExpectedParenAfter, + tokens().GetKind(default_token)); AddLeafNode(start_kind, default_token, /*has_error=*/true); return std::nullopt; } @@ -79,8 +80,8 @@ auto Context::ConsumeAndAddCloseSymbol(Lex::TokenIndex expected_open, // diagnostic. CARBON_DIAGNOSTIC(ExpectedCloseSymbol, Error, "unexpected tokens before `{0}`", Lex::TokenKind); - emitter_->Emit(*position_, ExpectedCloseSymbol, - open_token_kind.closing_symbol()); + emitter_.Emit(*position_, ExpectedCloseSymbol, + open_token_kind.closing_symbol()); SkipTo(tokens().GetMatchedClosingToken(expected_open)); AddNode(close_kind, Consume(), /*has_error=*/true); @@ -289,7 +290,7 @@ auto Context::DiagnoseOperatorFixity(OperatorFixity fixity) -> void { } else if (tokens().HasTrailingWhitespace(*position_)) { pos.value = -1; } - emitter_->Emit(*position_, BinaryOperatorRequiresWhitespace, pos); + emitter_.Emit(*position_, BinaryOperatorRequiresWhitespace, pos); } } else { bool prefix = fixity == OperatorFixity::Prefix; @@ -302,14 +303,14 @@ auto Context::DiagnoseOperatorFixity(OperatorFixity fixity) -> void { UnaryOperatorHasWhitespace, Error, "whitespace is not allowed {0:after|before} this unary operator", BoolAsSelect); - emitter_->Emit(*position_, UnaryOperatorHasWhitespace, prefix); + emitter_.Emit(*position_, UnaryOperatorHasWhitespace, prefix); } else if (IsLexicallyValidInfixOperator()) { // Pre/postfix operators must not satisfy the infix operator rules. CARBON_DIAGNOSTIC( UnaryOperatorRequiresWhitespace, Error, "whitespace is required {0:before|after} this unary operator", BoolAsSelect); - emitter_->Emit(*position_, UnaryOperatorRequiresWhitespace, prefix); + emitter_.Emit(*position_, UnaryOperatorRequiresWhitespace, prefix); } } } @@ -321,7 +322,7 @@ auto Context::ConsumeListToken(NodeKind comma_kind, Lex::TokenKind close_kind, if (!already_has_error) { CARBON_DIAGNOSTIC(UnexpectedTokenAfterListElement, Error, "expected `,` or `{0}`", Lex::TokenKind); - emitter_->Emit(*position_, UnexpectedTokenAfterListElement, close_kind); + emitter_.Emit(*position_, UnexpectedTokenAfterListElement, close_kind); ReturnErrorOnState(); } diff --git a/toolchain/parse/context.h b/toolchain/parse/context.h index 2d238b4ce2b9..51d00539bbe2 100644 --- a/toolchain/parse/context.h +++ b/toolchain/parse/context.h @@ -9,6 +9,7 @@ #include "common/check.h" #include "common/vlog.h" +#include "toolchain/diagnostics/diagnostic_converter.h" #include "toolchain/lex/token_kind.h" #include "toolchain/lex/tokenized_buffer.h" #include "toolchain/parse/node_kind.h" @@ -91,8 +92,8 @@ class Context { static_assert(sizeof(StateStackEntry) == 12, "StateStackEntry has unexpected size!"); - explicit Context(Tree& tree, Lex::TokenizedBuffer& tokens, - Lex::TokenDiagnosticEmitter& emitter, + explicit Context(Tree* tree, Lex::TokenizedBuffer* tokens, + DiagnosticConsumer* consumer, llvm::raw_ostream* vlog_stream); // Adds a node to the parse tree that has no children (a leaf). @@ -358,7 +359,9 @@ class Context { auto tokens() const -> const Lex::TokenizedBuffer& { return *tokens_; } - auto emitter() -> Lex::TokenDiagnosticEmitter& { return *emitter_; } + auto has_errors() const -> bool { return err_tracker_.seen_error(); } + + auto emitter() -> Lex::TokenDiagnosticEmitter& { return emitter_; } auto position() -> Lex::TokenIterator& { return position_; } auto position() const -> Lex::TokenIterator { return position_; } @@ -384,13 +387,38 @@ class Context { } private: + // Applies the `position_` to the `last_byte_offset` returned by `ConvertLoc`. + class TokenDiagnosticConverterForParse + : public Lex::TokenDiagnosticConverter { + public: + explicit TokenDiagnosticConverterForParse(Lex::TokenizedBuffer* tokens, + Lex::TokenIterator* position) + : Lex::TokenDiagnosticConverter(tokens), position_(position) {} + + auto ConvertLoc(Lex::TokenIndex token, ContextFnT context_fn) const + -> ConvertedDiagnosticLoc override { + auto converted = + Lex::TokenDiagnosticConverter::ConvertLoc(token, context_fn); + converted.last_byte_offset = std::max( + converted.last_byte_offset, tokens().GetByteOffset(**position_)); + return converted; + } + + private: + // 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; Tree* tree_; Lex::TokenizedBuffer* tokens_; - Lex::TokenDiagnosticEmitter* emitter_; + + TokenDiagnosticConverterForParse converter_; + ErrorTrackingDiagnosticConsumer err_tracker_; + Lex::TokenDiagnosticEmitter emitter_; // Whether to print verbose output. llvm::raw_ostream* vlog_stream_; diff --git a/toolchain/parse/parse.cpp b/toolchain/parse/parse.cpp index 8c98ed10a5b1..6f026f7ea69f 100644 --- a/toolchain/parse/parse.cpp +++ b/toolchain/parse/parse.cpp @@ -19,13 +19,9 @@ auto HandleInvalid(Context& context) -> void { auto Parse(Lex::TokenizedBuffer& tokens, DiagnosticConsumer& consumer, llvm::raw_ostream* vlog_stream) -> Tree { - Lex::TokenDiagnosticConverter converter(&tokens); - ErrorTrackingDiagnosticConsumer err_tracker(consumer); - Lex::TokenDiagnosticEmitter emitter(converter, err_tracker); - // Delegate to the parser. Tree tree(tokens); - Context context(tree, tokens, emitter, vlog_stream); + Context context(&tree, &tokens, &consumer, vlog_stream); PrettyStackTraceFunction context_dumper( [&](llvm::raw_ostream& output) { context.PrintForStackDump(output); }); @@ -48,7 +44,7 @@ auto Parse(Lex::TokenizedBuffer& tokens, DiagnosticConsumer& consumer, // Mark the tree as potentially having errors if there were errors coming in // from the tokenized buffer or we diagnosed new errors. - tree.set_has_errors(tokens.has_errors() || err_tracker.seen_error()); + tree.set_has_errors(tokens.has_errors() || context.has_errors()); if (auto verify = tree.Verify(); !verify.ok()) { // TODO: This is temporarily printing to stderr directly during development. diff --git a/toolchain/parse/tree_node_diagnostic_converter.h b/toolchain/parse/tree_node_diagnostic_converter.h index 2d1a4dd3dd90..af115399b532 100644 --- a/toolchain/parse/tree_node_diagnostic_converter.h +++ b/toolchain/parse/tree_node_diagnostic_converter.h @@ -42,13 +42,13 @@ class NodeLocConverter : public DiagnosticConverter { filename_(filename), get_tree_and_subtrees_(get_tree_and_subtrees) {} - // Map the given token into a diagnostic location. + // Implements `DiagnosticConverter::ConvertLoc`. auto ConvertLoc(NodeLoc node_loc, ContextFnT context_fn) const - -> DiagnosticLoc override { + -> ConvertedDiagnosticLoc override { // Support the invalid token as a way to emit only the filename, when there // is no line association. if (!node_loc.node_id().is_valid()) { - return {.filename = filename_}; + return {{.filename = filename_}, -1}; } const auto& tree = get_tree_and_subtrees_(); @@ -73,20 +73,21 @@ class NodeLocConverter : public DiagnosticConverter { end_token = desc_token; } } - DiagnosticLoc start_loc = - token_converter_.ConvertLoc(start_token, context_fn); + auto start_loc = token_converter_.ConvertLoc(start_token, context_fn); if (start_token == end_token) { return start_loc; } - DiagnosticLoc end_loc = token_converter_.ConvertLoc(end_token, context_fn); + auto end_loc = token_converter_.ConvertLoc(end_token, context_fn); + start_loc.last_byte_offset = end_loc.last_byte_offset; // For multiline locations we simply return the rest of the line for now // since true multiline locations are not yet supported. - if (start_loc.line_number != end_loc.line_number) { - start_loc.length = start_loc.line.size() - start_loc.column_number + 1; + if (start_loc.loc.line_number != end_loc.loc.line_number) { + start_loc.loc.length = + start_loc.loc.line.size() - start_loc.loc.column_number + 1; } else { - if (start_loc.column_number != end_loc.column_number) { - start_loc.length = - end_loc.column_number + end_loc.length - start_loc.column_number; + if (start_loc.loc.column_number != end_loc.loc.column_number) { + start_loc.loc.length = end_loc.loc.column_number + end_loc.loc.length - + start_loc.loc.column_number; } } return start_loc; diff --git a/toolchain/source/source_buffer.cpp b/toolchain/source/source_buffer.cpp index 9edfce557db3..b93af3a7dfb6 100644 --- a/toolchain/source/source_buffer.cpp +++ b/toolchain/source/source_buffer.cpp @@ -13,8 +13,8 @@ namespace Carbon { namespace { struct FilenameConverter : DiagnosticConverter { auto ConvertLoc(llvm::StringRef filename, ContextFnT /*context_fn*/) const - -> DiagnosticLoc override { - return {.filename = filename}; + -> ConvertedDiagnosticLoc override { + return {.loc = {.filename = filename}, .last_byte_offset = -1}; } }; } // namespace