From 3d113b97692909e45aeda87259c7f7494bf66d8c Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Tue, 16 Apr 2024 09:48:03 -0700 Subject: [PATCH] Refactor pre-merge redeclaration checking for sharing. (#3884) I'm poking at adding similar validation for `class` merging; this refactors support so that it can easily be shared. --- toolchain/check/function.cpp | 115 ++-------------- toolchain/check/function.h | 2 +- toolchain/check/merge.cpp | 130 +++++++++++++++++- toolchain/check/merge.h | 23 ++++ .../class/cross_package_import.carbon | 2 +- .../class/fail_method_redefinition.carbon | 2 +- .../testdata/class/fail_redefinition.carbon | 6 +- .../function/builtin/fail_redefined.carbon | 6 +- .../function/declaration/extern.carbon | 4 +- .../function/declaration/fail_redecl.carbon | 8 +- .../declaration/implicit_import.carbon | 4 +- .../function/definition/extern.carbon | 8 +- .../function/definition/fail_redef.carbon | 2 +- .../definition/implicit_import.carbon | 6 +- .../function/definition/import.carbon | 8 +- .../testdata/namespace/fail_duplicate.carbon | 2 +- .../packages/cross_package_import.carbon | 2 +- toolchain/diagnostics/diagnostic_kind.def | 18 +-- 18 files changed, 197 insertions(+), 151 deletions(-) diff --git a/toolchain/check/function.cpp b/toolchain/check/function.cpp index 5247142c6659..9398943e571e 100644 --- a/toolchain/check/function.cpp +++ b/toolchain/check/function.cpp @@ -10,8 +10,6 @@ namespace Carbon::Check { -CARBON_DIAGNOSTIC(FunctionPreviousDecl, Note, "Previously declared here."); - // Returns true if there was an error in declaring the function, which will have // previously been diagnosed. static auto FunctionDeclHasError(Context& context, const SemIR::Function& fn) @@ -195,105 +193,6 @@ auto CheckFunctionTypeMatches(Context& context, context.functions().Get(prev_function_id), substitutions); } -// Emits a redundant redeclaration diagnostic. -static auto EmitRedundantRedecl(Context& context, SemIR::LocId loc_id, - const SemIR::Function& prev_function) { - CARBON_DIAGNOSTIC(FunctionRedecl, Error, - "Redundant redeclaration of function {0}.", SemIR::NameId); - context.emitter() - .Build(loc_id, FunctionRedecl, prev_function.name_id) - .Note(prev_function.decl_id, FunctionPreviousDecl) - .Emit(); -} - -// Emits a redefinition diagnostic. -static auto EmitRedefinition(Context& context, SemIR::LocId loc_id, - const SemIR::Function& prev_function) { - CARBON_DIAGNOSTIC(FunctionRedefinition, Error, - "Redefinition of function {0}.", SemIR::NameId); - CARBON_DIAGNOSTIC(FunctionPreviousDefinition, Note, - "Previously defined here."); - context.emitter() - .Build(loc_id, FunctionRedefinition, prev_function.name_id) - .Note(prev_function.definition_id, FunctionPreviousDefinition) - .Emit(); -} - -// Checks to see if a structurally valid redeclaration is allowed in context. -// These all still merge. -static auto CheckIsAllowedRedecl(Context& context, SemIR::LocId loc_id, - const SemIR::Function& new_function, - bool new_is_definition, - const SemIR::Function& prev_function, - SemIR::ImportIRInstId prev_import_ir_inst_id) - -> void { - if (!prev_import_ir_inst_id.is_valid()) { - // Check for disallowed redeclarations in the same file. - if (!new_is_definition) { - EmitRedundantRedecl(context, loc_id, prev_function); - return; - } - if (prev_function.definition_id.is_valid()) { - EmitRedefinition(context, loc_id, prev_function); - return; - } - // `extern` definitions are prevented in handle_function.cpp; this is only - // checking for a non-`extern` definition after an `extern` declaration. - if (prev_function.is_extern) { - CARBON_DIAGNOSTIC(FunctionDefiningExtern, Error, - "Redeclaring `extern` function `{0}` as non-`extern`.", - SemIR::NameId); - CARBON_DIAGNOSTIC(FunctionPreviousExternDecl, Note, - "Previously declared `extern` here."); - context.emitter() - .Build(loc_id, FunctionDefiningExtern, prev_function.name_id) - .Note(prev_function.decl_id, FunctionPreviousExternDecl) - .Emit(); - return; - } - return; - } - - auto import_ir_id = - context.import_ir_insts().Get(prev_import_ir_inst_id).ir_id; - if (import_ir_id == SemIR::ImportIRId::ApiForImpl) { - // Check for disallowed redeclarations in the same library. Note that a - // forward declaration in the impl is allowed. - if (prev_function.definition_id.is_valid()) { - if (new_function.definition_id.is_valid()) { - EmitRedefinition(context, loc_id, prev_function); - } else { - EmitRedundantRedecl(context, loc_id, prev_function); - } - return; - } - if (prev_function.is_extern != new_function.is_extern) { - CARBON_DIAGNOSTIC( - FunctionExternMismatch, Error, - "Redeclarations in the same library must match use of `extern`."); - context.emitter() - .Build(loc_id, FunctionExternMismatch) - .Note(prev_function.decl_id, FunctionPreviousDecl) - .Emit(); - return; - } - return; - } - - // Check for disallowed redeclarations cross-library. - if (!new_function.is_extern && !prev_function.is_extern) { - CARBON_DIAGNOSTIC( - FunctionNonExternRedecl, Error, - "Only one library can declare function {0} without `extern`.", - SemIR::NameId); - context.emitter() - .Build(loc_id, FunctionNonExternRedecl, prev_function.name_id) - .Note(prev_function.decl_id, FunctionPreviousDecl) - .Emit(); - return; - } -} - // Returns the return slot usage for a function given the computed usage for two // different declarations of the function. static auto MergeReturnSlot(SemIR::Function::ReturnSlot a, @@ -316,7 +215,7 @@ static auto MergeReturnSlot(SemIR::Function::ReturnSlot a, return a; } -auto MergeFunctionRedecl(Context& context, SemIR::LocId loc_id, +auto MergeFunctionRedecl(Context& context, SemIRLoc new_loc, SemIR::Function& new_function, bool new_is_import, bool new_is_definition, SemIR::FunctionId prev_function_id, @@ -327,8 +226,16 @@ auto MergeFunctionRedecl(Context& context, SemIR::LocId loc_id, return false; } - CheckIsAllowedRedecl(context, loc_id, new_function, new_is_definition, - prev_function, prev_import_ir_inst_id); + CheckIsAllowedRedecl(context, Lex::TokenKind::Fn, prev_function.name_id, + {.loc = new_loc, + .is_definition = new_is_definition, + .is_extern = new_function.is_extern}, + {.loc = prev_function.definition_id.is_valid() + ? prev_function.definition_id + : prev_function.decl_id, + .is_definition = prev_function.definition_id.is_valid(), + .is_extern = prev_function.is_extern}, + prev_import_ir_inst_id); if (new_is_definition) { // Track the signature from the definition, so that IDs in the body diff --git a/toolchain/check/function.h b/toolchain/check/function.h index cbac2e0a5c94..9138f949d5e6 100644 --- a/toolchain/check/function.h +++ b/toolchain/check/function.h @@ -42,7 +42,7 @@ auto CheckFunctionTypeMatches(Context& context, // // If merging is successful, updates the FunctionId on new_function and returns // true. Otherwise, returns false. Prints a diagnostic when appropriate. -auto MergeFunctionRedecl(Context& context, SemIR::LocId loc_id, +auto MergeFunctionRedecl(Context& context, SemIRLoc new_loc, SemIR::Function& new_function, bool new_is_import, bool new_is_definition, SemIR::FunctionId prev_function_id, diff --git a/toolchain/check/merge.cpp b/toolchain/check/merge.cpp index a038a9c20177..7f08cc7b3027 100644 --- a/toolchain/check/merge.cpp +++ b/toolchain/check/merge.cpp @@ -12,6 +12,118 @@ namespace Carbon::Check { +CARBON_DIAGNOSTIC(RedeclPrevDecl, Note, "Previously declared here."); + +// Diagnoses a redeclaration which is redundant. +static auto DiagnoseRedundant(Context& context, Lex::TokenKind decl_kind, + SemIR::NameId name_id, SemIRLoc new_loc, + SemIRLoc prev_loc) { + CARBON_DIAGNOSTIC(RedeclRedundant, Error, + "Redeclaration of `{0} {1}` is redundant.", Lex::TokenKind, + SemIR::NameId); + context.emitter() + .Build(new_loc, RedeclRedundant, decl_kind, name_id) + .Note(prev_loc, RedeclPrevDecl) + .Emit(); +} + +// Diagnoses a redefinition. +static auto DiagnoseRedef(Context& context, Lex::TokenKind decl_kind, + SemIR::NameId name_id, SemIRLoc new_loc, + SemIRLoc prev_loc) { + CARBON_DIAGNOSTIC(RedeclRedef, Error, "Redefinition of `{0} {1}`.", + Lex::TokenKind, SemIR::NameId); + CARBON_DIAGNOSTIC(RedeclPrevDef, Note, "Previously defined here."); + context.emitter() + .Build(new_loc, RedeclRedef, decl_kind, name_id) + .Note(prev_loc, RedeclPrevDef) + .Emit(); +} + +// Diagnoses an `extern` versus non-`extern` mismatch. +static auto DiagnoseExternMismatch(Context& context, Lex::TokenKind decl_kind, + SemIR::NameId name_id, SemIRLoc new_loc, + SemIRLoc prev_loc) { + CARBON_DIAGNOSTIC(RedeclExternMismatch, Error, + "Redeclarations of `{0} {1}` in the same library must " + "match use of `extern`.", + Lex::TokenKind, SemIR::NameId); + context.emitter() + .Build(new_loc, RedeclExternMismatch, decl_kind, name_id) + .Note(prev_loc, RedeclPrevDecl) + .Emit(); +} + +// Diagnoses when multiple non-`extern` declarations are found. +static auto DiagnoseNonExtern(Context& context, Lex::TokenKind decl_kind, + SemIR::NameId name_id, SemIRLoc new_loc, + SemIRLoc prev_loc) { + CARBON_DIAGNOSTIC(RedeclNonExtern, Error, + "Only one library can declare `{0} {1}` without `extern`.", + Lex::TokenKind, SemIR::NameId); + context.emitter() + .Build(new_loc, RedeclNonExtern, decl_kind, name_id) + .Note(prev_loc, RedeclPrevDecl) + .Emit(); +} + +// Checks to see if a structurally valid redeclaration is allowed in context. +// These all still merge. +auto CheckIsAllowedRedecl(Context& context, Lex::TokenKind decl_kind, + SemIR::NameId name_id, RedeclInfo new_decl, + RedeclInfo prev_decl, + SemIR::ImportIRInstId prev_import_ir_inst_id) + -> void { + if (!prev_import_ir_inst_id.is_valid()) { + // Check for disallowed redeclarations in the same file. + if (!new_decl.is_definition) { + DiagnoseRedundant(context, decl_kind, name_id, new_decl.loc, + prev_decl.loc); + return; + } + if (prev_decl.is_definition) { + DiagnoseRedef(context, decl_kind, name_id, new_decl.loc, prev_decl.loc); + return; + } + // `extern` definitions are prevented at creation; this is only + // checking for a non-`extern` definition after an `extern` declaration. + if (prev_decl.is_extern) { + DiagnoseExternMismatch(context, decl_kind, name_id, new_decl.loc, + prev_decl.loc); + return; + } + return; + } + + auto import_ir_id = + context.import_ir_insts().Get(prev_import_ir_inst_id).ir_id; + if (import_ir_id == SemIR::ImportIRId::ApiForImpl) { + // Check for disallowed redeclarations in the same library. Note that a + // forward declaration in the impl is allowed. + if (prev_decl.is_definition) { + if (new_decl.is_definition) { + DiagnoseRedef(context, decl_kind, name_id, new_decl.loc, prev_decl.loc); + } else { + DiagnoseRedundant(context, decl_kind, name_id, new_decl.loc, + prev_decl.loc); + } + return; + } + if (prev_decl.is_extern != new_decl.is_extern) { + DiagnoseExternMismatch(context, decl_kind, name_id, new_decl.loc, + prev_decl.loc); + return; + } + return; + } + + // Check for disallowed redeclarations cross-library. + if (!new_decl.is_extern && !prev_decl.is_extern) { + DiagnoseNonExtern(context, decl_kind, name_id, new_decl.loc, prev_decl.loc); + return; + } +} + auto ResolvePrevInstForMerge(Context& context, Parse::NodeId node_id, SemIR::InstId prev_inst_id) -> InstForMerge { auto prev_inst = context.insts().Get(prev_inst_id); @@ -51,6 +163,7 @@ static auto ResolveMergeableInst(Context& context, SemIR::InstId inst_id) LoadImportRef(context, inst_id, SemIR::LocId::Invalid); break; + case SemIR::ImportRefLoaded::Kind: case SemIR::ImportRefUsed::Kind: // Already loaded. break; @@ -86,6 +199,8 @@ auto ReplacePrevInstForMerge(Context& context, SemIR::NameScopeId scope_id, } } +// TODO: On successful merges, this may need to "spoil" new_inst_id in order to +// prevent it from being emitted in lowering. auto MergeImportRef(Context& context, SemIR::InstId new_inst_id, SemIR::InstId prev_inst_id) -> void { auto new_inst = ResolveMergeableInst(context, new_inst_id); @@ -105,20 +220,21 @@ auto MergeImportRef(Context& context, SemIR::InstId new_inst_id, CARBON_KIND_SWITCH(new_inst->inst) { case CARBON_KIND(SemIR::FunctionDecl new_decl): { - auto prev_decl = prev_inst->inst.As(); + auto prev_decl = prev_inst->inst.TryAs(); + if (!prev_decl) { + break; + } auto new_fn = context.functions().Get(new_decl.function_id); - // TODO: May need to "spoil" the new function to prevent it from being - // emitted, since it will already be added. - MergeFunctionRedecl(context, context.insts().GetLocId(new_inst_id), - new_fn, + MergeFunctionRedecl(context, new_inst_id, new_fn, /*new_is_import=*/true, - /*new_is_definition=*/false, prev_decl.function_id, + /*new_is_definition=*/false, prev_decl->function_id, prev_inst->import_ir_inst_id); return; } default: - context.TODO(new_inst_id, "Merging not yet supported."); + context.TODO(new_inst_id, llvm::formatv("Merging {0} not yet supported.", + new_inst->inst.kind())); return; } } diff --git a/toolchain/check/merge.h b/toolchain/check/merge.h index 360c6b8f4ea2..de1bd769736c 100644 --- a/toolchain/check/merge.h +++ b/toolchain/check/merge.h @@ -11,6 +11,29 @@ namespace Carbon::Check { +// Information on new and previous declarations for CheckIsAllowedRedecl. +struct RedeclInfo { + // The associated diagnostic location. + SemIRLoc loc; + // True if a definition. + bool is_definition; + // True if an `extern` declaration. + bool is_extern; +}; + +// Checks if a redeclaration is allowed prior to merging. This may emit a +// diagnostic, but diagnostics do not prevent merging. +// +// The kinds of things this verifies are: +// - A declaration is not redundant. +// - A definition doesn't redefine a prior definition. +// - The use of `extern` is consistent within a library. +// - Multiple libraries do not declare non-`extern`. +auto CheckIsAllowedRedecl(Context& context, Lex::TokenKind decl_kind, + SemIR::NameId name_id, RedeclInfo new_decl, + RedeclInfo prev_decl, + SemIR::ImportIRInstId prev_import_ir_inst_id) -> void; + struct InstForMerge { // The resolved instruction. SemIR::Inst inst; diff --git a/toolchain/check/testdata/class/cross_package_import.carbon b/toolchain/check/testdata/class/cross_package_import.carbon index 482fcae694bd..a92b6047b545 100644 --- a/toolchain/check/testdata/class/cross_package_import.carbon +++ b/toolchain/check/testdata/class/cross_package_import.carbon @@ -65,7 +65,7 @@ import Other library "define"; // CHECK:STDERR: fail_merge_define_extern.carbon:[[@LINE+6]]:1: In import. // CHECK:STDERR: import Other library "extern"; // CHECK:STDERR: ^~~~~~ -// CHECK:STDERR: other_extern.carbon:5:1: ERROR: Semantics TODO: `Merging not yet supported.`. +// CHECK:STDERR: other_extern.carbon:5:1: ERROR: Semantics TODO: `Merging ClassType not yet supported.`. // CHECK:STDERR: class C; // CHECK:STDERR: ^~~~~~~~ import Other library "extern"; diff --git a/toolchain/check/testdata/class/fail_method_redefinition.carbon b/toolchain/check/testdata/class/fail_method_redefinition.carbon index ce6cc0c141ff..206fcc249e44 100644 --- a/toolchain/check/testdata/class/fail_method_redefinition.carbon +++ b/toolchain/check/testdata/class/fail_method_redefinition.carbon @@ -6,7 +6,7 @@ class Class { fn F() {} - // CHECK:STDERR: fail_method_redefinition.carbon:[[@LINE+6]]:3: ERROR: Redefinition of function F. + // CHECK:STDERR: fail_method_redefinition.carbon:[[@LINE+6]]:3: ERROR: Redefinition of `fn F`. // CHECK:STDERR: fn F() {} // CHECK:STDERR: ^~~~~~~~ // CHECK:STDERR: fail_method_redefinition.carbon:[[@LINE-4]]:3: Previously defined here. diff --git a/toolchain/check/testdata/class/fail_redefinition.carbon b/toolchain/check/testdata/class/fail_redefinition.carbon index a7655a434175..3b13c7fa7184 100644 --- a/toolchain/check/testdata/class/fail_redefinition.carbon +++ b/toolchain/check/testdata/class/fail_redefinition.carbon @@ -19,7 +19,7 @@ class Class { // CHECK:STDERR: class Class { fn G(); - // CHECK:STDERR: fail_redefinition.carbon:[[@LINE+7]]:3: ERROR: Redundant redeclaration of function H. + // CHECK:STDERR: fail_redefinition.carbon:[[@LINE+7]]:3: ERROR: Redeclaration of `fn H` is redundant. // CHECK:STDERR: fn H(); // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: fail_redefinition.carbon:[[@LINE-16]]:3: Previously declared here. @@ -27,7 +27,7 @@ class Class { // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: fn H(); - // CHECK:STDERR: fail_redefinition.carbon:[[@LINE+7]]:3: ERROR: Redefinition of function I. + // CHECK:STDERR: fail_redefinition.carbon:[[@LINE+7]]:3: ERROR: Redefinition of `fn I`. // CHECK:STDERR: fn I() {} // CHECK:STDERR: ^~~~~~~~ // CHECK:STDERR: fail_redefinition.carbon:[[@LINE-23]]:3: Previously defined here. @@ -40,7 +40,7 @@ class Class { fn Class.F() {} fn Class.G() {} fn Class.H() {} -// CHECK:STDERR: fail_redefinition.carbon:[[@LINE+6]]:1: ERROR: Redefinition of function I. +// CHECK:STDERR: fail_redefinition.carbon:[[@LINE+6]]:1: ERROR: Redefinition of `fn I`. // CHECK:STDERR: fn Class.I() {} // CHECK:STDERR: ^~~~~~~~~~~~~~ // CHECK:STDERR: fail_redefinition.carbon:[[@LINE-9]]:3: Previously defined here. diff --git a/toolchain/check/testdata/function/builtin/fail_redefined.carbon b/toolchain/check/testdata/function/builtin/fail_redefined.carbon index 9261586ae136..b1fb703b9f80 100644 --- a/toolchain/check/testdata/function/builtin/fail_redefined.carbon +++ b/toolchain/check/testdata/function/builtin/fail_redefined.carbon @@ -5,7 +5,7 @@ // AUTOUPDATE fn A(n: i32, m: i32) -> i32 = "int.add"; -// CHECK:STDERR: fail_redefined.carbon:[[@LINE+7]]:1: ERROR: Redefinition of function A. +// CHECK:STDERR: fail_redefined.carbon:[[@LINE+7]]:1: ERROR: Redefinition of `fn A`. // CHECK:STDERR: fn A(n: i32, m: i32) -> i32 { return n; } // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ // CHECK:STDERR: fail_redefined.carbon:[[@LINE-4]]:1: Previously defined here. @@ -15,7 +15,7 @@ fn A(n: i32, m: i32) -> i32 = "int.add"; fn A(n: i32, m: i32) -> i32 { return n; } fn B(n: i32, m: i32) -> i32 { return n; } -// CHECK:STDERR: fail_redefined.carbon:[[@LINE+7]]:1: ERROR: Redefinition of function B. +// CHECK:STDERR: fail_redefined.carbon:[[@LINE+7]]:1: ERROR: Redefinition of `fn B`. // CHECK:STDERR: fn B(n: i32, m: i32) -> i32 = "int.add"; // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ // CHECK:STDERR: fail_redefined.carbon:[[@LINE-4]]:1: Previously defined here. @@ -25,7 +25,7 @@ fn B(n: i32, m: i32) -> i32 { return n; } fn B(n: i32, m: i32) -> i32 = "int.add"; fn C(n: i32, m: i32) -> i32 = "int.add"; -// CHECK:STDERR: fail_redefined.carbon:[[@LINE+6]]:1: ERROR: Redefinition of function C. +// CHECK:STDERR: fail_redefined.carbon:[[@LINE+6]]:1: ERROR: Redefinition of `fn C`. // CHECK:STDERR: fn C(n: i32, m: i32) -> i32 = "int.add"; // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ // CHECK:STDERR: fail_redefined.carbon:[[@LINE-4]]:1: Previously defined here. diff --git a/toolchain/check/testdata/function/declaration/extern.carbon b/toolchain/check/testdata/function/declaration/extern.carbon index b05bfee5227f..80be43b52be8 100644 --- a/toolchain/check/testdata/function/declaration/extern.carbon +++ b/toolchain/check/testdata/function/declaration/extern.carbon @@ -15,7 +15,7 @@ extern fn F(); library "redecl" api; extern fn F(); -// CHECK:STDERR: fail_redecl.carbon:[[@LINE+7]]:1: ERROR: Redundant redeclaration of function F. +// CHECK:STDERR: fail_redecl.carbon:[[@LINE+7]]:1: ERROR: Redeclaration of `fn F` is redundant. // CHECK:STDERR: extern fn F(); // CHECK:STDERR: ^~~~~~~~~~~~~~ // CHECK:STDERR: fail_redecl.carbon:[[@LINE-4]]:1: Previously declared here. @@ -29,7 +29,7 @@ extern fn F(); library "redecl_extern" api; extern fn F(); -// CHECK:STDERR: fail_redecl_extern.carbon:[[@LINE+7]]:1: ERROR: Redundant redeclaration of function F. +// CHECK:STDERR: fail_redecl_extern.carbon:[[@LINE+7]]:1: ERROR: Redeclaration of `fn F` is redundant. // CHECK:STDERR: fn F(); // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: fail_redecl_extern.carbon:[[@LINE-4]]:1: Previously declared here. diff --git a/toolchain/check/testdata/function/declaration/fail_redecl.carbon b/toolchain/check/testdata/function/declaration/fail_redecl.carbon index 92e78ea5bf65..e23f90fa71b6 100644 --- a/toolchain/check/testdata/function/declaration/fail_redecl.carbon +++ b/toolchain/check/testdata/function/declaration/fail_redecl.carbon @@ -5,7 +5,7 @@ // AUTOUPDATE fn A(); -// CHECK:STDERR: fail_redecl.carbon:[[@LINE+7]]:1: ERROR: Redundant redeclaration of function A. +// CHECK:STDERR: fail_redecl.carbon:[[@LINE+7]]:1: ERROR: Redeclaration of `fn A` is redundant. // CHECK:STDERR: fn A(); // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: fail_redecl.carbon:[[@LINE-4]]:1: Previously declared here. @@ -15,7 +15,7 @@ fn A(); fn A(); fn B(x: i32); -// CHECK:STDERR: fail_redecl.carbon:[[@LINE+7]]:1: ERROR: Redundant redeclaration of function B. +// CHECK:STDERR: fail_redecl.carbon:[[@LINE+7]]:1: ERROR: Redeclaration of `fn B` is redundant. // CHECK:STDERR: fn B(x: i32); // CHECK:STDERR: ^~~~~~~~~~~~~ // CHECK:STDERR: fail_redecl.carbon:[[@LINE-4]]:1: Previously declared here. @@ -35,7 +35,7 @@ fn C(); fn C(x: i32); fn D() {} -// CHECK:STDERR: fail_redecl.carbon:[[@LINE+7]]:1: ERROR: Redundant redeclaration of function D. +// CHECK:STDERR: fail_redecl.carbon:[[@LINE+7]]:1: ERROR: Redeclaration of `fn D` is redundant. // CHECK:STDERR: fn D(); // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: fail_redecl.carbon:[[@LINE-4]]:1: Previously declared here. @@ -45,7 +45,7 @@ fn D() {} fn D(); fn E() {} -// CHECK:STDERR: fail_redecl.carbon:[[@LINE+6]]:1: ERROR: Redefinition of function E. +// CHECK:STDERR: fail_redecl.carbon:[[@LINE+6]]:1: ERROR: Redefinition of `fn E`. // CHECK:STDERR: fn E() {} // CHECK:STDERR: ^~~~~~~~ // CHECK:STDERR: fail_redecl.carbon:[[@LINE-4]]:1: Previously defined here. diff --git a/toolchain/check/testdata/function/declaration/implicit_import.carbon b/toolchain/check/testdata/function/declaration/implicit_import.carbon index 8152a17ae66b..134f82bd604d 100644 --- a/toolchain/check/testdata/function/declaration/implicit_import.carbon +++ b/toolchain/check/testdata/function/declaration/implicit_import.carbon @@ -26,7 +26,7 @@ extern fn A(); library "extern_api" impl; -// CHECK:STDERR: fail_extern_api.impl.carbon:[[@LINE+10]]:1: ERROR: Redeclarations in the same library must match use of `extern`. +// CHECK:STDERR: fail_extern_api.impl.carbon:[[@LINE+10]]:1: ERROR: Redeclarations of `fn A` in the same library must match use of `extern`. // CHECK:STDERR: fn A(); // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: fail_extern_api.impl.carbon:[[@LINE-5]]:1: In import. @@ -48,7 +48,7 @@ fn A(); library "extern_impl" impl; -// CHECK:STDERR: fail_extern_impl.impl.carbon:[[@LINE+9]]:1: ERROR: Redeclarations in the same library must match use of `extern`. +// CHECK:STDERR: fail_extern_impl.impl.carbon:[[@LINE+9]]:1: ERROR: Redeclarations of `fn A` in the same library must match use of `extern`. // CHECK:STDERR: extern fn A(); // CHECK:STDERR: ^~~~~~~~~~~~~~ // CHECK:STDERR: fail_extern_impl.impl.carbon:[[@LINE-5]]:1: In import. diff --git a/toolchain/check/testdata/function/definition/extern.carbon b/toolchain/check/testdata/function/definition/extern.carbon index 3e9c8ad1e265..d58c74e9fce8 100644 --- a/toolchain/check/testdata/function/definition/extern.carbon +++ b/toolchain/check/testdata/function/definition/extern.carbon @@ -19,10 +19,10 @@ extern fn F() {} library "def_for_extern_decl" api; extern fn F(); -// CHECK:STDERR: fail_def_for_extern_decl.carbon:[[@LINE+7]]:1: ERROR: Redeclaring `extern` function `F` as non-`extern`. +// CHECK:STDERR: fail_def_for_extern_decl.carbon:[[@LINE+7]]:1: ERROR: Redeclarations of `fn F` in the same library must match use of `extern`. // CHECK:STDERR: fn F() {} // CHECK:STDERR: ^~~~~~~~ -// CHECK:STDERR: fail_def_for_extern_decl.carbon:[[@LINE-4]]:1: Previously declared `extern` here. +// CHECK:STDERR: fail_def_for_extern_decl.carbon:[[@LINE-4]]:1: Previously declared here. // CHECK:STDERR: extern fn F(); // CHECK:STDERR: ^~~~~~~~~~~~~~ // CHECK:STDERR: @@ -33,7 +33,7 @@ fn F() {} library "extern_diag_suppressed" api; extern fn F(); -// CHECK:STDERR: fail_extern_diag_suppressed.carbon:[[@LINE+7]]:1: ERROR: Redundant redeclaration of function F. +// CHECK:STDERR: fail_extern_diag_suppressed.carbon:[[@LINE+7]]:1: ERROR: Redeclaration of `fn F` is redundant. // CHECK:STDERR: fn F(); // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: fail_extern_diag_suppressed.carbon:[[@LINE-4]]:1: Previously declared here. @@ -48,7 +48,7 @@ fn F() {} library "extern_decl_after_def" api; fn F() {} -// CHECK:STDERR: fail_extern_decl_after_def.carbon:[[@LINE+6]]:1: ERROR: Redundant redeclaration of function F. +// CHECK:STDERR: fail_extern_decl_after_def.carbon:[[@LINE+6]]:1: ERROR: Redeclaration of `fn F` is redundant. // CHECK:STDERR: extern fn F(); // CHECK:STDERR: ^~~~~~~~~~~~~~ // CHECK:STDERR: fail_extern_decl_after_def.carbon:[[@LINE-4]]:1: Previously declared here. diff --git a/toolchain/check/testdata/function/definition/fail_redef.carbon b/toolchain/check/testdata/function/definition/fail_redef.carbon index a3ec1c0d5ff3..8f0d40bd01c7 100644 --- a/toolchain/check/testdata/function/definition/fail_redef.carbon +++ b/toolchain/check/testdata/function/definition/fail_redef.carbon @@ -5,7 +5,7 @@ // AUTOUPDATE fn F() {} -// CHECK:STDERR: fail_redef.carbon:[[@LINE+6]]:1: ERROR: Redefinition of function F. +// CHECK:STDERR: fail_redef.carbon:[[@LINE+6]]:1: ERROR: Redefinition of `fn F`. // CHECK:STDERR: fn F() {} // CHECK:STDERR: ^~~~~~~~ // CHECK:STDERR: fail_redef.carbon:[[@LINE-4]]:1: Previously defined here. diff --git a/toolchain/check/testdata/function/definition/implicit_import.carbon b/toolchain/check/testdata/function/definition/implicit_import.carbon index c24ed6670d6f..19b04d8c3b74 100644 --- a/toolchain/check/testdata/function/definition/implicit_import.carbon +++ b/toolchain/check/testdata/function/definition/implicit_import.carbon @@ -26,7 +26,7 @@ extern fn A(); library "extern_api" impl; -// CHECK:STDERR: fail_extern_api.impl.carbon:[[@LINE+10]]:1: ERROR: Redeclarations in the same library must match use of `extern`. +// CHECK:STDERR: fail_extern_api.impl.carbon:[[@LINE+10]]:1: ERROR: Redeclarations of `fn A` in the same library must match use of `extern`. // CHECK:STDERR: fn A() {} // CHECK:STDERR: ^~~~~~~~ // CHECK:STDERR: fail_extern_api.impl.carbon:[[@LINE-5]]:1: In import. @@ -64,7 +64,7 @@ fn A() {} library "redecl_after_def" impl; -// CHECK:STDERR: fail_redecl_after_def.impl.carbon:[[@LINE+10]]:1: ERROR: Redundant redeclaration of function A. +// CHECK:STDERR: fail_redecl_after_def.impl.carbon:[[@LINE+10]]:1: ERROR: Redeclaration of `fn A` is redundant. // CHECK:STDERR: fn A(); // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: fail_redecl_after_def.impl.carbon:[[@LINE-5]]:1: In import. @@ -86,7 +86,7 @@ fn A() {} library "redef_after_def" impl; -// CHECK:STDERR: fail_redef_after_def.impl.carbon:[[@LINE+9]]:1: ERROR: Redefinition of function A. +// CHECK:STDERR: fail_redef_after_def.impl.carbon:[[@LINE+9]]:1: ERROR: Redefinition of `fn A`. // CHECK:STDERR: fn A() {} // CHECK:STDERR: ^~~~~~~~ // CHECK:STDERR: fail_redef_after_def.impl.carbon:[[@LINE-5]]:1: In import. diff --git a/toolchain/check/testdata/function/definition/import.carbon b/toolchain/check/testdata/function/definition/import.carbon index 76fbf321b9af..35e47ab8325f 100644 --- a/toolchain/check/testdata/function/definition/import.carbon +++ b/toolchain/check/testdata/function/definition/import.carbon @@ -43,7 +43,7 @@ library "def_ownership" api; import library "fns"; -// CHECK:STDERR: fail_def_ownership.carbon:[[@LINE+10]]:1: ERROR: Only one library can declare function A without `extern`. +// CHECK:STDERR: fail_def_ownership.carbon:[[@LINE+10]]:1: ERROR: Only one library can declare `fn A` without `extern`. // CHECK:STDERR: fn A() {}; // CHECK:STDERR: ^~~~~~~~ // CHECK:STDERR: fail_def_ownership.carbon:[[@LINE-5]]:1: In import. @@ -54,7 +54,7 @@ import library "fns"; // CHECK:STDERR: ^~~~~~~~ // CHECK:STDERR: fn A() {}; -// CHECK:STDERR: fail_def_ownership.carbon:[[@LINE+10]]:1: ERROR: Only one library can declare function B without `extern`. +// CHECK:STDERR: fail_def_ownership.carbon:[[@LINE+10]]:1: ERROR: Only one library can declare `fn B` without `extern`. // CHECK:STDERR: fn B(b: i32) -> i32; // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~ // CHECK:STDERR: fail_def_ownership.carbon:[[@LINE-16]]:1: In import. @@ -82,10 +82,10 @@ library "mix_extern_decl" api; import library "fns"; extern fn D(); -// CHECK:STDERR: fail_mix_extern_decl.carbon:[[@LINE+6]]:1: ERROR: Redeclaring `extern` function `D` as non-`extern`. +// CHECK:STDERR: fail_mix_extern_decl.carbon:[[@LINE+6]]:1: ERROR: Redeclarations of `fn D` in the same library must match use of `extern`. // CHECK:STDERR: fn D() {} // CHECK:STDERR: ^~~~~~~~ -// CHECK:STDERR: fail_mix_extern_decl.carbon:[[@LINE-4]]:1: Previously declared `extern` here. +// CHECK:STDERR: fail_mix_extern_decl.carbon:[[@LINE-4]]:1: Previously declared here. // CHECK:STDERR: extern fn D(); // CHECK:STDERR: ^~~~~~~~~~~~~~ fn D() {} diff --git a/toolchain/check/testdata/namespace/fail_duplicate.carbon b/toolchain/check/testdata/namespace/fail_duplicate.carbon index c383f0ef3477..07d9e36de3a7 100644 --- a/toolchain/check/testdata/namespace/fail_duplicate.carbon +++ b/toolchain/check/testdata/namespace/fail_duplicate.carbon @@ -9,7 +9,7 @@ namespace Foo; fn Foo.Baz() { } -// CHECK:STDERR: fail_duplicate.carbon:[[@LINE+6]]:1: ERROR: Redefinition of function Baz. +// CHECK:STDERR: fail_duplicate.carbon:[[@LINE+6]]:1: ERROR: Redefinition of `fn Baz`. // CHECK:STDERR: fn Foo.Baz() { // CHECK:STDERR: ^~~~~~~~~~~~~~ // CHECK:STDERR: fail_duplicate.carbon:[[@LINE-6]]:1: Previously defined here. diff --git a/toolchain/check/testdata/packages/cross_package_import.carbon b/toolchain/check/testdata/packages/cross_package_import.carbon index 293a072b6441..52c87ff7cc4d 100644 --- a/toolchain/check/testdata/packages/cross_package_import.carbon +++ b/toolchain/check/testdata/packages/cross_package_import.carbon @@ -116,7 +116,7 @@ import library "other_ns"; // CHECK:STDERR: import Other library "fn"; -// CHECK:STDERR: fail_main_namespace_conflict.carbon:[[@LINE+10]]:1: ERROR: Only one library can declare function F without `extern`. +// CHECK:STDERR: fail_main_namespace_conflict.carbon:[[@LINE+10]]:1: ERROR: Only one library can declare `fn F` without `extern`. // CHECK:STDERR: fn Other.F() {} // CHECK:STDERR: ^~~~~~~~~~~~~~ // CHECK:STDERR: fail_main_namespace_conflict.carbon:[[@LINE-5]]:1: In import. diff --git a/toolchain/diagnostics/diagnostic_kind.def b/toolchain/diagnostics/diagnostic_kind.def index a3ac04fe2bc9..976cc1bf2826 100644 --- a/toolchain/diagnostics/diagnostic_kind.def +++ b/toolchain/diagnostics/diagnostic_kind.def @@ -148,9 +148,17 @@ CARBON_DIAGNOSTIC_KIND(ImportSelf) CARBON_DIAGNOSTIC_KIND(ExplicitImportApi) CARBON_DIAGNOSTIC_KIND(RepeatedImport) CARBON_DIAGNOSTIC_KIND(FirstImported) -CARBON_DIAGNOSTIC_KIND(RedeclOfUsedImport) CARBON_DIAGNOSTIC_KIND(UsedImportLoc) +// Merge-related redeclaration checking. +CARBON_DIAGNOSTIC_KIND(RedeclOfUsedImport) +CARBON_DIAGNOSTIC_KIND(RedeclPrevDecl) +CARBON_DIAGNOSTIC_KIND(RedeclRedundant) +CARBON_DIAGNOSTIC_KIND(RedeclNonExtern) +CARBON_DIAGNOSTIC_KIND(RedeclPrevDef) +CARBON_DIAGNOSTIC_KIND(RedeclRedef) +CARBON_DIAGNOSTIC_KIND(RedeclExternMismatch) + // Function call checking. CARBON_DIAGNOSTIC_KIND(AddrSelfIsNonRef) CARBON_DIAGNOSTIC_KIND(CallArgCountMismatch) @@ -162,13 +170,6 @@ CARBON_DIAGNOSTIC_KIND(InCallToFunctionSelf) CARBON_DIAGNOSTIC_KIND(MissingObjectInMethodCall) // Function declaration checking. -CARBON_DIAGNOSTIC_KIND(FunctionPreviousDecl) -CARBON_DIAGNOSTIC_KIND(FunctionRedecl) -CARBON_DIAGNOSTIC_KIND(FunctionNonExternRedecl) -CARBON_DIAGNOSTIC_KIND(FunctionPreviousDefinition) -CARBON_DIAGNOSTIC_KIND(FunctionRedefinition) -CARBON_DIAGNOSTIC_KIND(FunctionDefiningExtern) -CARBON_DIAGNOSTIC_KIND(FunctionPreviousExternDecl) CARBON_DIAGNOSTIC_KIND(FunctionRedeclParamCountDiffers) CARBON_DIAGNOSTIC_KIND(FunctionRedeclParamCountPrevious) CARBON_DIAGNOSTIC_KIND(FunctionRedeclParamDiffers) @@ -181,7 +182,6 @@ CARBON_DIAGNOSTIC_KIND(InvalidMainRunSignature) CARBON_DIAGNOSTIC_KIND(MissingReturnStatement) CARBON_DIAGNOSTIC_KIND(UnknownBuiltinFunctionName) CARBON_DIAGNOSTIC_KIND(InvalidBuiltinSignature) -CARBON_DIAGNOSTIC_KIND(FunctionExternMismatch) // Class checking. CARBON_DIAGNOSTIC_KIND(AdaptWithBase)