diff --git a/toolchain/check/handle_function.cpp b/toolchain/check/handle_function.cpp index bd5971541925..fa36daee5185 100644 --- a/toolchain/check/handle_function.cpp +++ b/toolchain/check/handle_function.cpp @@ -70,26 +70,25 @@ static auto BuildFunctionDecl(Context& context, auto return_type_id = SemIR::TypeId::Invalid; auto return_slot_id = SemIR::InstId::Invalid; - if (auto return_node_and_id = + if (auto [return_node, return_storage_id] = context.node_stack() - .PopWithParseNodeIf()) { - auto return_storage_id = return_node_and_id->second; - return_type_id = context.insts().Get(return_storage_id).type_id(); + .PopWithParseNodeIf(); + return_storage_id) { + return_type_id = context.insts().Get(*return_storage_id).type_id(); return_type_id = context.AsCompleteType(return_type_id, [&] { CARBON_DIAGNOSTIC(IncompleteTypeInFunctionReturnType, Error, "Function returns incomplete type `{0}`.", SemIR::TypeId); - return context.emitter().Build(return_node_and_id->first, - IncompleteTypeInFunctionReturnType, - return_type_id); + return context.emitter().Build( + return_node, IncompleteTypeInFunctionReturnType, return_type_id); }); if (!SemIR::GetInitRepr(context.sem_ir(), return_type_id) .has_return_slot()) { // The function only has a return slot if it uses in-place initialization. } else { - return_slot_id = return_storage_id; + return_slot_id = *return_storage_id; } } diff --git a/toolchain/check/handle_impl.cpp b/toolchain/check/handle_impl.cpp index d57515a4b77b..4d6b86b26420 100644 --- a/toolchain/check/handle_impl.cpp +++ b/toolchain/check/handle_impl.cpp @@ -6,6 +6,7 @@ #include "toolchain/check/convert.h" #include "toolchain/check/decl_name_stack.h" #include "toolchain/check/modifiers.h" +#include "toolchain/parse/typed_nodes.h" #include "toolchain/sem_ir/ids.h" #include "toolchain/sem_ir/typed_insts.h" @@ -63,8 +64,7 @@ static auto TryAsClassScope(Context& context, SemIR::NameScopeId scope_id) return context.insts().TryGetAs(scope.inst_id); } -static auto GetDefaultSelfType(Context& context, Parse::NodeId parse_node) - -> SemIR::TypeId { +static auto GetDefaultSelfType(Context& context) -> SemIR::TypeId { auto enclosing_scope_id = context.decl_name_stack().PeekTargetScope(); if (auto class_decl = TryAsClassScope(context, enclosing_scope_id)) { @@ -73,24 +73,32 @@ static auto GetDefaultSelfType(Context& context, Parse::NodeId parse_node) // TODO: This is also valid in a mixin. - CARBON_DIAGNOSTIC(ImplAsOutsideClass, Error, - "`impl as` can only be used in a class."); - context.emitter().Emit(parse_node, ImplAsOutsideClass); - return SemIR::TypeId::Error; + return SemIR::TypeId::Invalid; } auto HandleDefaultSelfImplAs(Context& context, Parse::DefaultSelfImplAsId parse_node) -> bool { - auto self_type_id = GetDefaultSelfType(context, parse_node); + auto self_type_id = GetDefaultSelfType(context); + if (!self_type_id.is_valid()) { + CARBON_DIAGNOSTIC(ImplAsOutsideClass, Error, + "`impl as` can only be used in a class."); + context.emitter().Emit(parse_node, ImplAsOutsideClass); + self_type_id = SemIR::TypeId::Error; + } + context.node_stack().Push(parse_node, self_type_id); return true; } // Process an `extend impl` declaration by extending the impl scope with the // `impl`'s scope. -static auto ExtendImpl(Context& context, Parse::AnyImplDeclId parse_node, - SemIR::TypeId constraint_id) -> void { +static auto ExtendImpl(Context& context, Parse::NodeId extend_node, + Parse::AnyImplDeclId parse_node, + Parse::NodeId self_type_node, SemIR::TypeId self_type_id, + Parse::NodeId params_node, SemIR::TypeId constraint_id) + -> void { auto enclosing_scope_id = context.decl_name_stack().PeekTargetScope(); + auto& enclosing_scope = context.name_scopes().Get(enclosing_scope_id); // TODO: This is also valid in a mixin. if (!TryAsClassScope(context, enclosing_scope_id)) { @@ -99,7 +107,38 @@ static auto ExtendImpl(Context& context, Parse::AnyImplDeclId parse_node, context.emitter().Emit(parse_node, ExtendImplOutsideClass); return; } - auto& enclosing_scope = context.name_scopes().Get(enclosing_scope_id); + + if (params_node.is_valid()) { + CARBON_DIAGNOSTIC(ExtendImplForall, Error, + "Cannot `extend` a parameterized `impl`."); + context.emitter().Emit(extend_node, ExtendImplForall); + enclosing_scope.has_error = true; + return; + } + + if (context.parse_tree().node_kind(self_type_node) == + Parse::NodeKind::TypeImplAs) { + CARBON_DIAGNOSTIC(ExtendImplSelfAs, Error, + "Cannot `extend` an `impl` with an explicit self type."); + auto diag = context.emitter().Build(extend_node, ExtendImplSelfAs); + + // If the explicit self type is not the default, just bail out. + if (self_type_id != GetDefaultSelfType(context)) { + diag.Emit(); + enclosing_scope.has_error = true; + return; + } + + // The explicit self type is the same as the default self type, so suggest + // removing it and recover as if it were not present. + if (auto self_as = + context.parse_tree().ExtractAs(self_type_node)) { + CARBON_DIAGNOSTIC(ExtendImplSelfAsDefault, Note, + "Remove the explicit `Self` type here."); + diag.Note(self_as->type_expr, ExtendImplSelfAsDefault); + } + diag.Emit(); + } auto interface_type = context.types().TryGetAs(constraint_id); @@ -132,9 +171,10 @@ static auto BuildImplDecl(Context& context, Parse::AnyImplDeclId parse_node) -> std::pair { auto [constraint_node, constraint_id] = context.node_stack().PopExprWithParseNode(); - auto self_type_id = context.node_stack().Pop(); - - auto params_id = context.node_stack().PopIf(); + auto [self_type_node, self_type_id] = + context.node_stack().PopWithParseNode(); + auto [params_node, params_id] = + context.node_stack().PopWithParseNodeIf(); auto decl_block_id = context.inst_block_stack().Pop(); context.node_stack().PopForSoloParseNode(); @@ -168,9 +208,9 @@ static auto BuildImplDecl(Context& context, Parse::AnyImplDeclId parse_node) // For an `extend impl` declaration, mark the impl as extending this `impl`. if (!!(context.decl_state_stack().innermost().modifier_set & KeywordModifierSet::Extend)) { - // TODO: Diagnose combining `extend` with `forall`. - // TODO: Diagnose combining `extend` with an explicit self type. - ExtendImpl(context, parse_node, constraint_type_id); + auto extend_node = context.decl_state_stack().innermost().saw_decl_modifier; + ExtendImpl(context, extend_node, parse_node, self_type_node, self_type_id, + params_node, constraint_type_id); } context.decl_state_stack().Pop(DeclState::Impl); diff --git a/toolchain/check/node_stack.h b/toolchain/check/node_stack.h index 9665b71de598..2878534bbb30 100644 --- a/toolchain/check/node_stack.h +++ b/toolchain/check/node_stack.h @@ -9,6 +9,7 @@ #include "common/vlog.h" #include "llvm/ADT/SmallVector.h" +#include "toolchain/parse/node_ids.h" #include "toolchain/parse/node_kind.h" #include "toolchain/parse/tree.h" #include "toolchain/parse/typed_nodes.h" @@ -247,28 +248,6 @@ class NodeStack { return std::make_pair(parse_node, id); } - // Pops the top of the stack and returns the parse_node and the ID if it is - // of the specified kind. - template - auto PopWithParseNodeIf() - -> std::optional())> { - if (!PeekIs()) { - return std::nullopt; - } - return PopWithParseNode(); - } - - // Pops the top of the stack and returns the parse_node and the ID if it is - // of the specified category - template - auto PopWithParseNodeIf() - -> std::optional())> { - if (!PeekIs()) { - return std::nullopt; - } - return PopWithParseNode(); - } - // Pops an expression from the top of the stack and returns the ID. // Expressions always map Parse::NodeCategory::Expr nodes to SemIR::InstId. auto PopExpr() -> SemIR::InstId { return PopExprWithParseNode().second; } @@ -320,6 +299,30 @@ class NodeStack { return std::nullopt; } + // Pops the top of the stack and returns the parse_node and the ID if it is + // of the specified kind. + template + auto PopWithParseNodeIf() + -> std::pair, + decltype(PopIf())> { + if (!PeekIs()) { + return {Parse::NodeId::Invalid, std::nullopt}; + } + return PopWithParseNode(); + } + + // Pops the top of the stack and returns the parse_node and the ID if it is + // of the specified category. + template + auto PopWithParseNodeIf() + -> std::pair, + decltype(PopIf())> { + if (!PeekIs()) { + return {Parse::NodeId::Invalid, std::nullopt}; + } + return PopWithParseNode(); + } + // Peeks at the parse node of the top of the node stack. auto PeekParseNode() const -> Parse::NodeId { return stack_.back().parse_node; diff --git a/toolchain/check/testdata/impl/fail_extend_impl_forall.carbon b/toolchain/check/testdata/impl/fail_extend_impl_forall.carbon index 518e6003a9b5..68d308c6e616 100644 --- a/toolchain/check/testdata/impl/fail_extend_impl_forall.carbon +++ b/toolchain/check/testdata/impl/fail_extend_impl_forall.carbon @@ -12,10 +12,9 @@ interface GenericInterface(T:! type) { } class C { - // TODO: We should reject this for combining `extend` with `forall`. - // CHECK:STDERR: fail_extend_impl_forall.carbon:[[@LINE+6]]:3: ERROR: Semantics TODO: `extending non-interface constraint`. + // CHECK:STDERR: fail_extend_impl_forall.carbon:[[@LINE+6]]:3: ERROR: Cannot `extend` a parameterized `impl`. // CHECK:STDERR: extend impl forall [T:! type] as GenericInterface(T) { - // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + // CHECK:STDERR: ^~~~~~ // CHECK:STDERR: fail_extend_impl_forall.carbon:[[@LINE+3]]:36: ERROR: Value of type `type` is not callable. // CHECK:STDERR: extend impl forall [T:! type] as GenericInterface(T) { // CHECK:STDERR: ^~~~~~~~~~~~~~~~~ diff --git a/toolchain/check/testdata/impl/fail_extend_impl_type_as.carbon b/toolchain/check/testdata/impl/fail_extend_impl_type_as.carbon new file mode 100644 index 000000000000..e9291da323f3 --- /dev/null +++ b/toolchain/check/testdata/impl/fail_extend_impl_type_as.carbon @@ -0,0 +1,91 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// AUTOUPDATE + +interface I {} + +class C { + // CHECK:STDERR: fail_extend_impl_type_as.carbon:[[@LINE+3]]:3: ERROR: Cannot `extend` an `impl` with an explicit self type. + // CHECK:STDERR: extend impl i32 as I {} + // CHECK:STDERR: ^~~~~~ + extend impl i32 as I {} +} + +class D { + // CHECK:STDERR: fail_extend_impl_type_as.carbon:[[@LINE+6]]:3: ERROR: Cannot `extend` an `impl` with an explicit self type. + // CHECK:STDERR: extend impl D as I; + // CHECK:STDERR: ^~~~~~ + // CHECK:STDERR: fail_extend_impl_type_as.carbon:[[@LINE+3]]:15: Remove the explicit `Self` type here. + // CHECK:STDERR: extend impl D as I; + // CHECK:STDERR: ^ + extend impl D as I; +} + +class E { + // CHECK:STDERR: fail_extend_impl_type_as.carbon:[[@LINE+6]]:3: ERROR: Cannot `extend` an `impl` with an explicit self type. + // CHECK:STDERR: extend impl Self as I {} + // CHECK:STDERR: ^~~~~~ + // CHECK:STDERR: fail_extend_impl_type_as.carbon:[[@LINE+3]]:15: Remove the explicit `Self` type here. + // CHECK:STDERR: extend impl Self as I {} + // CHECK:STDERR: ^~~~ + extend impl Self as I {} +} + +// CHECK:STDOUT: --- fail_extend_impl_type_as.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: constants { +// CHECK:STDOUT: %C: type = class_type @C [template] +// CHECK:STDOUT: %.1: type = interface_type @I [template] +// CHECK:STDOUT: %.2: type = struct_type {} [template] +// CHECK:STDOUT: %D: type = class_type @D [template] +// CHECK:STDOUT: %E: type = class_type @E [template] +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace {.I = %I.decl, .C = %C.decl, .D = %D.decl, .E = %E.decl} [template] +// CHECK:STDOUT: %I.decl = interface_decl @I, () +// CHECK:STDOUT: %C.decl = class_decl @C, () +// CHECK:STDOUT: %D.decl = class_decl @D, () +// CHECK:STDOUT: %E.decl = class_decl @E, () +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: interface @I { +// CHECK:STDOUT: +// CHECK:STDOUT: !members: +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: impl @impl.1: i32 as I { +// CHECK:STDOUT: +// CHECK:STDOUT: !members: +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: impl @impl.2: D as I; +// CHECK:STDOUT: +// CHECK:STDOUT: impl @impl.3: E as I { +// CHECK:STDOUT: +// CHECK:STDOUT: !members: +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: class @C { +// CHECK:STDOUT: impl_decl @impl.1, () +// CHECK:STDOUT: +// CHECK:STDOUT: !members: +// CHECK:STDOUT: has_error +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: class @D { +// CHECK:STDOUT: impl_decl @impl.2, (, ) +// CHECK:STDOUT: +// CHECK:STDOUT: !members: +// CHECK:STDOUT: extend name_scope1 +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: class @E { +// CHECK:STDOUT: impl_decl @impl.3, (, ) +// CHECK:STDOUT: +// CHECK:STDOUT: !members: +// CHECK:STDOUT: extend name_scope1 +// CHECK:STDOUT: } +// CHECK:STDOUT: diff --git a/toolchain/check/testdata/impl/todo_extend_impl_type_as.carbon b/toolchain/check/testdata/impl/todo_extend_impl_type_as.carbon deleted file mode 100644 index c4b744f3160e..000000000000 --- a/toolchain/check/testdata/impl/todo_extend_impl_type_as.carbon +++ /dev/null @@ -1,44 +0,0 @@ -// Part of the Carbon Language project, under the Apache License v2.0 with LLVM -// Exceptions. See /LICENSE for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -// AUTOUPDATE - -interface I {} - -class C { - // TODO: We should reject this for combining `extend` with a specific self type. - extend impl i32 as I {} -} - -// CHECK:STDOUT: --- todo_extend_impl_type_as.carbon -// CHECK:STDOUT: -// CHECK:STDOUT: constants { -// CHECK:STDOUT: %C: type = class_type @C [template] -// CHECK:STDOUT: %.1: type = interface_type @I [template] -// CHECK:STDOUT: %.2: type = struct_type {} [template] -// CHECK:STDOUT: } -// CHECK:STDOUT: -// CHECK:STDOUT: file { -// CHECK:STDOUT: package: = namespace {.I = %I.decl, .C = %C.decl} [template] -// CHECK:STDOUT: %I.decl = interface_decl @I, () -// CHECK:STDOUT: %C.decl = class_decl @C, () -// CHECK:STDOUT: } -// CHECK:STDOUT: -// CHECK:STDOUT: interface @I { -// CHECK:STDOUT: -// CHECK:STDOUT: !members: -// CHECK:STDOUT: } -// CHECK:STDOUT: -// CHECK:STDOUT: impl @impl: i32 as I { -// CHECK:STDOUT: -// CHECK:STDOUT: !members: -// CHECK:STDOUT: } -// CHECK:STDOUT: -// CHECK:STDOUT: class @C { -// CHECK:STDOUT: impl_decl @impl, () -// CHECK:STDOUT: -// CHECK:STDOUT: !members: -// CHECK:STDOUT: extend name_scope1 -// CHECK:STDOUT: } -// CHECK:STDOUT: diff --git a/toolchain/diagnostics/diagnostic_kind.def b/toolchain/diagnostics/diagnostic_kind.def index 932c3955feab..6257a6586ade 100644 --- a/toolchain/diagnostics/diagnostic_kind.def +++ b/toolchain/diagnostics/diagnostic_kind.def @@ -175,7 +175,10 @@ CARBON_DIAGNOSTIC_KIND(InterfaceForwardDeclaredHere) CARBON_DIAGNOSTIC_KIND(InterfaceUndefinedWithinDefinition) // Impl checking. +CARBON_DIAGNOSTIC_KIND(ExtendImplForall) CARBON_DIAGNOSTIC_KIND(ExtendImplOutsideClass) +CARBON_DIAGNOSTIC_KIND(ExtendImplSelfAs) +CARBON_DIAGNOSTIC_KIND(ExtendImplSelfAsDefault) CARBON_DIAGNOSTIC_KIND(ExtendUndefinedInterface) CARBON_DIAGNOSTIC_KIND(ImplAsOutsideClass) CARBON_DIAGNOSTIC_KIND(ImplPreviousDefinition) diff --git a/toolchain/parse/typed_nodes.h b/toolchain/parse/typed_nodes.h index 68067fb5e4ee..c85f90fbf540 100644 --- a/toolchain/parse/typed_nodes.h +++ b/toolchain/parse/typed_nodes.h @@ -956,7 +956,7 @@ struct TypeImplAs { static constexpr auto Kind = NodeKind::TypeImplAs.Define(NodeCategory::ImplAs); - std::optional type_expr; + AnyExprId type_expr; }; // `impl T as I`