Start adding extern logic for functions. (#3809)

This starts propagating is_extern on import, and warns when merging an
imported non-extern declaration with a local non-extern declaration.
Note this doesn't address import conflicts yet (i.e., two libraries
define an equivalent name) because they don't call merge logic.

On merge, I'm only setting values when new_is_definition because I think
it better matches the comment and resulting behavior. Note I now set
them even for bad redefinitions; I think this matches the comment, and
there's not a perfect choice here. I could change the flow back if
preferred.

Note this puts a spotlight on invalid nodes on imported decls, which I
think I'm going to need to address now. This isn't addressed by
ImportRef logic directly because the Function's decl_id is a
FunctionDecl, rather than the ImportRef that led to it. While I could
add the ImportRef link to each decl, I think adjusting the associated
NodeId is a better approach.
This commit is contained in:
Jon Ross-Perkins
2024-03-27 22:40:28 +00:00
committed by GitHub
parent 6c458ffe7e
commit a79027120a
11 changed files with 748 additions and 163 deletions
+77 -44
View File
@@ -191,60 +191,93 @@ auto CheckFunctionTypeMatches(Context& context,
context.functions().Get(prev_function_id), substitutions);
}
// Checks to see if a structurally valid redeclaration is allowed in context.
// These all still merge.
static auto CheckIsAllowedRedecl(Context& context, Parse::NodeId node_id,
SemIR::Function& new_function,
bool new_is_definition,
SemIR::Function& prev_function,
bool prev_is_import) -> void {
CARBON_DIAGNOSTIC(FunctionPreviousDecl, Note, "Previously declared here.");
if (prev_is_import) {
// TODO: Allow non-extern declarations in the same 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(node_id, FunctionNonExternRedecl, prev_function.name_id)
.Note(prev_function.decl_id, FunctionPreviousDecl)
.Emit();
return;
}
} else {
if (!new_is_definition) {
CARBON_DIAGNOSTIC(FunctionRedecl, Error,
"Redundant redeclaration of function {0}.",
SemIR::NameId);
context.emitter()
.Build(node_id, FunctionRedecl, prev_function.name_id)
.Note(prev_function.decl_id, FunctionPreviousDecl)
.Emit();
return;
}
if (prev_function.definition_id.is_valid()) {
CARBON_DIAGNOSTIC(FunctionRedefinition, Error,
"Redefinition of function {0}.", SemIR::NameId);
CARBON_DIAGNOSTIC(FunctionPreviousDefinition, Note,
"Previously defined here.");
context.emitter()
.Build(node_id, FunctionRedefinition, prev_function.name_id)
.Note(prev_function.definition_id, FunctionPreviousDefinition)
.Emit();
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(node_id, FunctionDefiningExtern, prev_function.name_id)
.Note(prev_function.decl_id, FunctionPreviousExternDecl)
.Emit();
return;
}
}
}
// TODO: Detect conflicting cross-file declarations, as well as uses of imported
// declarations followed by a redeclaration.
auto MergeFunctionRedecl(Context& context, Parse::NodeId node_id,
SemIR::Function& new_function,
SemIR::FunctionId prev_function_id, bool is_definition)
-> bool {
SemIR::Function& new_function, bool new_is_definition,
SemIR::FunctionId prev_function_id,
bool prev_is_import) -> bool {
auto& prev_function = context.functions().Get(prev_function_id);
if (!CheckRedecl(context, new_function, prev_function, {})) {
return false;
}
if (!is_definition) {
CARBON_DIAGNOSTIC(FunctionRedecl, Error,
"Redundant redeclaration of function {0}.",
SemIR::NameId);
CARBON_DIAGNOSTIC(FunctionPreviousDecl, Note, "Previously declared here.");
context.emitter()
.Build(node_id, FunctionRedecl, prev_function.name_id)
.Note(prev_function.decl_id, FunctionPreviousDecl)
.Emit();
// The diagnostic doesn't prevent a merge.
return true;
} else if (prev_function.definition_id.is_valid()) {
CARBON_DIAGNOSTIC(FunctionRedefinition, Error,
"Redefinition of function {0}.", SemIR::NameId);
CARBON_DIAGNOSTIC(FunctionPreviousDefinition, Note,
"Previously defined here.");
context.emitter()
.Build(node_id, FunctionRedefinition, prev_function.name_id)
.Note(prev_function.definition_id, FunctionPreviousDefinition)
.Emit();
// The second definition will be unused as a consequence of the error.
return true;
} else if (prev_function.is_extern) {
CARBON_DIAGNOSTIC(FunctionDefiningExtern, Error,
"Cannot define `extern` function `{0}`.", SemIR::NameId);
CARBON_DIAGNOSTIC(FunctionPreviousExternDecl, Note,
"Previously declared `extern` here.");
context.emitter()
.Build(node_id, FunctionDefiningExtern, prev_function.name_id)
.Note(prev_function.decl_id, FunctionPreviousExternDecl)
.Emit();
// The diagnostic doesn't prevent a merge.
return true;
}
CheckIsAllowedRedecl(context, node_id, new_function, new_is_definition,
prev_function, prev_is_import);
// Track the signature from the definition, so that IDs in the body
// match IDs in the signature.
prev_function.definition_id = new_function.definition_id;
prev_function.implicit_param_refs_id = new_function.implicit_param_refs_id;
prev_function.param_refs_id = new_function.param_refs_id;
prev_function.return_type_id = new_function.return_type_id;
prev_function.return_slot_id = new_function.return_slot_id;
if (new_is_definition) {
// Track the signature from the definition, so that IDs in the body
// match IDs in the signature.
prev_function.definition_id = new_function.definition_id;
prev_function.implicit_param_refs_id = new_function.implicit_param_refs_id;
prev_function.param_refs_id = new_function.param_refs_id;
prev_function.return_type_id = new_function.return_type_id;
prev_function.return_slot_id = new_function.return_slot_id;
}
if (!new_function.is_extern) {
prev_function.is_extern = false;
}
return true;
}