From ad7ea755b073dff8307e724ebb93259c16fe6012 Mon Sep 17 00:00:00 2001 From: Geoff Romer Date: Tue, 16 Dec 2025 16:00:14 -0800 Subject: [PATCH] Add `Abstract` enumerator to `InitRepr::Kind` (#6513) This fixes a bug where `CheckFunctionReturnType` could sometimes fail to diagnose an abstract return type. --- toolchain/check/call.cpp | 6 +++-- toolchain/check/function.cpp | 3 ++- .../check/testdata/class/fail_abstract.carbon | 25 +++++++++---------- toolchain/lower/function_context.cpp | 3 +++ toolchain/lower/handle.cpp | 3 +++ toolchain/lower/handle_aggregates.cpp | 4 +++ toolchain/lower/handle_expr_category.cpp | 2 ++ toolchain/sem_ir/type_info.cpp | 7 ++++-- toolchain/sem_ir/type_info.h | 17 +++++++++---- 9 files changed, 47 insertions(+), 23 deletions(-) diff --git a/toolchain/check/call.cpp b/toolchain/check/call.cpp index eea60809e6ad..dcfcf561da15 100644 --- a/toolchain/check/call.cpp +++ b/toolchain/check/call.cpp @@ -268,9 +268,11 @@ auto PerformCallToFunction(Context& context, SemIR::LocId loc_id, break; case SemIR::InitRepr::ByCopy: break; + case SemIR::InitRepr::Abstract: case SemIR::InitRepr::Incomplete: - // Don't form an initializing expression with an incomplete type. - // CheckFunctionReturnType will have diagnosed this for us if needed. + // Don't form an initializing expression with an abstract or incomplete + // type. CheckFunctionReturnType will have diagnosed this for us if + // needed. return_info.type_id = SemIR::ErrorInst::TypeId; break; } diff --git a/toolchain/check/function.cpp b/toolchain/check/function.cpp index ac88eed2839c..9c2355dba75a 100644 --- a/toolchain/check/function.cpp +++ b/toolchain/check/function.cpp @@ -100,7 +100,8 @@ auto CheckFunctionReturnType(Context& context, SemIR::LocId loc_id, // If we couldn't determine the return information due to the return type // being incomplete, try to complete it now. - if (return_info.init_repr.kind == SemIR::InitRepr::Incomplete) { + if (return_info.init_repr.kind == SemIR::InitRepr::Incomplete || + return_info.init_repr.kind == SemIR::InitRepr::Abstract) { auto diagnose_incomplete_return_type = [&] { CARBON_DIAGNOSTIC(IncompleteTypeInFunctionReturnType, Error, "function returns incomplete type {0}", SemIR::TypeId); diff --git a/toolchain/check/testdata/class/fail_abstract.carbon b/toolchain/check/testdata/class/fail_abstract.carbon index 20a11636b3b0..347af2e2759e 100644 --- a/toolchain/check/testdata/class/fail_abstract.carbon +++ b/toolchain/check/testdata/class/fail_abstract.carbon @@ -146,15 +146,14 @@ class Derived { var d: {}; } +// CHECK:STDERR: fail_return_abstract.carbon:[[@LINE+7]]:27: error: function returns abstract type `Abstract` [AbstractTypeInFunctionReturnType] +// CHECK:STDERR: fn Return(a: Abstract) -> Abstract { +// CHECK:STDERR: ^~~~~~~~ +// CHECK:STDERR: fail_return_abstract.carbon:[[@LINE-12]]:1: note: class was declared abstract here [ClassAbstractHere] +// CHECK:STDERR: abstract class Abstract { +// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~ +// CHECK:STDERR: fn Return(a: Abstract) -> Abstract { - // TODO: Seems like this would be better off failing with "function returns abstract type" here instead of this \/ - // CHECK:STDERR: fail_return_abstract.carbon:[[@LINE+7]]:3: error: initialization of abstract type `Abstract` [AbstractTypeInInit] - // CHECK:STDERR: return a; - // CHECK:STDERR: ^~~~~~~~~ - // CHECK:STDERR: fail_return_abstract.carbon:[[@LINE-14]]:1: note: class was declared abstract here [ClassAbstractHere] - // CHECK:STDERR: abstract class Abstract { - // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~ - // CHECK:STDERR: return a; } @@ -643,9 +642,9 @@ fn CallReturnAbstract() { // CHECK:STDOUT: %return.patt: %pattern_type = return_slot_pattern [concrete] // CHECK:STDOUT: %return.param_patt: %pattern_type = out_param_pattern %return.patt, call_param1 [concrete] // CHECK:STDOUT: } { -// CHECK:STDOUT: %Abstract.ref.loc13_27: type = name_ref Abstract, file.%Abstract.decl [concrete = constants.%Abstract] +// CHECK:STDOUT: %Abstract.ref.loc20_27: type = name_ref Abstract, file.%Abstract.decl [concrete = constants.%Abstract] // CHECK:STDOUT: %a.param: %Abstract = value_param call_param0 -// CHECK:STDOUT: %Abstract.ref.loc13_14: type = name_ref Abstract, file.%Abstract.decl [concrete = constants.%Abstract] +// CHECK:STDOUT: %Abstract.ref.loc20_14: type = name_ref Abstract, file.%Abstract.decl [concrete = constants.%Abstract] // CHECK:STDOUT: %a: %Abstract = value_binding a, %a.param // CHECK:STDOUT: %return.param: ref %Abstract = out_param call_param1 // CHECK:STDOUT: %return: ref %Abstract = return_slot %return.param @@ -677,10 +676,10 @@ fn CallReturnAbstract() { // CHECK:STDOUT: extend %Abstract.ref // CHECK:STDOUT: } // CHECK:STDOUT: -// CHECK:STDOUT: fn @Return(%a.param: %Abstract) -> %return.param: %Abstract { +// CHECK:STDOUT: fn @Return(%a.param: %Abstract) -> %Abstract { // CHECK:STDOUT: !entry: // CHECK:STDOUT: %a.ref: %Abstract = name_ref a, %a -// CHECK:STDOUT: return to %return +// CHECK:STDOUT: return // CHECK:STDOUT: } // CHECK:STDOUT: // CHECK:STDOUT: --- access_abstract_subobject.carbon @@ -871,7 +870,7 @@ fn CallReturnAbstract() { // CHECK:STDOUT: .Self = constants.%Abstract // CHECK:STDOUT: } // CHECK:STDOUT: -// CHECK:STDOUT: fn @ReturnAbstract() -> %return.param: %Abstract; +// CHECK:STDOUT: fn @ReturnAbstract() -> %Abstract; // CHECK:STDOUT: // CHECK:STDOUT: fn @CallReturnAbstract() { // CHECK:STDOUT: !entry: diff --git a/toolchain/lower/function_context.cpp b/toolchain/lower/function_context.cpp index 1ee3469b222b..3b71013302ca 100644 --- a/toolchain/lower/function_context.cpp +++ b/toolchain/lower/function_context.cpp @@ -282,6 +282,9 @@ auto FunctionContext::FinishInit(TypeInFile type, SemIR::InstId dest_id, case SemIR::InitRepr::ByCopy: CopyValue(type, source_id, dest_id); break; + case SemIR::InitRepr::Abstract: + CARBON_FATAL("Lowering aggregate initialization of abstract type {0}", + type.file->types().GetAsInst(type.type_id)); case SemIR::InitRepr::Incomplete: CARBON_FATAL("Lowering aggregate initialization of incomplete type {0}", type.file->types().GetAsInst(type.type_id)); diff --git a/toolchain/lower/handle.cpp b/toolchain/lower/handle.cpp index 31d35f8ab440..a8e3259990aa 100644 --- a/toolchain/lower/handle.cpp +++ b/toolchain/lower/handle.cpp @@ -266,6 +266,9 @@ auto HandleInst(FunctionContext& context, SemIR::InstId /*inst_id*/, // The expression produces the value representation for the type. context.builder().CreateRet(context.GetValue(inst.expr_id)); return; + case SemIR::InitRepr::Abstract: + CARBON_FATAL("Lowering return of abstract type {0}", + result_type.file->types().GetAsInst(result_type.type_id)); case SemIR::InitRepr::Incomplete: CARBON_FATAL("Lowering return of incomplete type {0}", result_type.file->types().GetAsInst(result_type.type_id)); diff --git a/toolchain/lower/handle_aggregates.cpp b/toolchain/lower/handle_aggregates.cpp index 822664d4b2b1..c1a1a8143212 100644 --- a/toolchain/lower/handle_aggregates.cpp +++ b/toolchain/lower/handle_aggregates.cpp @@ -207,6 +207,10 @@ static auto EmitAggregateInitializer(FunctionContext& context, name); } + case SemIR::InitRepr::Abstract: + CARBON_FATAL("Lowering aggregate initialization of abstract type {0}", + type.file->types().GetAsInst(type.type_id)); + case SemIR::InitRepr::Incomplete: CARBON_FATAL("Lowering aggregate initialization of incomplete type {0}", type.file->types().GetAsInst(type.type_id)); diff --git a/toolchain/lower/handle_expr_category.cpp b/toolchain/lower/handle_expr_category.cpp index eb6355feb382..4de7578cfcc4 100644 --- a/toolchain/lower/handle_expr_category.cpp +++ b/toolchain/lower/handle_expr_category.cpp @@ -67,6 +67,8 @@ auto HandleInst(FunctionContext& context, SemIR::InstId inst_id, "TODO: Add support for InPlaceInit with custom value rep"); } break; + case SemIR::InitRepr::Abstract: + CARBON_FATAL("Unexpected abstract type"); case SemIR::InitRepr::Incomplete: CARBON_FATAL("Unexpected incomplete type"); case SemIR::InitRepr::Dependent: diff --git a/toolchain/sem_ir/type_info.cpp b/toolchain/sem_ir/type_info.cpp index e45802e5de1a..8c301a12d9e2 100644 --- a/toolchain/sem_ir/type_info.cpp +++ b/toolchain/sem_ir/type_info.cpp @@ -54,8 +54,11 @@ auto ValueRepr::IsCopyOfObjectRepr(const File& file, TypeId orig_type_id) const } auto InitRepr::ForType(const File& file, TypeId type_id) -> InitRepr { - auto value_rep = ValueRepr::ForType(file, type_id); - switch (value_rep.kind) { + auto type_info = file.types().GetCompleteTypeInfo(type_id); + if (type_info.abstract_class_id.has_value()) { + return {.kind = InitRepr::Abstract}; + } + switch (type_info.value_repr.kind) { case ValueRepr::None: return {.kind = InitRepr::None}; diff --git a/toolchain/sem_ir/type_info.h b/toolchain/sem_ir/type_info.h index aa6acc561499..48b2a1d5b30e 100644 --- a/toolchain/sem_ir/type_info.h +++ b/toolchain/sem_ir/type_info.h @@ -85,7 +85,7 @@ struct CompleteTypeInfo : public Printable { ClassId abstract_class_id = ClassId::None; }; -// The initializing representation to use when returning by value. +// The representation to use for an initializing expression of some type. struct InitRepr : Printable { // Returns information about the initializing representation to use for a // type. @@ -104,7 +104,9 @@ struct InitRepr : Printable { // An initializing expression takes a location as input, which is // initialized as a side effect of evaluating the expression. InPlace, - // No initializing expressions should exist because the type is not + // No initializing expressions should exist because the type is abstract. + Abstract, + // No initializing expressions should exist yet, because the type is not // complete. Incomplete, // TODO: Consider adding a kind where the expression takes an advisory @@ -114,9 +116,11 @@ struct InitRepr : Printable { // The kind of initializing representation used by this type. Kind kind; - // Returns whether the initializing representation information could be fully - // computed. - auto is_valid() const -> bool { return kind != Incomplete; } + // Returns whether the type can be used as the type of an initializing + // expression in the current context. + auto is_valid() const -> bool { + return kind != Incomplete && kind != Abstract; + } // Returns whether the initializing representation is a copy of the object // representation of the type. Provided for symmetry with `ValueRepr`. @@ -149,6 +153,9 @@ struct InitRepr : Printable { case InPlace: out << "InPlace"; break; + case Abstract: + out << "Abstract"; + break; case Incomplete: out << "Incomplete"; break;