diff --git a/toolchain/check/check.cpp b/toolchain/check/check.cpp index 12e6c661a9b02..fef83b9bf0cc7 100644 --- a/toolchain/check/check.cpp +++ b/toolchain/check/check.cpp @@ -861,7 +861,7 @@ static auto ProcessNodeIds(Context& context, llvm::raw_ostream* vlog_stream, // On crash, report which token we were handling. PrettyStackTraceFunction node_dumper([&](llvm::raw_ostream& output) { auto loc = converter.ConvertLoc( - node_id, [](DiagnosticLoc, const Internal::DiagnosticBase<>&) {}); + node_id, [](DiagnosticLoc, const DiagnosticBase<>&) {}); 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. diff --git a/toolchain/check/modifiers.cpp b/toolchain/check/modifiers.cpp index e53836fb310ec..d829f5f89007a 100644 --- a/toolchain/check/modifiers.cpp +++ b/toolchain/check/modifiers.cpp @@ -8,16 +8,37 @@ namespace Carbon::Check { -static auto DiagnoseNotAllowed(Context& context, Parse::NodeId modifier_node, - Lex::TokenKind decl_kind, - llvm::StringRef context_string, - SemIR::LocId context_loc_id) -> void { - CARBON_DIAGNOSTIC(ModifierNotAllowedOn, Error, - "`{0}` not allowed on `{1}` declaration{2}", Lex::TokenKind, - Lex::TokenKind, std::string); - auto diag = context.emitter().Build(modifier_node, ModifierNotAllowedOn, - context.token_kind(modifier_node), - decl_kind, context_string.str()); +// Builds the diagnostic for DiagnoseNotAllowed. +template +static auto StartDiagnoseNotAllowed( + Context& context, const DiagnosticBase& diagnostic_base, + Parse::NodeId modifier_node, Lex::TokenKind declaration_kind) + -> DiagnosticEmitter::DiagnosticBuilder { + if constexpr (sizeof...(TokenKinds) == 0) { + return context.emitter().Build(modifier_node, diagnostic_base); + } else if constexpr (sizeof...(TokenKinds) == 1) { + return context.emitter().Build(modifier_node, diagnostic_base, + context.token_kind(modifier_node)); + } else { + static_assert(sizeof...(TokenKinds) == 2); + return context.emitter().Build(modifier_node, diagnostic_base, + context.token_kind(modifier_node), + declaration_kind); + } +} + +// Diagnoses that a modifier wasn't allowed. Handles adding context when +// possible. +// +// The diagnostic can take up to two TokenKinds: the modifier kind, and the +// declaration kind. +template +static auto DiagnoseNotAllowed( + Context& context, const DiagnosticBase& diagnostic_base, + Parse::NodeId modifier_node, Lex::TokenKind decl_kind, + SemIR::LocId context_loc_id) -> void { + auto diag = StartDiagnoseNotAllowed(context, diagnostic_base, modifier_node, + decl_kind); if (context_loc_id.is_valid()) { CARBON_DIAGNOSTIC(ModifierNotInContext, Note, "containing definition here"); diag.Note(context_loc_id, ModifierNotInContext); @@ -37,10 +58,16 @@ static auto ModifierOrderAsSet(ModifierOrder order) -> KeywordModifierSet { } } -auto ForbidModifiersOnDecl(Context& context, DeclIntroducerState& introducer, - KeywordModifierSet forbidden, - llvm::StringRef context_string, - SemIR::LocId context_loc_id) -> void { +// Like `LimitModifiersOnDecl`, except says which modifiers are forbidden, and a +// `context_string` (and optional `context_loc_id`) specifying the context in +// which those modifiers are forbidden. +// +// See DiagnoseNotAllowed for details regarding diagnostic_base. +template +static auto ForbidModifiersOnDecl( + Context& context, const DiagnosticBaseT& diagnostic_base, + DeclIntroducerState& introducer, KeywordModifierSet forbidden, + SemIR::LocId context_loc_id = SemIR::LocId::Invalid) -> void { auto not_allowed = introducer.modifier_set & forbidden; if (not_allowed.empty()) { return; @@ -50,8 +77,9 @@ auto ForbidModifiersOnDecl(Context& context, DeclIntroducerState& introducer, order_index <= static_cast(ModifierOrder::Last); ++order_index) { auto order = static_cast(order_index); if (not_allowed.HasAnyOf(ModifierOrderAsSet(order))) { - DiagnoseNotAllowed(context, introducer.modifier_node_id(order), - introducer.kind, context_string, context_loc_id); + DiagnoseNotAllowed(context, diagnostic_base, + introducer.modifier_node_id(order), introducer.kind, + context_loc_id); introducer.set_modifier_node_id(order, Parse::NodeId::Invalid); } } @@ -59,19 +87,40 @@ auto ForbidModifiersOnDecl(Context& context, DeclIntroducerState& introducer, introducer.modifier_set.Remove(forbidden); } +auto LimitModifiersOnDecl(Context& context, DeclIntroducerState& introducer, + KeywordModifierSet allowed) -> void { + CARBON_DIAGNOSTIC(ModifierNotAllowedOnDeclaration, Error, + "`{0}` not allowed on `{1}` declaration", Lex::TokenKind, + Lex::TokenKind); + ForbidModifiersOnDecl(context, ModifierNotAllowedOnDeclaration, introducer, + ~allowed); +} + +auto LimitModifiersOnNotDefinition(Context& context, + DeclIntroducerState& introducer, + KeywordModifierSet allowed) -> void { + CARBON_DIAGNOSTIC( + ModifierOnlyAllowedOnDefinition, Error, + "`{0}` not allowed on `{1}` forward declaration, only definition", + Lex::TokenKind, Lex::TokenKind); + ForbidModifiersOnDecl(context, ModifierOnlyAllowedOnDefinition, introducer, + ~allowed); +} + auto CheckAccessModifiersOnDecl(Context& context, DeclIntroducerState& introducer, std::optional parent_scope_inst) -> void { + CARBON_DIAGNOSTIC(ModifierProtectedNotAllowed, Error, + "`protected` not allowed; requires class scope"); if (parent_scope_inst) { if (parent_scope_inst->Is()) { // TODO: This assumes that namespaces can only be declared at file scope. // If we add support for non-file-scope namespaces, we will need to check // the parents of the target scope to determine whether we're at file // scope. - ForbidModifiersOnDecl( - context, introducer, KeywordModifierSet::Protected, - " at file scope, `protected` is only allowed on class members"); + ForbidModifiersOnDecl(context, ModifierProtectedNotAllowed, introducer, + KeywordModifierSet::Protected); return; } @@ -82,11 +131,13 @@ auto CheckAccessModifiersOnDecl(Context& context, } // Otherwise neither `private` nor `protected` allowed. - ForbidModifiersOnDecl(context, introducer, KeywordModifierSet::Protected, - ", `protected` is only allowed on class members"); - ForbidModifiersOnDecl( - context, introducer, KeywordModifierSet::Private, - ", `private` is only allowed on class members and at file scope"); + ForbidModifiersOnDecl(context, ModifierProtectedNotAllowed, introducer, + KeywordModifierSet::Protected); + + CARBON_DIAGNOSTIC(ModifierPrivateNotAllowed, Error, + "`private` not allowed; requires class or file scope"); + ForbidModifiersOnDecl(context, ModifierPrivateNotAllowed, introducer, + KeywordModifierSet::Private); } auto CheckMethodModifiersOnFunction( @@ -98,21 +149,29 @@ auto CheckMethodModifiersOnFunction( auto inheritance_kind = context.classes().Get(class_decl->class_id).inheritance_kind; if (inheritance_kind == SemIR::Class::Final) { - ForbidModifiersOnDecl(context, introducer, KeywordModifierSet::Virtual, - " in a non-abstract non-base `class` definition", + CARBON_DIAGNOSTIC( + ModifierVirtualNotAllowed, Error, + "`virtual` not allowed; requires `abstract` or `base` class scope"); + ForbidModifiersOnDecl(context, ModifierVirtualNotAllowed, introducer, + KeywordModifierSet::Virtual, context.insts().GetLocId(parent_scope_inst_id)); } if (inheritance_kind != SemIR::Class::Abstract) { - ForbidModifiersOnDecl(context, introducer, KeywordModifierSet::Abstract, - " in a non-abstract `class` definition", + CARBON_DIAGNOSTIC( + ModifierAbstractNotAllowed, Error, + "`abstract` not allowed; requires `abstract` class scope"); + ForbidModifiersOnDecl(context, ModifierAbstractNotAllowed, introducer, + KeywordModifierSet::Abstract, context.insts().GetLocId(parent_scope_inst_id)); } return; } } - ForbidModifiersOnDecl(context, introducer, KeywordModifierSet::Method, - " outside of a class"); + CARBON_DIAGNOSTIC(ModifierRequiresClass, Error, + "`{0}` not allowed; requires class scope", Lex::TokenKind); + ForbidModifiersOnDecl(context, ModifierRequiresClass, introducer, + KeywordModifierSet::Method); } auto RestrictExternModifierOnDecl(Context& context, @@ -124,8 +183,11 @@ auto RestrictExternModifierOnDecl(Context& context, } if (parent_scope_inst && !parent_scope_inst->Is()) { - ForbidModifiersOnDecl(context, introducer, KeywordModifierSet::Extern, - " that is a member"); + CARBON_DIAGNOSTIC(ModifierExternNotAllowed, Error, + "`{0}` not allowed; requires file or namespace scope", + Lex::TokenKind); + ForbidModifiersOnDecl(context, ModifierExternNotAllowed, introducer, + KeywordModifierSet::Extern); // Treat as unset. introducer.extern_library = SemIR::LibraryNameId::Invalid; return; @@ -158,8 +220,11 @@ auto RequireDefaultFinalOnlyInInterfaces( // Both `default` and `final` allowed in an interface definition. return; } - ForbidModifiersOnDecl(context, introducer, KeywordModifierSet::Interface, - " outside of an interface"); + CARBON_DIAGNOSTIC(ModifierRequiresInterface, Error, + "`{0}` not allowed; requires interface scope", + Lex::TokenKind); + ForbidModifiersOnDecl(context, ModifierRequiresInterface, introducer, + KeywordModifierSet::Interface); } } // namespace Carbon::Check diff --git a/toolchain/check/modifiers.h b/toolchain/check/modifiers.h index c9921f12a8593..fd1cda704b46b 100644 --- a/toolchain/check/modifiers.h +++ b/toolchain/check/modifiers.h @@ -28,29 +28,14 @@ auto CheckMethodModifiersOnFunction( SemIR::InstId parent_scope_inst_id, std::optional parent_scope_inst) -> void; -// Like `LimitModifiersOnDecl`, except says which modifiers are forbidden, and a -// `context_string` (and optional `context_loc_id`) specifying the context in -// which those modifiers are forbidden. -// TODO: Take another look at diagnostic phrasing for callers. -auto ForbidModifiersOnDecl(Context& context, DeclIntroducerState& introducer, - KeywordModifierSet forbidden, - llvm::StringRef context_string, - SemIR::LocId context_loc_id = SemIR::LocId::Invalid) - -> void; - // Reports a diagnostic (using `decl_kind`) if modifiers on this declaration are // not in `allowed`. Updates `introducer`. -inline auto LimitModifiersOnDecl(Context& context, - DeclIntroducerState& introducer, - KeywordModifierSet allowed) -> void { - ForbidModifiersOnDecl(context, introducer, ~allowed, ""); -} +auto LimitModifiersOnDecl(Context& context, DeclIntroducerState& introducer, + KeywordModifierSet allowed) -> void; -inline auto LimitModifiersOnNotDefinition(Context& context, - DeclIntroducerState& introducer, - KeywordModifierSet allowed) -> void { - ForbidModifiersOnDecl(context, introducer, ~allowed, ", only definition"); -} +auto LimitModifiersOnNotDefinition(Context& context, + DeclIntroducerState& introducer, + KeywordModifierSet allowed) -> void; // Restricts the `extern` modifier to only be used on namespace-scoped // declarations. Diagnoses and cleans up: diff --git a/toolchain/check/testdata/class/fail_base_bad_type.carbon b/toolchain/check/testdata/class/fail_base_bad_type.carbon index c52d45b3beab4..f5f53cd1f2b63 100644 --- a/toolchain/check/testdata/class/fail_base_bad_type.carbon +++ b/toolchain/check/testdata/class/fail_base_bad_type.carbon @@ -118,7 +118,7 @@ fn AccessMemberWithInvalidBaseStruct(p: DeriveFromStruct*) -> i32 { return (*p). // --- fail_derive_from_incomplete.carbon -// CHECK:STDERR: fail_derive_from_incomplete.carbon:[[@LINE+4]]:1: error: `base` not allowed on `class` declaration, only definition +// CHECK:STDERR: fail_derive_from_incomplete.carbon:[[@LINE+4]]:1: error: `base` not allowed on `class` forward declaration, only definition // CHECK:STDERR: base class Incomplete; // CHECK:STDERR: ^~~~ // CHECK:STDERR: diff --git a/toolchain/check/testdata/class/fail_field_modifiers.carbon b/toolchain/check/testdata/class/fail_field_modifiers.carbon index f32d6756d0951..37bbffbc063d1 100644 --- a/toolchain/check/testdata/class/fail_field_modifiers.carbon +++ b/toolchain/check/testdata/class/fail_field_modifiers.carbon @@ -22,13 +22,13 @@ class Class { // CHECK:STDERR: final var k: i32; - // CHECK:STDERR: fail_field_modifiers.carbon:[[@LINE+4]]:3: error: `default` not allowed on `let` declaration outside of an interface + // CHECK:STDERR: fail_field_modifiers.carbon:[[@LINE+4]]:3: error: `default` not allowed; requires interface scope // CHECK:STDERR: default let l: i32 = 0; // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: default let l: i32 = 0; - // CHECK:STDERR: fail_field_modifiers.carbon:[[@LINE+3]]:3: error: `final` not allowed on `let` declaration outside of an interface + // CHECK:STDERR: fail_field_modifiers.carbon:[[@LINE+3]]:3: error: `final` not allowed; requires interface scope // CHECK:STDERR: final let m: i32 = 1; // CHECK:STDERR: ^~~~~ final let m: i32 = 1; diff --git a/toolchain/check/testdata/class/fail_method_modifiers.carbon b/toolchain/check/testdata/class/fail_method_modifiers.carbon index 47a6bf5229d3b..e32da14a51a4b 100644 --- a/toolchain/check/testdata/class/fail_method_modifiers.carbon +++ b/toolchain/check/testdata/class/fail_method_modifiers.carbon @@ -10,7 +10,7 @@ class FinalClass { - // CHECK:STDERR: fail_method_modifiers.carbon:[[@LINE+7]]:3: error: `abstract` not allowed on `fn` declaration in a non-abstract `class` definition + // CHECK:STDERR: fail_method_modifiers.carbon:[[@LINE+7]]:3: error: `abstract` not allowed; requires `abstract` class scope // CHECK:STDERR: abstract fn Abstract[self: Self](); // CHECK:STDERR: ^~~~~~~~ // CHECK:STDERR: fail_method_modifiers.carbon:[[@LINE-5]]:1: note: containing definition here @@ -19,7 +19,7 @@ class FinalClass { // CHECK:STDERR: abstract fn Abstract[self: Self](); - // CHECK:STDERR: fail_method_modifiers.carbon:[[@LINE+7]]:3: error: `virtual` not allowed on `fn` declaration in a non-abstract non-base `class` definition + // CHECK:STDERR: fail_method_modifiers.carbon:[[@LINE+7]]:3: error: `virtual` not allowed; requires `abstract` or `base` class scope // CHECK:STDERR: virtual fn Virtual[self: Self](); // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: fail_method_modifiers.carbon:[[@LINE-14]]:1: note: containing definition here @@ -31,13 +31,13 @@ class FinalClass { abstract class AbstractClass { - // CHECK:STDERR: fail_method_modifiers.carbon:[[@LINE+4]]:3: error: `default` not allowed on `fn` declaration outside of an interface + // CHECK:STDERR: fail_method_modifiers.carbon:[[@LINE+4]]:3: error: `default` not allowed; requires interface scope // CHECK:STDERR: default fn Default[self: Self](); // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: default fn Default[self: Self](); - // CHECK:STDERR: fail_method_modifiers.carbon:[[@LINE+4]]:3: error: `final` not allowed on `fn` declaration outside of an interface + // CHECK:STDERR: fail_method_modifiers.carbon:[[@LINE+4]]:3: error: `final` not allowed; requires interface scope // CHECK:STDERR: final fn Final[self: Self](); // CHECK:STDERR: ^~~~~ // CHECK:STDERR: @@ -46,7 +46,7 @@ abstract class AbstractClass { base class BaseClass { - // CHECK:STDERR: fail_method_modifiers.carbon:[[@LINE+6]]:3: error: `abstract` not allowed on `fn` declaration in a non-abstract `class` definition + // CHECK:STDERR: fail_method_modifiers.carbon:[[@LINE+6]]:3: error: `abstract` not allowed; requires `abstract` class scope // CHECK:STDERR: abstract fn Abstract[self: Self](); // CHECK:STDERR: ^~~~~~~~ // CHECK:STDERR: fail_method_modifiers.carbon:[[@LINE-5]]:1: note: containing definition here diff --git a/toolchain/check/testdata/class/fail_modifiers.carbon b/toolchain/check/testdata/class/fail_modifiers.carbon index ad9b081eb7571..45a6391b74ef4 100644 --- a/toolchain/check/testdata/class/fail_modifiers.carbon +++ b/toolchain/check/testdata/class/fail_modifiers.carbon @@ -17,7 +17,7 @@ // CHECK:STDERR: private private class DuplicatePrivate; -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:1: error: `abstract` not allowed on `class` declaration, only definition +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:1: error: `abstract` not allowed on `class` forward declaration, only definition // CHECK:STDERR: abstract class AbstractDecl; // CHECK:STDERR: ^~~~~~~~ // CHECK:STDERR: @@ -32,7 +32,7 @@ abstract class AbstractDecl; // CHECK:STDERR: private protected class TwoAccess; -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:1: error: `base` not allowed on `class` declaration, only definition +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:1: error: `base` not allowed on `class` forward declaration, only definition // CHECK:STDERR: base class BaseDecl; // CHECK:STDERR: ^~~~ // CHECK:STDERR: @@ -47,7 +47,7 @@ base class BaseDecl; // CHECK:STDERR: abstract abstract class TwoAbstract { } -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+15]]:1: error: `protected` not allowed on `class` declaration at file scope, `protected` is only allowed on class members +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+15]]:1: error: `protected` not allowed; requires class scope // CHECK:STDERR: protected virtual base class Virtual {} // CHECK:STDERR: ^~~~~~~~~ // CHECK:STDERR: diff --git a/toolchain/check/testdata/class/no_prelude/extern.carbon b/toolchain/check/testdata/class/no_prelude/extern.carbon index 3d7a564d8abed..88b42705e53d2 100644 --- a/toolchain/check/testdata/class/no_prelude/extern.carbon +++ b/toolchain/check/testdata/class/no_prelude/extern.carbon @@ -93,7 +93,7 @@ class C; library "[[@TEST_NAME]]"; class C { - // CHECK:STDERR: fail_extern_member_class.carbon:[[@LINE+4]]:3: error: `extern` not allowed on `class` declaration that is a member + // CHECK:STDERR: fail_extern_member_class.carbon:[[@LINE+4]]:3: error: `extern` not allowed; requires file or namespace scope // CHECK:STDERR: extern class D; // CHECK:STDERR: ^~~~~~ // CHECK:STDERR: diff --git a/toolchain/check/testdata/function/declaration/no_prelude/extern.carbon b/toolchain/check/testdata/function/declaration/no_prelude/extern.carbon index 0533799d7f50d..1f07140e8173c 100644 --- a/toolchain/check/testdata/function/declaration/no_prelude/extern.carbon +++ b/toolchain/check/testdata/function/declaration/no_prelude/extern.carbon @@ -53,12 +53,12 @@ fn F(); library "[[@TEST_NAME]]"; class C { - // CHECK:STDERR: fail_member_extern.carbon:[[@LINE+4]]:3: error: `extern` not allowed on `fn` declaration that is a member + // CHECK:STDERR: fail_member_extern.carbon:[[@LINE+4]]:3: error: `extern` not allowed; requires file or namespace scope // CHECK:STDERR: extern fn F(); // CHECK:STDERR: ^~~~~~ // CHECK:STDERR: extern fn F(); - // CHECK:STDERR: fail_member_extern.carbon:[[@LINE+3]]:3: error: `extern` not allowed on `fn` declaration that is a member + // CHECK:STDERR: fail_member_extern.carbon:[[@LINE+3]]:3: error: `extern` not allowed; requires file or namespace scope // CHECK:STDERR: extern fn G[self: Self](); // CHECK:STDERR: ^~~~~~ extern fn G[self: Self](); 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 ef65ac604138a..5838b573680b7 100644 --- a/toolchain/check/testdata/function/declaration/no_prelude/fail_modifiers.carbon +++ b/toolchain/check/testdata/function/declaration/no_prelude/fail_modifiers.carbon @@ -8,7 +8,7 @@ // 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 on `fn` declaration outside of an interface +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:1: error: `default` not allowed; requires interface scope // CHECK:STDERR: default protected fn WrongOrder(); // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: @@ -21,7 +21,7 @@ // CHECK:STDERR: default protected fn WrongOrder(); -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:1: error: `virtual` not allowed on `fn` declaration outside of a class +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:1: error: `virtual` not allowed; requires class scope // CHECK:STDERR: virtual virtual fn DuplicateVirtual() {} // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: @@ -43,7 +43,7 @@ virtual virtual fn DuplicateVirtual() {} // CHECK:STDERR: private protected fn TwoAccess(); -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:1: error: `abstract` not allowed on `fn` declaration outside of a class +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:1: error: `abstract` not allowed; requires class scope // CHECK:STDERR: abstract virtual fn ModifiersConflict() {} // CHECK:STDERR: ^~~~~~~~ // CHECK:STDERR: @@ -62,7 +62,7 @@ abstract virtual fn ModifiersConflict() {} // CHECK:STDERR: base fn InvalidModifier(); -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+18]]:1: error: `default` not allowed on `fn` declaration outside of an interface +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+18]]:1: error: `default` not allowed; requires interface scope // CHECK:STDERR: default final virtual fn ModifiersConflict2() {} // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: diff --git a/toolchain/check/testdata/interface/no_prelude/fail_modifiers.carbon b/toolchain/check/testdata/interface/no_prelude/fail_modifiers.carbon index 938e424076e30..f1312d583dc55 100644 --- a/toolchain/check/testdata/interface/no_prelude/fail_modifiers.carbon +++ b/toolchain/check/testdata/interface/no_prelude/fail_modifiers.carbon @@ -28,7 +28,7 @@ default interface Default; virtual interface Virtual { } -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+3]]:1: error: `protected` not allowed on `interface` declaration at file scope, `protected` is only allowed on class members +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+3]]:1: error: `protected` not allowed; requires class scope // CHECK:STDERR: protected interface Protected; // CHECK:STDERR: ^~~~~~~~~ protected interface Protected; diff --git a/toolchain/check/testdata/let/fail_modifiers.carbon b/toolchain/check/testdata/let/fail_modifiers.carbon index 255e08c415cc3..b863d8a358a26 100644 --- a/toolchain/check/testdata/let/fail_modifiers.carbon +++ b/toolchain/check/testdata/let/fail_modifiers.carbon @@ -8,19 +8,19 @@ // TIP: To dump output, run: // TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/let/fail_modifiers.carbon -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:1: error: `protected` not allowed on `let` declaration at file scope, `protected` is only allowed on class members +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:1: error: `protected` not allowed; requires class scope // CHECK:STDERR: protected let b: i32 = 1; // CHECK:STDERR: ^~~~~~~~~ // CHECK:STDERR: protected let b: i32 = 1; -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:1: error: `default` not allowed on `let` declaration outside of an interface +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:1: error: `default` not allowed; requires interface scope // CHECK:STDERR: default let c: i32 = 1; // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: default let c: i32 = 1; -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:1: error: `final` not allowed on `let` declaration outside of an interface +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:1: error: `final` not allowed; requires interface scope // CHECK:STDERR: final let d: i32 = 1; // CHECK:STDERR: ^~~~~ // CHECK:STDERR: @@ -32,7 +32,7 @@ final let d: i32 = 1; // CHECK:STDERR: virtual let e: i32 = 1; -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:1: error: `default` not allowed on `let` declaration outside of an interface +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:1: error: `default` not allowed; requires interface scope // CHECK:STDERR: default final let f: i32 = 1; // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: @@ -45,7 +45,7 @@ virtual let e: i32 = 1; // CHECK:STDERR: default final let f: i32 = 1; -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:1: error: `default` not allowed on `let` declaration outside of an interface +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:1: error: `default` not allowed; requires interface scope // CHECK:STDERR: default default let g: i32 = 1; // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: @@ -58,7 +58,7 @@ default final let f: i32 = 1; // CHECK:STDERR: default default let g: i32 = 1; -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:1: error: `protected` not allowed on `let` declaration at file scope, `protected` is only allowed on class members +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:1: error: `protected` not allowed; requires class scope // CHECK:STDERR: protected private let h: i32 = 1; // CHECK:STDERR: ^~~~~~~~~ // CHECK:STDERR: @@ -71,7 +71,7 @@ default default let g: i32 = 1; // CHECK:STDERR: protected private let h: i32 = 1; -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+10]]:1: error: `protected` not allowed on `let` declaration at file scope, `protected` is only allowed on class members +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+10]]:1: error: `protected` not allowed; requires class scope // CHECK:STDERR: protected protected let i: i32 = 1; // CHECK:STDERR: ^~~~~~~~~ // CHECK:STDERR: diff --git a/toolchain/check/testdata/var/no_prelude/fail_modifiers.carbon b/toolchain/check/testdata/var/no_prelude/fail_modifiers.carbon index a301dd8056391..02aec97a0bafd 100644 --- a/toolchain/check/testdata/var/no_prelude/fail_modifiers.carbon +++ b/toolchain/check/testdata/var/no_prelude/fail_modifiers.carbon @@ -8,7 +8,7 @@ // TIP: To dump output, run: // TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/var/no_prelude/fail_modifiers.carbon -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:1: error: `protected` not allowed on `var` declaration at file scope, `protected` is only allowed on class members +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:1: error: `protected` not allowed; requires class scope // CHECK:STDERR: protected var b: (); // CHECK:STDERR: ^~~~~~~~~ // CHECK:STDERR: @@ -23,7 +23,7 @@ protected var b: (); // CHECK:STDERR: private protected var c: (); -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:1: error: `protected` not allowed on `var` declaration at file scope, `protected` is only allowed on class members +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+11]]:1: error: `protected` not allowed; requires class scope // CHECK:STDERR: protected protected var d: (); // CHECK:STDERR: ^~~~~~~~~ // CHECK:STDERR: diff --git a/toolchain/diagnostics/diagnostic.h b/toolchain/diagnostics/diagnostic.h index 2d312c10351fb..dbbc51cceaeae 100644 --- a/toolchain/diagnostics/diagnostic.h +++ b/toolchain/diagnostics/diagnostic.h @@ -42,7 +42,7 @@ enum class DiagnosticLevel : int8_t { // See `DiagnosticEmitter::Emit` for comments about argument lifetimes. #define CARBON_DIAGNOSTIC(DiagnosticName, Level, Format, ...) \ static constexpr auto DiagnosticName = \ - ::Carbon::Internal::DiagnosticBase<__VA_ARGS__>( \ + ::Carbon::DiagnosticBase<__VA_ARGS__>( \ ::Carbon::DiagnosticKind::DiagnosticName, \ ::Carbon::DiagnosticLevel::Level, Format) @@ -110,8 +110,6 @@ struct Diagnostic { llvm::SmallVector messages; }; -namespace Internal { - // Use the DIAGNOSTIC macro to instantiate this. // This stores static information about a diagnostic category. template @@ -132,8 +130,6 @@ struct DiagnosticBase { llvm::StringLiteral Format; }; -} // namespace Internal - } // namespace Carbon #endif // CARBON_TOOLCHAIN_DIAGNOSTICS_DIAGNOSTIC_H_ diff --git a/toolchain/diagnostics/diagnostic_converter.h b/toolchain/diagnostics/diagnostic_converter.h index 200c0c10b194c..9c92837ef1e27 100644 --- a/toolchain/diagnostics/diagnostic_converter.h +++ b/toolchain/diagnostics/diagnostic_converter.h @@ -18,8 +18,8 @@ class DiagnosticConverter { // 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&)>; + using ContextFnT = + llvm::function_ref&)>; virtual ~DiagnosticConverter() = default; diff --git a/toolchain/diagnostics/diagnostic_emitter.h b/toolchain/diagnostics/diagnostic_emitter.h index 95e6fd69f0dff..a03da49a56427 100644 --- a/toolchain/diagnostics/diagnostic_emitter.h +++ b/toolchain/diagnostics/diagnostic_emitter.h @@ -60,8 +60,7 @@ class DiagnosticEmitter { // The API mirrors the main emission API: `DiagnosticEmitter::Emit`. // For the expected usage see the builder API: `DiagnosticEmitter::Build`. template - auto Note(LocT loc, - const Internal::DiagnosticBase& diagnostic_base, + auto Note(LocT loc, const DiagnosticBase& diagnostic_base, Internal::NoTypeDeduction... args) -> DiagnosticBuilder& { if (!emitter_) { return *this; @@ -95,10 +94,9 @@ class DiagnosticEmitter { friend class DiagnosticEmitter; template - explicit DiagnosticBuilder( - DiagnosticEmitter* emitter, LocT loc, - const Internal::DiagnosticBase& diagnostic_base, - llvm::SmallVector args) + explicit DiagnosticBuilder(DiagnosticEmitter* emitter, LocT loc, + const DiagnosticBase& diagnostic_base, + llvm::SmallVector args) : emitter_(emitter), diagnostic_({.level = diagnostic_base.Level}) { AddMessage(loc, diagnostic_base, std::move(args)); CARBON_CHECK(diagnostic_base.Level != DiagnosticLevel::Note); @@ -111,8 +109,7 @@ class DiagnosticEmitter { // Adds a message to the diagnostic, handling conversion of the location and // arguments. template - auto AddMessage(LocT loc, - const Internal::DiagnosticBase& diagnostic_base, + auto AddMessage(LocT loc, const DiagnosticBase& diagnostic_base, llvm::SmallVector args) -> void { if (!emitter_) { return; @@ -121,7 +118,7 @@ class DiagnosticEmitter { emitter_->converter_->ConvertLoc( loc, [&](DiagnosticLoc context_loc, - const Internal::DiagnosticBase<>& context_diagnostic_base) { + const DiagnosticBase<>& context_diagnostic_base) { AddMessageWithDiagnosticLoc(context_loc, context_diagnostic_base, {}); }), @@ -133,8 +130,7 @@ class DiagnosticEmitter { // avoid potential recursion. template auto AddMessageWithDiagnosticLoc( - DiagnosticLoc loc, - const Internal::DiagnosticBase& diagnostic_base, + DiagnosticLoc loc, const DiagnosticBase& diagnostic_base, llvm::SmallVector args) -> void { if (!emitter_) { return; @@ -185,7 +181,7 @@ class DiagnosticEmitter { // When passing arguments, they may be buffered. As a consequence, lifetimes // may outlive the `Emit` call. template - auto Emit(LocT loc, const Internal::DiagnosticBase& diagnostic_base, + auto Emit(LocT loc, const DiagnosticBase& diagnostic_base, Internal::NoTypeDeduction... args) -> void { DiagnosticBuilder(this, loc, diagnostic_base, {MakeAny(args)...}) .Emit(); @@ -198,7 +194,7 @@ class DiagnosticEmitter { // .Note(loc2, MyDiagnosticNote) // .Emit(); template - auto Build(LocT loc, const Internal::DiagnosticBase& diagnostic_base, + auto Build(LocT loc, const DiagnosticBase& diagnostic_base, Internal::NoTypeDeduction... args) -> DiagnosticBuilder { return DiagnosticBuilder(this, loc, diagnostic_base, {MakeAny(args)...}); diff --git a/toolchain/diagnostics/diagnostic_kind.def b/toolchain/diagnostics/diagnostic_kind.def index f8aff13a2220b..36a9594ecfbab 100644 --- a/toolchain/diagnostics/diagnostic_kind.def +++ b/toolchain/diagnostics/diagnostic_kind.def @@ -356,7 +356,15 @@ CARBON_DIAGNOSTIC_KIND(QualifiedExprNameNotFound) CARBON_DIAGNOSTIC_KIND(UseOfNonExprAsValue) // Modifier checking. -CARBON_DIAGNOSTIC_KIND(ModifierNotAllowedOn) +CARBON_DIAGNOSTIC_KIND(ModifierNotAllowedOnDeclaration) +CARBON_DIAGNOSTIC_KIND(ModifierOnlyAllowedOnDefinition) +CARBON_DIAGNOSTIC_KIND(ModifierPrivateNotAllowed) +CARBON_DIAGNOSTIC_KIND(ModifierProtectedNotAllowed) +CARBON_DIAGNOSTIC_KIND(ModifierVirtualNotAllowed) +CARBON_DIAGNOSTIC_KIND(ModifierAbstractNotAllowed) +CARBON_DIAGNOSTIC_KIND(ModifierRequiresClass) +CARBON_DIAGNOSTIC_KIND(ModifierRequiresInterface) +CARBON_DIAGNOSTIC_KIND(ModifierExternNotAllowed) CARBON_DIAGNOSTIC_KIND(ModifierNotInContext) CARBON_DIAGNOSTIC_KIND(ModifierRepeated) CARBON_DIAGNOSTIC_KIND(ModifierNotAllowedWith) diff --git a/toolchain/lower/file_context.cpp b/toolchain/lower/file_context.cpp index 0aabfb231c95f..0176cb9c8b2a4 100644 --- a/toolchain/lower/file_context.cpp +++ b/toolchain/lower/file_context.cpp @@ -568,9 +568,8 @@ auto FileContext::BuildGlobalVariableDecl(SemIR::VarStorage var_storage) auto FileContext::GetLocForDI(SemIR::InstId inst_id) -> LocForDI { auto diag_loc = converter_.ConvertLoc( - inst_id, - [&](DiagnosticLoc /*context_loc*/, - const Internal::DiagnosticBase<>& /*context_diagnostic_base*/) {}); + 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 =