Skip to content

Commit

Permalink
Add fields for extern to EntityWithParamsBase (#4206)
Browse files Browse the repository at this point in the history
This adds fields to `EntityWithParamsBase` to reflect the intention with
`extern library` design. I'm renaming `decl_id` because it shouldn't be
expected to be assigned anymore. import_ref.cpp I'm deliberately keeping
on `first_owning_decl_id` (which will break when importing `extern
library` declarations). Most other cases are for diagnostics, and I'm
using `latest_decl_id` to try and get the closest declaration to the
error. Note I'm partly splitting out this PR to show the test effect,
which apparently we don't test related cases.
  • Loading branch information
jonmeow authored Aug 12, 2024
1 parent c5b5d36 commit 0feb757
Show file tree
Hide file tree
Showing 14 changed files with 80 additions and 54 deletions.
4 changes: 2 additions & 2 deletions toolchain/check/context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ auto Context::NoteIncompleteClass(SemIR::ClassId class_id,
} else {
CARBON_DIAGNOSTIC(ClassForwardDeclaredHere, Note,
"Class was forward declared here.");
builder.Note(class_info.decl_id, ClassForwardDeclaredHere);
builder.Note(class_info.latest_decl_id(), ClassForwardDeclaredHere);
}
}

Expand All @@ -214,7 +214,7 @@ auto Context::NoteUndefinedInterface(SemIR::InterfaceId interface_id,
} else {
CARBON_DIAGNOSTIC(InterfaceForwardDeclaredHere, Note,
"Interface was forward declared here.");
builder.Note(interface_info.decl_id, InterfaceForwardDeclaredHere);
builder.Note(interface_info.latest_decl_id(), InterfaceForwardDeclaredHere);
}
}

Expand Down
10 changes: 5 additions & 5 deletions toolchain/check/convert.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1193,7 +1193,7 @@ auto ConvertCallArgs(Context& context, SemIR::LocId call_loc_id,
context.emitter()
.Build(call_loc_id, CallArgCountMismatch, arg_refs.size(),
param_refs.size())
.Note(callee.decl_id, InCallToFunction)
.Note(callee.latest_decl_id(), InCallToFunction)
.Emit();
return SemIR::InstBlockId::Invalid;
}
Expand All @@ -1210,9 +1210,9 @@ auto ConvertCallArgs(Context& context, SemIR::LocId call_loc_id,
auto [param_id, param] = SemIR::Function::GetParamFromParamRefId(
context.sem_ir(), implicit_param_id);
if (param.name_id == SemIR::NameId::SelfValue) {
auto converted_self_id =
ConvertSelf(context, call_loc_id, callee.decl_id, callee_specific_id,
addr_pattern, param_id, param, self_id);
auto converted_self_id = ConvertSelf(
context, call_loc_id, callee.latest_decl_id(), callee_specific_id,
addr_pattern, param_id, param, self_id);
if (converted_self_id == SemIR::InstId::BuiltinError) {
return SemIR::InstBlockId::Invalid;
}
Expand All @@ -1230,7 +1230,7 @@ auto ConvertCallArgs(Context& context, SemIR::LocId call_loc_id,
CARBON_DIAGNOSTIC(
InCallToFunctionParam, Note,
"Initializing parameter {0} of function declared here.", int);
builder.Note(callee.decl_id, InCallToFunctionParam,
builder.Note(callee.latest_decl_id(), InCallToFunctionParam,
diag_param_index + 1);
});

Expand Down
9 changes: 6 additions & 3 deletions toolchain/check/decl_name_stack.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ class DeclNameStack {

// Combines name information to produce a base struct for entity
// construction.
auto MakeEntityWithParamsBase(SemIR::InstId decl_id,
const NameComponent& name)
auto MakeEntityWithParamsBase(const NameComponent& name,
SemIR::InstId decl_id, bool is_extern)
-> SemIR::EntityWithParamsBase {
return {
.name_id = name_id_for_new_inst(),
Expand All @@ -101,7 +101,10 @@ class DeclNameStack {
.last_param_node_id = name.last_param_node_id,
.implicit_param_refs_id = name.implicit_params_id,
.param_refs_id = name.params_id,
.decl_id = decl_id,
.is_extern = is_extern,
.extern_library_id = StringLiteralValueId::Invalid,
.non_owning_decl_id = SemIR::InstId::Invalid,
.first_owning_decl_id = decl_id,
};
}

Expand Down
10 changes: 5 additions & 5 deletions toolchain/check/function.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,21 +41,21 @@ auto CheckFunctionTypeMatches(Context& context,
"Function redeclaration differs because no return type is provided.");
auto diag =
new_return_type_id.is_valid()
? context.emitter().Build(new_function.decl_id,
? context.emitter().Build(new_function.latest_decl_id(),
FunctionRedeclReturnTypeDiffers,
new_return_type_id)
: context.emitter().Build(new_function.decl_id,
: context.emitter().Build(new_function.latest_decl_id(),
FunctionRedeclReturnTypeDiffersNoReturn);
if (prev_return_type_id.is_valid()) {
CARBON_DIAGNOSTIC(FunctionRedeclReturnTypePrevious, Note,
"Previously declared with return type `{0}`.",
SemIR::TypeId);
diag.Note(prev_function.decl_id, FunctionRedeclReturnTypePrevious,
prev_return_type_id);
diag.Note(prev_function.latest_decl_id(),
FunctionRedeclReturnTypePrevious, prev_return_type_id);
} else {
CARBON_DIAGNOSTIC(FunctionRedeclReturnTypePreviousNoReturn, Note,
"Previously declared with no return type.");
diag.Note(prev_function.decl_id,
diag.Note(prev_function.latest_decl_id(),
FunctionRedeclReturnTypePreviousNoReturn);
}
diag.Emit();
Expand Down
6 changes: 4 additions & 2 deletions toolchain/check/global_init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,11 @@ auto GlobalInit::Finalize() -> void {
.last_param_node_id = Parse::NodeId::Invalid,
.implicit_param_refs_id = SemIR::InstBlockId::Invalid,
.param_refs_id = SemIR::InstBlockId::Empty,
.decl_id = SemIR::InstId::Invalid},
{.return_storage_id = SemIR::InstId::Invalid,
.is_extern = false,
.extern_library_id = StringLiteralValueId::Invalid,
.non_owning_decl_id = SemIR::InstId::Invalid,
.first_owning_decl_id = SemIR::InstId::Invalid},
{.return_storage_id = SemIR::InstId::Invalid,
.body_block_ids = {SemIR::InstBlockId::GlobalInit}}});
}

Expand Down
6 changes: 3 additions & 3 deletions toolchain/check/handle_class.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,10 @@ static auto MergeClassRedecl(Context& context, SemIRLoc new_loc,

if ((prev_import_ir_id.is_valid() && !new_is_import) ||
(prev_is_extern && !new_is_extern)) {
prev_class.decl_id = new_class.decl_id;
prev_class.first_owning_decl_id = new_class.first_owning_decl_id;
ReplacePrevInstForMerge(
context, prev_class.parent_scope_id, prev_class.name_id,
new_is_import ? new_loc.inst_id : new_class.decl_id);
new_is_import ? new_loc.inst_id : new_class.first_owning_decl_id);
}
return true;
}
Expand Down Expand Up @@ -219,7 +219,7 @@ static auto BuildClassDecl(Context& context, Parse::AnyClassDeclId node_id,

// TODO: Store state regarding is_extern.
SemIR::Class class_info = {
name_context.MakeEntityWithParamsBase(class_decl_id, name),
name_context.MakeEntityWithParamsBase(name, class_decl_id, is_extern),
{// `.self_type_id` depends on the ClassType, so is set below.
.self_type_id = SemIR::TypeId::Invalid,
.inheritance_kind = inheritance_kind}};
Expand Down
13 changes: 7 additions & 6 deletions toolchain/check/handle_function.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,10 @@ static auto MergeFunctionRedecl(Context& context, SemIRLoc new_loc,
if ((prev_import_ir_id.is_valid() && !new_is_import) ||
(prev_function.is_extern && !new_function.is_extern)) {
prev_function.is_extern = new_function.is_extern;
prev_function.decl_id = new_function.decl_id;
prev_function.first_owning_decl_id = new_function.first_owning_decl_id;
ReplacePrevInstForMerge(context, prev_function.parent_scope_id,
prev_function.name_id, new_function.decl_id);
prev_function.name_id,
new_function.first_owning_decl_id);
}
return true;
}
Expand Down Expand Up @@ -151,7 +152,7 @@ static auto TryMergeRedecl(Context& context, Parse::AnyFunctionDeclId node_id,
}

if (!prev_function_id.is_valid()) {
context.DiagnoseDuplicateName(function_info.decl_id, prev_id);
context.DiagnoseDuplicateName(function_info.latest_decl_id(), prev_id);
return;
}

Expand Down Expand Up @@ -216,8 +217,8 @@ static auto BuildFunctionDecl(Context& context,
// Build the function entity. This will be merged into an existing function if
// there is one, or otherwise added to the function store.
auto function_info = SemIR::Function{
{name_context.MakeEntityWithParamsBase(decl_id, name)},
{.return_storage_id = return_storage_id, .is_extern = is_extern}};
{name_context.MakeEntityWithParamsBase(name, decl_id, is_extern)},
{.return_storage_id = return_storage_id}};
if (is_definition) {
function_info.definition_id = decl_id;
}
Expand All @@ -244,7 +245,7 @@ static auto BuildFunctionDecl(Context& context,
if (!name_context.prev_inst_id().is_valid()) {
// At interface scope, a function declaration introduces an associated
// function.
auto lookup_result_id = function_info.decl_id;
auto lookup_result_id = decl_id;
if (parent_scope_inst && !name_context.has_qualifiers) {
if (auto interface_scope =
parent_scope_inst->TryAs<SemIR::InterfaceDecl>()) {
Expand Down
6 changes: 3 additions & 3 deletions toolchain/check/handle_interface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ static auto BuildInterfaceDecl(Context& context,
{.loc = node_id,
.is_definition = is_definition,
.is_extern = false},
{.loc = existing_interface.decl_id,
{.loc = existing_interface.latest_decl_id(),
.is_definition = existing_interface.is_defined(),
.is_extern = false},
/*prev_import_ir_id=*/SemIR::ImportIRId::Invalid);
Expand Down Expand Up @@ -103,8 +103,8 @@ static auto BuildInterfaceDecl(Context& context,
// there was an error in the qualifier, we will have lost track of the
// interface name here. We should keep track of it even if the name is
// invalid.
SemIR::Interface interface_info = {
name_context.MakeEntityWithParamsBase(interface_decl_id, name)};
SemIR::Interface interface_info = {name_context.MakeEntityWithParamsBase(
name, interface_decl_id, /*is_extern=*/false)};
interface_info.generic_id = FinishGenericDecl(context, interface_decl_id);
interface_decl.interface_id = context.interfaces().Add(interface_info);
if (interface_info.has_parameters()) {
Expand Down
3 changes: 2 additions & 1 deletion toolchain/check/impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ static auto NoteAssociatedFunction(Context& context,
CARBON_DIAGNOSTIC(ImplAssociatedFunctionHere, Note,
"Associated function {0} declared here.", SemIR::NameId);
const auto& function = context.functions().Get(function_id);
builder.Note(function.decl_id, ImplAssociatedFunctionHere, function.name_id);
builder.Note(function.latest_decl_id(), ImplAssociatedFunctionHere,
function.name_id);
}

// Gets the self specific of a generic declaration that is an interface member,
Expand Down
38 changes: 22 additions & 16 deletions toolchain/check/import_ref.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -838,6 +838,7 @@ class ImportRefResolver {
// of the parameter lists that we know whether this interface is a generic
// interface and can build the right constant value for it.
//
// TODO: Support extern.
// TODO: Add a better way to represent a generic prior to importing the
// parameters.
auto GetIncompleteLocalEntityBase(
Expand All @@ -855,7 +856,10 @@ class ImportRefResolver {
.param_refs_id = import_base.param_refs_id.is_valid()
? SemIR::InstBlockId::Empty
: SemIR::InstBlockId::Invalid,
.decl_id = decl_id,
.is_extern = import_base.is_extern,
.extern_library_id = StringLiteralValueId::Invalid,
.non_owning_decl_id = SemIR::InstId::Invalid,
.first_owning_decl_id = decl_id,
};
}

Expand Down Expand Up @@ -1176,8 +1180,9 @@ class ImportRefResolver {
SemIR::ClassDecl class_decl = {.type_id = SemIR::TypeId::TypeType,
.class_id = SemIR::ClassId::Invalid,
.decl_block_id = SemIR::InstBlockId::Empty};
auto class_decl_id = context_.AddPlaceholderInstInNoBlock(
SemIR::LocIdAndInst(AddImportIRInst(import_class.decl_id), class_decl));
auto class_decl_id =
context_.AddPlaceholderInstInNoBlock(SemIR::LocIdAndInst(
AddImportIRInst(import_class.latest_decl_id()), class_decl));
// Regardless of whether ClassDecl is a complete type, we first need an
// incomplete type so that any references have something to point at.
class_decl.class_id = context_.classes().Add(
Expand All @@ -1200,13 +1205,14 @@ class ImportRefResolver {
SemIR::Class& new_class,
SemIR::ConstantId object_repr_const_id,
SemIR::InstId base_id) -> void {
new_class.definition_id = new_class.decl_id;
new_class.definition_id = new_class.first_owning_decl_id;

new_class.object_repr_id =
context_.GetTypeIdForTypeConstant(object_repr_const_id);

new_class.scope_id = context_.name_scopes().Add(
new_class.decl_id, SemIR::NameId::Invalid, new_class.parent_scope_id);
new_class.first_owning_decl_id, SemIR::NameId::Invalid,
new_class.parent_scope_id);
auto& new_scope = context_.name_scopes().Get(new_class.scope_id);
const auto& import_scope =
import_ir_.name_scopes().Get(import_class.scope_id);
Expand Down Expand Up @@ -1301,8 +1307,8 @@ class ImportRefResolver {

auto TryResolveTypedInst(SemIR::ClassType inst) -> ResolveResult {
CARBON_CHECK(inst.type_id == SemIR::TypeId::TypeType);
auto class_const_id =
GetLocalConstantId(import_ir_.classes().Get(inst.class_id).decl_id);
auto class_const_id = GetLocalConstantId(
import_ir_.classes().Get(inst.class_id).first_owning_decl_id);
auto specific_data = GetLocalSpecificData(inst.specific_id);
if (HasNewWork()) {
return Retry();
Expand Down Expand Up @@ -1373,7 +1379,6 @@ class ImportRefResolver {
function_decl.function_id = context_.functions().Add(
{GetIncompleteLocalEntityBase(function_decl_id, import_function),
{.return_storage_id = SemIR::InstId::Invalid,
.is_extern = import_function.is_extern,
.builtin_function_kind = import_function.builtin_function_kind}});

function_decl.type_id =
Expand Down Expand Up @@ -1457,7 +1462,7 @@ class ImportRefResolver {
}

if (import_function.definition_id.is_valid()) {
new_function.definition_id = new_function.decl_id;
new_function.definition_id = new_function.first_owning_decl_id;
}

return ResolveAsConstant(function_const_id);
Expand All @@ -1466,7 +1471,7 @@ class ImportRefResolver {
auto TryResolveTypedInst(SemIR::FunctionType inst) -> ResolveResult {
CARBON_CHECK(inst.type_id == SemIR::TypeId::TypeType);
auto fn_val_id = GetLocalConstantInstId(
import_ir_.functions().Get(inst.function_id).decl_id);
import_ir_.functions().Get(inst.function_id).first_owning_decl_id);
auto specific_data = GetLocalSpecificData(inst.specific_id);
if (HasNewWork()) {
return Retry();
Expand All @@ -1483,8 +1488,8 @@ class ImportRefResolver {

auto TryResolveTypedInst(SemIR::GenericClassType inst) -> ResolveResult {
CARBON_CHECK(inst.type_id == SemIR::TypeId::TypeType);
auto class_val_id =
GetLocalConstantInstId(import_ir_.classes().Get(inst.class_id).decl_id);
auto class_val_id = GetLocalConstantInstId(
import_ir_.classes().Get(inst.class_id).first_owning_decl_id);
if (HasNewWork()) {
return Retry();
}
Expand All @@ -1498,7 +1503,7 @@ class ImportRefResolver {
auto TryResolveTypedInst(SemIR::GenericInterfaceType inst) -> ResolveResult {
CARBON_CHECK(inst.type_id == SemIR::TypeId::TypeType);
auto interface_val_id = GetLocalConstantInstId(
import_ir_.interfaces().Get(inst.interface_id).decl_id);
import_ir_.interfaces().Get(inst.interface_id).first_owning_decl_id);
if (HasNewWork()) {
return Retry();
}
Expand Down Expand Up @@ -1537,7 +1542,8 @@ class ImportRefResolver {
.decl_block_id = SemIR::InstBlockId::Empty};
auto interface_decl_id =
context_.AddPlaceholderInstInNoBlock(SemIR::LocIdAndInst(
AddImportIRInst(import_interface.decl_id), interface_decl));
AddImportIRInst(import_interface.first_owning_decl_id),
interface_decl));

// Start with an incomplete interface.
interface_decl.interface_id = context_.interfaces().Add(
Expand All @@ -1561,7 +1567,7 @@ class ImportRefResolver {
SemIR::Interface& new_interface,
SemIR::InstId self_param_id) -> void {
new_interface.scope_id = context_.name_scopes().Add(
new_interface.decl_id, SemIR::NameId::Invalid,
new_interface.first_owning_decl_id, SemIR::NameId::Invalid,
new_interface.parent_scope_id);
auto& new_scope = context_.name_scopes().Get(new_interface.scope_id);
const auto& import_scope =
Expand Down Expand Up @@ -1647,7 +1653,7 @@ class ImportRefResolver {
auto TryResolveTypedInst(SemIR::InterfaceType inst) -> ResolveResult {
CARBON_CHECK(inst.type_id == SemIR::TypeId::TypeType);
auto interface_const_id = GetLocalConstantId(
import_ir_.interfaces().Get(inst.interface_id).decl_id);
import_ir_.interfaces().Get(inst.interface_id).first_owning_decl_id);
auto specific_data = GetLocalSpecificData(inst.specific_id);
if (HasNewWork()) {
return Retry();
Expand Down
2 changes: 1 addition & 1 deletion toolchain/check/merge.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ auto ReplacePrevInstForMerge(Context& context, SemIR::NameScopeId scope_id,
// different kinds of entity such as classes and functions.
struct DeclParams {
explicit DeclParams(const SemIR::EntityWithParamsBase& base)
: loc(base.decl_id),
: loc(base.latest_decl_id()),
first_param_node_id(base.first_param_node_id),
last_param_node_id(base.last_param_node_id),
implicit_param_refs_id(base.implicit_param_refs_id),
Expand Down
2 changes: 1 addition & 1 deletion toolchain/check/return.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ static auto NoteNoReturnTypeProvided(Context::DiagnosticBuilder& diag,
const SemIR::Function& function) {
CARBON_DIAGNOSTIC(ReturnTypeOmittedNote, Note,
"There was no return type provided.");
diag.Note(function.decl_id, ReturnTypeOmittedNote);
diag.Note(function.latest_decl_id(), ReturnTypeOmittedNote);
}

// Produces a note describing the return type of the given function.
Expand Down
23 changes: 19 additions & 4 deletions toolchain/sem_ir/entity_with_params_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,13 @@ struct EntityWithParamsBase {

// Returns the instruction for the latest declaration.
auto latest_decl_id() const -> SemIR::InstId {
return definition_id.is_valid() ? definition_id : decl_id;
if (definition_id.is_valid()) {
return definition_id;
}
if (first_owning_decl_id.is_valid()) {
return first_owning_decl_id;
}
return non_owning_decl_id;
}

// Determines whether this entity has any parameter lists.
Expand All @@ -77,12 +83,21 @@ struct EntityWithParamsBase {
InstBlockId implicit_param_refs_id;
// A block containing a single reference instruction per parameter.
InstBlockId param_refs_id;
// The first declaration of the entity. This will be a <entity>Decl.
InstId decl_id = InstId::Invalid;
// True if declarations are `extern`.
bool is_extern;
// For an `extern library` declaration, the library name.
StringLiteralValueId extern_library_id;
// The non-owning declaration of the entity, if present. This will be a
// <entity>Decl.
InstId non_owning_decl_id;
// The first owning declaration of the entity, if present. This will be a
// <entity>Decl. It may either be a forward declaration, or the same as
// `definition_id`.
InstId first_owning_decl_id;

// The following members are set at the `{` of the definition.

// The first declaration of the entity. This will be a <entity>Decl.
// The definition of the entity. This will be a <entity>Decl.
InstId definition_id = InstId::Invalid;
};

Expand Down
Loading

0 comments on commit 0feb757

Please sign in to comment.