Skip to content

Commit

Permalink
Emit diagnostics missing declaration of owned function. (#4962)
Browse files Browse the repository at this point in the history 
Emit diagnostics for a function declared in a non-owning library, that
is not redeclared (or defined) in the owning library.

---------

Co-authored-by: jonmeow <[email protected]>
  • Loading branch information
@alinas @jonmeow
alinas and jonmeow authored Feb 22, 2025
1 parent 210c26e commit 1d48270
Show file tree
Hide file tree
Showing 6 changed files with 341 additions and 36 deletions.
41 changes: 41 additions & 0 deletions toolchain/check/check_unit.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "toolchain/check/inst.h"
#include "toolchain/check/node_id_traversal.h"
#include "toolchain/check/type.h"
#include "toolchain/sem_ir/import_ir.h"

namespace Carbon::Check {

Expand Down Expand Up @@ -77,6 +78,8 @@ auto CheckUnit::Run() -> void {

CheckRequiredDefinitions();

CheckRequiredDeclarations();

context_.Finalize();

context_.VerifyOnFinish();
Expand Down Expand Up @@ -398,6 +401,44 @@ auto CheckUnit::ProcessNodeIds() -> bool {
return true;
}

auto CheckUnit::CheckRequiredDeclarations() -> void {
for (const auto& function : context_.functions().array_ref()) {
if (!function.first_owning_decl_id.has_value() &&
function.extern_library_id == context_.sem_ir().library_id()) {
auto function_loc_id =
context_.insts().GetLocId(function.non_owning_decl_id);
CARBON_CHECK(function_loc_id.is_import_ir_inst_id());
auto import_ir_id = context_.sem_ir()
.import_ir_insts()
.Get(function_loc_id.import_ir_inst_id())
.ir_id;
auto& import_ir = context_.import_irs().Get(import_ir_id);
if (import_ir.sem_ir->package_id().has_value() !=
context_.sem_ir().package_id().has_value()) {
continue;
}

CARBON_DIAGNOSTIC(
MissingOwningDeclarationInApi, Error,
"owning declaration required for non-owning declaration");
if (!import_ir.sem_ir->package_id().has_value() &&
!context_.sem_ir().package_id().has_value()) {
emitter_.Emit(function.non_owning_decl_id,
MissingOwningDeclarationInApi);
continue;
}

if (import_ir.sem_ir->identifiers().Get(
import_ir.sem_ir->package_id().AsIdentifierId()) ==
context_.sem_ir().identifiers().Get(
context_.sem_ir().package_id().AsIdentifierId())) {
emitter_.Emit(function.non_owning_decl_id,
MissingOwningDeclarationInApi);
}
}
}
}

auto CheckUnit::CheckRequiredDefinitions() -> void {
CARBON_DIAGNOSTIC(MissingDefinitionInImpl, Error,
"no definition found for declaration in impl file");
Expand Down
5 changes: 5 additions & 0 deletions toolchain/check/check_unit.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,11 @@ class CheckUnit {
// definition.
auto CheckRequiredDefinitions() -> void;

// Checks that each required declaration is available. This applies for
// declarations that should exist in an owning library, for which an extern
// declaration exists that assigns ownership to the current API.
auto CheckRequiredDeclarations() -> void;

// Loops over all nodes in the tree. On some errors, this may return early,
// for example if an unrecoverable state is encountered.
// NOLINTNEXTLINE(readability-function-size)
Expand Down
50 changes: 47 additions & 3 deletions toolchain/check/import.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,40 @@ static auto CopyAncestorNameScopesFromImportIR(
return scope_cursor;
}

// Imports the function if it's a non-owning declaration with the current file
// as owner.
static auto LoadImportForOwningFunction(Context& context,
const SemIR::File& import_sem_ir,
const SemIR::Function& function,
SemIR::InstId import_ref) {
if (!function.extern_library_id.has_value()) {
return;
}
CARBON_CHECK(function.is_extern && "Expected extern functions");
auto lib_id = function.extern_library_id;
bool is_lib_default = lib_id == SemIR::LibraryNameId::Default;

auto current_id = context.sem_ir().library_id();
bool is_current_default = current_id == SemIR::LibraryNameId::Default;

if (is_lib_default == is_current_default) {
if (is_lib_default) {
// Both libraries are default, import ref.
LoadImportRef(context, import_ref);
} else {
// Both libraries are non-default: check if they're the same named
// library, import ref if yes.
auto str_owner_library = context.string_literal_values().Get(
current_id.AsStringLiteralValueId());
auto str_decl_library = import_sem_ir.string_literal_values().Get(
lib_id.AsStringLiteralValueId());
if (str_owner_library == str_decl_library) {
LoadImportRef(context, import_ref);
}
}
}
}

// Adds an ImportRef for an entity, handling merging if needed.
static auto AddImportRefOrMerge(Context& context, SemIR::ImportIRId ir_id,
const SemIR::File& import_sem_ir,
Expand All @@ -281,10 +315,20 @@ static auto AddImportRefOrMerge(Context& context, SemIR::ImportIRId ir_id,
if (inserted) {
auto entity_name_id = context.entity_names().Add(
{.name_id = name_id, .parent_scope_id = parent_scope_id});
auto import_ref = AddImportRef(
context, {.ir_id = ir_id, .inst_id = import_inst_id}, entity_name_id);
entry.result = SemIR::ScopeLookupResult::MakeFound(
AddImportRef(context, {.ir_id = ir_id, .inst_id = import_inst_id},
entity_name_id),
SemIR::AccessKind::Public);
import_ref, SemIR::AccessKind::Public);

// Import references for non-owning declarations that match current library.
if (auto function_decl =
import_sem_ir.insts().TryGetAs<SemIR::FunctionDecl>(
import_inst_id)) {
LoadImportForOwningFunction(
context, import_sem_ir,
import_sem_ir.functions().Get(function_decl->function_id),
import_ref);
}
return;
}

Expand Down
60 changes: 30 additions & 30 deletions toolchain/check/testdata/function/declaration/import.carbon
View file
Original file line number Diff line number Diff line change
Expand Up @@ -810,32 +810,32 @@ import library "extern_api";
// CHECK:STDOUT: %A.type: type = fn_type @A [concrete]
// CHECK:STDOUT: %empty_tuple.type: type = tuple_type () [concrete]
// CHECK:STDOUT: %A: %A.type = struct_value () [concrete]
// CHECK:STDOUT: %int_32: Core.IntLiteral = int_value 32 [concrete]
// CHECK:STDOUT: %i32: type = class_type @Int, @Int(%int_32) [concrete]
// CHECK:STDOUT: %B.type: type = fn_type @B [concrete]
// CHECK:STDOUT: %B: %B.type = struct_value () [concrete]
// CHECK:STDOUT: %tuple.type.85c: type = tuple_type (type) [concrete]
// CHECK:STDOUT: %tuple.type.a1c: type = tuple_type (%i32) [concrete]
// CHECK:STDOUT: %struct_type.c: type = struct_type {.c: %i32} [concrete]
// CHECK:STDOUT: %int_32: Core.IntLiteral = int_value 32 [concrete]
// CHECK:STDOUT: %i32: type = class_type @Int, @Int(%int_32) [concrete]
// CHECK:STDOUT: %C.type: type = fn_type @C [concrete]
// CHECK:STDOUT: %C: %C.type = struct_value () [concrete]
// CHECK:STDOUT: %tuple.type.dd4: type = tuple_type (%i32) [concrete]
// CHECK:STDOUT: %struct_type.c: type = struct_type {.c: %i32} [concrete]
// CHECK:STDOUT: %D.type: type = fn_type @D [concrete]
// CHECK:STDOUT: %D: %D.type = struct_value () [concrete]
// CHECK:STDOUT: %E.type: type = fn_type @E [concrete]
// CHECK:STDOUT: %E: %E.type = struct_value () [concrete]
// CHECK:STDOUT: %tuple.type.85c: type = tuple_type (type) [concrete]
// CHECK:STDOUT: %int_1.5b8: Core.IntLiteral = int_value 1 [concrete]
// CHECK:STDOUT: %ImplicitAs.type.205: type = facet_type <@ImplicitAs, @ImplicitAs(%i32)> [concrete]
// CHECK:STDOUT: %Convert.type.1b6: type = fn_type @Convert.1, @ImplicitAs(%i32) [concrete]
// CHECK:STDOUT: %impl_witness.d39: <witness> = impl_witness (imports.%Core.import_ref.a5b), @impl.1(%int_32) [concrete]
// CHECK:STDOUT: %Convert.type.035: type = fn_type @Convert.2, @impl.1(%int_32) [concrete]
// CHECK:STDOUT: %Convert.956: %Convert.type.035 = struct_value () [concrete]
// CHECK:STDOUT: %ImplicitAs.facet: %ImplicitAs.type.205 = facet_value Core.IntLiteral, %impl_witness.d39 [concrete]
// CHECK:STDOUT: %.a0b: type = fn_type_with_self_type %Convert.type.1b6, %ImplicitAs.facet [concrete]
// CHECK:STDOUT: %Convert.bound: <bound method> = bound_method %int_1.5b8, %Convert.956 [concrete]
// CHECK:STDOUT: %ImplicitAs.type.9ba: type = facet_type <@ImplicitAs, @ImplicitAs(%i32)> [concrete]
// CHECK:STDOUT: %Convert.type.6da: type = fn_type @Convert.1, @ImplicitAs(%i32) [concrete]
// CHECK:STDOUT: %impl_witness.b97: <witness> = impl_witness (imports.%Core.import_ref.a86), @impl.1(%int_32) [concrete]
// CHECK:STDOUT: %Convert.type.ed5: type = fn_type @Convert.2, @impl.1(%int_32) [concrete]
// CHECK:STDOUT: %Convert.16d: %Convert.type.ed5 = struct_value () [concrete]
// CHECK:STDOUT: %ImplicitAs.facet: %ImplicitAs.type.9ba = facet_value Core.IntLiteral, %impl_witness.b97 [concrete]
// CHECK:STDOUT: %.39b: type = fn_type_with_self_type %Convert.type.6da, %ImplicitAs.facet [concrete]
// CHECK:STDOUT: %Convert.bound: <bound method> = bound_method %int_1.5b8, %Convert.16d [concrete]
// CHECK:STDOUT: %Convert.specific_fn: <specific function> = specific_function %Convert.bound, @Convert.2(%int_32) [concrete]
// CHECK:STDOUT: %int_1.5d2: %i32 = int_value 1 [concrete]
// CHECK:STDOUT: %int_1.47b: %i32 = int_value 1 [concrete]
// CHECK:STDOUT: %tuple.type.985: type = tuple_type (Core.IntLiteral) [concrete]
// CHECK:STDOUT: %tuple: %tuple.type.a1c = tuple_value (%int_1.5d2) [concrete]
// CHECK:STDOUT: %tuple: %tuple.type.dd4 = tuple_value (%int_1.47b) [concrete]
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: imports {
Expand Down Expand Up @@ -884,14 +884,14 @@ import library "extern_api";
// CHECK:STDOUT: %int_32.loc8_32: Core.IntLiteral = int_value 32 [concrete = constants.%int_32]
// CHECK:STDOUT: %i32.loc8_32: type = class_type @Int, @Int(constants.%int_32) [concrete = constants.%i32]
// CHECK:STDOUT: %struct_type.c: type = struct_type {.c: %i32} [concrete = constants.%struct_type.c]
// CHECK:STDOUT: %c.param: %tuple.type.a1c = value_param runtime_param0
// CHECK:STDOUT: %.loc8_21.1: type = splice_block %.loc8_21.3 [concrete = constants.%tuple.type.a1c] {
// CHECK:STDOUT: %c.param: %tuple.type.dd4 = value_param runtime_param0
// CHECK:STDOUT: %.loc8_21.1: type = splice_block %.loc8_21.3 [concrete = constants.%tuple.type.dd4] {
// CHECK:STDOUT: %int_32.loc8_17: Core.IntLiteral = int_value 32 [concrete = constants.%int_32]
// CHECK:STDOUT: %i32.loc8_17: type = class_type @Int, @Int(constants.%int_32) [concrete = constants.%i32]
// CHECK:STDOUT: %.loc8_21.2: %tuple.type.85c = tuple_literal (%i32.loc8_17)
// CHECK:STDOUT: %.loc8_21.3: type = converted %.loc8_21.2, constants.%tuple.type.a1c [concrete = constants.%tuple.type.a1c]
// CHECK:STDOUT: %.loc8_21.3: type = converted %.loc8_21.2, constants.%tuple.type.dd4 [concrete = constants.%tuple.type.dd4]
// CHECK:STDOUT: }
// CHECK:STDOUT: %c: %tuple.type.a1c = bind_name c, %c.param
// CHECK:STDOUT: %c: %tuple.type.dd4 = bind_name c, %c.param
// CHECK:STDOUT: %return.param: ref %struct_type.c = out_param runtime_param1
// CHECK:STDOUT: %return: ref %struct_type.c = return_slot %return.param
// CHECK:STDOUT: }
Expand Down Expand Up @@ -954,7 +954,7 @@ import library "extern_api";
// CHECK:STDOUT:
// CHECK:STDOUT: extern fn @B(%b.param_patt: %i32) -> %i32;
// CHECK:STDOUT:
// CHECK:STDOUT: extern fn @C(%c.param_patt: %tuple.type.a1c) -> %struct_type.c;
// CHECK:STDOUT: extern fn @C(%c.param_patt: %tuple.type.dd4) -> %struct_type.c;
// CHECK:STDOUT:
// CHECK:STDOUT: extern fn @D();
// CHECK:STDOUT:
Expand All @@ -967,25 +967,25 @@ import library "extern_api";
// CHECK:STDOUT: assign file.%a.var, %A.call
// CHECK:STDOUT: %B.ref: %B.type = name_ref B, file.%B.decl [concrete = constants.%B]
// CHECK:STDOUT: %int_1.loc13: Core.IntLiteral = int_value 1 [concrete = constants.%int_1.5b8]
// CHECK:STDOUT: %impl.elem0.loc13: %.a0b = impl_witness_access constants.%impl_witness.d39, element0 [concrete = constants.%Convert.956]
// CHECK:STDOUT: %impl.elem0.loc13: %.39b = impl_witness_access constants.%impl_witness.b97, element0 [concrete = constants.%Convert.16d]
// CHECK:STDOUT: %bound_method.loc13: <bound method> = bound_method %int_1.loc13, %impl.elem0.loc13 [concrete = constants.%Convert.bound]
// CHECK:STDOUT: %specific_fn.loc13: <specific function> = specific_function %bound_method.loc13, @Convert.2(constants.%int_32) [concrete = constants.%Convert.specific_fn]
// CHECK:STDOUT: %int.convert_checked.loc13: init %i32 = call %specific_fn.loc13(%int_1.loc13) [concrete = constants.%int_1.5d2]
// CHECK:STDOUT: %.loc13_16.1: %i32 = value_of_initializer %int.convert_checked.loc13 [concrete = constants.%int_1.5d2]
// CHECK:STDOUT: %.loc13_16.2: %i32 = converted %int_1.loc13, %.loc13_16.1 [concrete = constants.%int_1.5d2]
// CHECK:STDOUT: %int.convert_checked.loc13: init %i32 = call %specific_fn.loc13(%int_1.loc13) [concrete = constants.%int_1.47b]
// CHECK:STDOUT: %.loc13_16.1: %i32 = value_of_initializer %int.convert_checked.loc13 [concrete = constants.%int_1.47b]
// CHECK:STDOUT: %.loc13_16.2: %i32 = converted %int_1.loc13, %.loc13_16.1 [concrete = constants.%int_1.47b]
// CHECK:STDOUT: %B.call: init %i32 = call %B.ref(%.loc13_16.2)
// CHECK:STDOUT: assign file.%b.var, %B.call
// CHECK:STDOUT: %C.ref: %C.type = name_ref C, file.%C.decl [concrete = constants.%C]
// CHECK:STDOUT: %int_1.loc14: Core.IntLiteral = int_value 1 [concrete = constants.%int_1.5b8]
// CHECK:STDOUT: %.loc14_25.1: %tuple.type.985 = tuple_literal (%int_1.loc14)
// CHECK:STDOUT: %impl.elem0.loc14: %.a0b = impl_witness_access constants.%impl_witness.d39, element0 [concrete = constants.%Convert.956]
// CHECK:STDOUT: %impl.elem0.loc14: %.39b = impl_witness_access constants.%impl_witness.b97, element0 [concrete = constants.%Convert.16d]
// CHECK:STDOUT: %bound_method.loc14: <bound method> = bound_method %int_1.loc14, %impl.elem0.loc14 [concrete = constants.%Convert.bound]
// CHECK:STDOUT: %specific_fn.loc14: <specific function> = specific_function %bound_method.loc14, @Convert.2(constants.%int_32) [concrete = constants.%Convert.specific_fn]
// CHECK:STDOUT: %int.convert_checked.loc14: init %i32 = call %specific_fn.loc14(%int_1.loc14) [concrete = constants.%int_1.5d2]
// CHECK:STDOUT: %.loc14_25.2: %i32 = value_of_initializer %int.convert_checked.loc14 [concrete = constants.%int_1.5d2]
// CHECK:STDOUT: %.loc14_25.3: %i32 = converted %int_1.loc14, %.loc14_25.2 [concrete = constants.%int_1.5d2]
// CHECK:STDOUT: %tuple: %tuple.type.a1c = tuple_value (%.loc14_25.3) [concrete = constants.%tuple]
// CHECK:STDOUT: %.loc14_25.4: %tuple.type.a1c = converted %.loc14_25.1, %tuple [concrete = constants.%tuple]
// CHECK:STDOUT: %int.convert_checked.loc14: init %i32 = call %specific_fn.loc14(%int_1.loc14) [concrete = constants.%int_1.47b]
// CHECK:STDOUT: %.loc14_25.2: %i32 = value_of_initializer %int.convert_checked.loc14 [concrete = constants.%int_1.47b]
// CHECK:STDOUT: %.loc14_25.3: %i32 = converted %int_1.loc14, %.loc14_25.2 [concrete = constants.%int_1.47b]
// CHECK:STDOUT: %tuple: %tuple.type.dd4 = tuple_value (%.loc14_25.3) [concrete = constants.%tuple]
// CHECK:STDOUT: %.loc14_25.4: %tuple.type.dd4 = converted %.loc14_25.1, %tuple [concrete = constants.%tuple]
// CHECK:STDOUT: %C.call: init %struct_type.c = call %C.ref(%.loc14_25.4)
// CHECK:STDOUT: assign file.%c.var, %C.call
// CHECK:STDOUT: %D.ref: %D.type = name_ref D, file.%D.decl [concrete = constants.%D]
Expand Down
Loading

0 comments on commit 1d48270

Please sign in to comment.