Prevent invalid decls from appearing in interface or constraint

toolchain/check/handle_class.cpp

@@ -18,12 +18,15 @@
 #include "toolchain/check/import.h"
 #include "toolchain/check/import_ref.h"
 #include "toolchain/check/inst.h"
+#include "toolchain/check/interface.h"
 #include "toolchain/check/merge.h"
 #include "toolchain/check/modifiers.h"
 #include "toolchain/check/name_component.h"
 #include "toolchain/check/name_lookup.h"
+#include "toolchain/check/name_scope.h"
 #include "toolchain/check/type.h"
 #include "toolchain/check/type_completion.h"
+#include "toolchain/lex/token_kind.h"
 #include "toolchain/parse/node_ids.h"
 #include "toolchain/sem_ir/function.h"
 #include "toolchain/sem_ir/ids.h"
@@ -182,6 +185,14 @@ static auto BuildClassDecl(Context& context, Parse::AnyClassDeclId node_id,
   context.node_stack()
       .PopAndDiscardSoloNodeId<Parse::NodeKind::ClassIntroducer>();
 
+  auto enclosing_scope_id = context.decl_name_stack().PeekParentScopeId();
+  if (TryAsInterfaceScope(context, enclosing_scope_id) ||
+      TryAsNamedConstraintScope(context, enclosing_scope_id)) {
+    DiagnoseDeclInInterfaceOrNamedConstraint(context, node_id,
+                                             Lex::TokenKind::Class);
+    // TODO: Make an ErrorInst somewhere.
+  }
+
   // Process modifiers.
   auto [_, parent_scope_inst] =
       context.name_scopes().GetInstIfValid(name_context.parent_scope_id);

toolchain/check/handle_export.cpp

@@ -7,9 +7,11 @@
 #include "toolchain/check/generic.h"
 #include "toolchain/check/handle.h"
 #include "toolchain/check/inst.h"
+#include "toolchain/check/interface.h"
 #include "toolchain/check/modifiers.h"
 #include "toolchain/check/name_component.h"
 #include "toolchain/check/name_lookup.h"
+#include "toolchain/check/name_scope.h"
 #include "toolchain/parse/typed_nodes.h"
 #include "toolchain/sem_ir/ids.h"
 #include "toolchain/sem_ir/typed_insts.h"
@@ -42,6 +44,14 @@ auto HandleParseNode(Context& context, Parse::ExportDeclId node_id) -> bool {
     return true;
   }
 
+  auto enclosing_scope_id = context.scope_stack().PeekNameScopeId();
+  if (TryAsInterfaceScope(context, enclosing_scope_id) ||
+      TryAsNamedConstraintScope(context, enclosing_scope_id)) {
+    DiagnoseDeclInInterfaceOrNamedConstraint(context, node_id,
+                                             Lex::TokenKind::Export);
+    return true;
+  }
+
   // Exporting uses the decl name primarily for lookup, so treat poisoning the
   // same as "not found".
   auto inst_id =

toolchain/check/handle_function.cpp

@@ -24,6 +24,7 @@
 #include "toolchain/check/modifiers.h"
 #include "toolchain/check/name_component.h"
 #include "toolchain/check/name_lookup.h"
+#include "toolchain/check/name_scope.h"
 #include "toolchain/check/type.h"
 #include "toolchain/check/type_completion.h"
 #include "toolchain/lex/token_kind.h"
@@ -249,7 +250,8 @@ static auto TryMergeRedecl(Context& context, Parse::AnyFunctionDeclId node_id,
 
 // Adds the declaration to name lookup when appropriate.
 static auto MaybeAddToNameLookup(
-    Context& context, const DeclNameStack::NameContext& name_context,
+    Context& context, SemIR::LocId loc_id,
+    const DeclNameStack::NameContext& name_context,
     const KeywordModifierSet& modifier_set,
     const std::optional<SemIR::Inst>& parent_scope_inst, SemIR::InstId decl_id)
     -> void {
@@ -258,6 +260,12 @@ static auto MaybeAddToNameLookup(
     return;
   }
 
+  if (parent_scope_inst &&
+      parent_scope_inst->Is<SemIR::NamedConstraintDecl>()) {
+    context.TODO(loc_id, "`fn` in named constraint");
+    return;
+  }
+
   // At interface scope, a function declaration introduces an associated
   // function.
   auto lookup_result_id = decl_id;
@@ -475,7 +483,7 @@ static auto BuildFunctionDecl(Context& context,
   }
 
   // Add to name lookup if needed, now that the decl is built.
-  MaybeAddToNameLookup(context, name_context, introducer.modifier_set,
+  MaybeAddToNameLookup(context, node_id, name_context, introducer.modifier_set,
                        parent_scope_inst, decl_id);
 
   ValidateForEntryPoint(context, node_id, function_decl.function_id,

toolchain/check/handle_impl.cpp

@@ -13,6 +13,7 @@
 #include "toolchain/check/handle.h"
 #include "toolchain/check/impl.h"
 #include "toolchain/check/inst.h"
+#include "toolchain/check/interface.h"
 #include "toolchain/check/modifiers.h"
 #include "toolchain/check/name_lookup.h"
 #include "toolchain/check/name_scope.h"
@@ -72,11 +73,19 @@ auto HandleParseNode(Context& context, Parse::ImplTypeAsId node_id) -> bool {
   auto [self_node, self_id] = context.node_stack().PopExprWithNodeId();
   auto self_type = ExprAsType(context, self_node, self_id);
 
+  auto enclosing_scope_id = context.decl_name_stack().PeekParentScopeId();
+  if (TryAsInterfaceScope(context, enclosing_scope_id) ||
+      TryAsNamedConstraintScope(context, enclosing_scope_id)) {
+    DiagnoseDeclInInterfaceOrNamedConstraint(context, node_id,
+                                             Lex::TokenKind::Impl);
+    context.node_stack().Push(node_id, SemIR::ErrorInst::TypeInstId);
+    return true;
+  }
+
   const auto& introducer = context.decl_introducer_state_stack().innermost();
   if (introducer.modifier_set.HasAnyOf(KeywordModifierSet::Extend)) {
     // TODO: Also handle the parent scope being a mixin.
-    if (auto class_scope = TryAsClassScope(
-            context, context.decl_name_stack().PeekParentScopeId())) {
+    if (auto class_scope = TryAsClassScope(context, enclosing_scope_id)) {
       // If we're not inside a class at all, that will be diagnosed against the
       // `extend` elsewhere.
       auto extend_node = introducer.modifier_node_id(ModifierOrder::Extend);
@@ -115,8 +124,8 @@ auto HandleParseNode(Context& context, Parse::ImplDefaultSelfAsId node_id)
     -> bool {
   auto self_inst_id = SemIR::TypeInstId::None;
 
-  if (auto class_scope = TryAsClassScope(
-          context, context.decl_name_stack().PeekParentScopeId())) {
+  auto enclosing_scope_id = context.decl_name_stack().PeekParentScopeId();
+  if (auto class_scope = TryAsClassScope(context, enclosing_scope_id)) {
     auto self_type_id = GetImplDefaultSelfType(context, *class_scope);
     // Build the implicit access to the enclosing `Self`.
     // TODO: Consider calling `HandleNameAsExpr` to build this implicit `Self`

toolchain/check/handle_interface.cpp

@@ -15,6 +15,7 @@
 #include "toolchain/check/modifiers.h"
 #include "toolchain/check/name_component.h"
 #include "toolchain/check/name_lookup.h"
+#include "toolchain/check/name_scope.h"
 #include "toolchain/check/type.h"
 #include "toolchain/sem_ir/entity_with_params_base.h"
 #include "toolchain/sem_ir/ids.h"
@@ -48,6 +49,14 @@ static auto BuildInterfaceDecl(Context& context,
   context.node_stack()
       .PopAndDiscardSoloNodeId<Parse::NodeKind::InterfaceIntroducer>();
 
+  auto enclosing_scope_id = context.decl_name_stack().PeekParentScopeId();
+  if (TryAsInterfaceScope(context, enclosing_scope_id) ||
+      TryAsNamedConstraintScope(context, enclosing_scope_id)) {
+    DiagnoseDeclInInterfaceOrNamedConstraint(context, node_id,
+                                             Lex::TokenKind::Interface);
+    // TODO: Produce an ErrorInst somewhere.
+  }
+
   // Process modifiers.
   auto [_, parent_scope_inst] =
       context.name_scopes().GetInstIfValid(name_context.parent_scope_id);

toolchain/check/handle_let_and_var.cpp

@@ -13,6 +13,7 @@
 #include "toolchain/check/interface.h"
 #include "toolchain/check/keyword_modifier_set.h"
 #include "toolchain/check/modifiers.h"
+#include "toolchain/check/name_scope.h"
 #include "toolchain/check/pattern.h"
 #include "toolchain/check/pattern_match.h"
 #include "toolchain/diagnostics/diagnostic_emitter.h"
@@ -207,7 +208,7 @@ struct DeclInfo {
 template <const Lex::TokenKind& IntroducerTokenKind,
           const Parse::NodeKind& IntroducerNodeKind,
           const Parse::NodeKind& InitializerNodeKind>
-static auto HandleDecl(Context& context) -> DeclInfo {
+static auto HandleDecl(Context& context, SemIR::LocId loc_id) -> DeclInfo {
   DeclInfo decl_info = DeclInfo();
 
   // Handle the optional initializer.
@@ -222,9 +223,13 @@ static auto HandleDecl(Context& context) -> DeclInfo {
     // For an associated constant declaration, handle the completed declaration
     // now. We will have done this at the `=` if there was an initializer.
     if (IntroducerNodeKind == Parse::NodeKind::AssociatedConstantIntroducer) {
-      auto interface_decl =
-          context.scope_stack().GetCurrentScopeAs<SemIR::InterfaceDecl>();
-      EndAssociatedConstantDeclRegion(context, interface_decl->interface_id);
+      if (auto interface_decl =
+              context.scope_stack().GetCurrentScopeAs<SemIR::InterfaceDecl>()) {
+        EndAssociatedConstantDeclRegion(context, interface_decl->interface_id);
+      } else {
+        DiscardGenericDecl(context);
+        context.TODO(loc_id, "associated constant in constraint");
+      }
     }
 
     EndFullPattern(context);
@@ -252,7 +257,7 @@ static auto HandleDecl(Context& context) -> DeclInfo {
 auto HandleParseNode(Context& context, Parse::LetDeclId node_id) -> bool {
   auto decl_info =
       HandleDecl<Lex::TokenKind::Let, Parse::NodeKind::LetIntroducer,
-                 Parse::NodeKind::LetInitializer>(context);
+                 Parse::NodeKind::LetInitializer>(context, node_id);
 
   LimitModifiersOnDecl(
       context, decl_info.introducer,
@@ -278,10 +283,10 @@ auto HandleParseNode(Context& context, Parse::LetDeclId node_id) -> bool {
 
 auto HandleParseNode(Context& context, Parse::AssociatedConstantDeclId node_id)
     -> bool {
-  auto decl_info =
-      HandleDecl<Lex::TokenKind::Let,
-                 Parse::NodeKind::AssociatedConstantIntroducer,
-                 Parse::NodeKind::AssociatedConstantInitializer>(context);
+  auto decl_info = HandleDecl<Lex::TokenKind::Let,
+                              Parse::NodeKind::AssociatedConstantIntroducer,
+                              Parse::NodeKind::AssociatedConstantInitializer>(
+      context, node_id);
 
   LimitModifiersOnDecl(
       context, decl_info.introducer,
@@ -333,11 +338,10 @@ auto HandleParseNode(Context& context, Parse::AssociatedConstantDeclId node_id)
   return true;
 }
 
-auto HandleParseNode(Context& context, Parse::VariableDeclId /*node_id*/)
-    -> bool {
+auto HandleParseNode(Context& context, Parse::VariableDeclId node_id) -> bool {
   auto decl_info =
       HandleDecl<Lex::TokenKind::Var, Parse::NodeKind::VariableIntroducer,
-                 Parse::NodeKind::VariableInitializer>(context);
+                 Parse::NodeKind::VariableInitializer>(context, node_id);
 
   LimitModifiersOnDecl(
       context, decl_info.introducer,

toolchain/check/handle_named_constraint.cpp

@@ -8,6 +8,7 @@
 #include "toolchain/check/inst.h"
 #include "toolchain/check/interface.h"
 #include "toolchain/check/modifiers.h"
+#include "toolchain/check/name_scope.h"
 #include "toolchain/check/type.h"
 #include "toolchain/sem_ir/ids.h"
 #include "toolchain/sem_ir/named_constraint.h"
@@ -43,6 +44,14 @@ static auto BuildNamedConstraintDecl(Context& context,
   // TODO: PopSoloNodeId(`template`) if it's present, and track that in the
   // NamedConstraint. Or maybe it should be a modifier, like `abstract class`?
 
+  auto enclosing_scope_id = context.decl_name_stack().PeekParentScopeId();
+  if (TryAsInterfaceScope(context, enclosing_scope_id) ||
+      TryAsNamedConstraintScope(context, enclosing_scope_id)) {
+    DiagnoseDeclInInterfaceOrNamedConstraint(context, node_id,
+                                             Lex::TokenKind::Constraint);
+    // TODO: Produce an ErrorInst somewhere.
+  }
+
   // Process modifiers.
   auto [_, parent_scope_inst] =
       context.name_scopes().GetInstIfValid(name_context.parent_scope_id);

toolchain/check/interface.cpp

@@ -350,4 +350,11 @@ template auto TryGetExistingDecl(Context& context, const NameComponent& name,
                                  bool is_definition)
     -> std::optional<SemIR::Inst>;
 
+auto DiagnoseDeclInInterfaceOrNamedConstraint(Context& context,
+                                              SemIR::LocId loc_id,
+                                              Lex::TokenKind tok) -> void {
+  context.TODO(loc_id,
+               llvm::formatv("`{0}` decl in interface or constraint", tok));
+}
+
 }  // namespace Carbon::Check

toolchain/check/interface.h

@@ -61,6 +61,10 @@ auto TryGetExistingDecl(Context& context, const NameComponent& name,
                         const EntityT& entity, bool is_definition)
     -> std::optional<SemIR::Inst>;
 
+auto DiagnoseDeclInInterfaceOrNamedConstraint(Context& context,
+                                              SemIR::LocId loc_id,
+                                              Lex::TokenKind tok) -> void;
+
 }  // namespace Carbon::Check
 
 #endif  // CARBON_TOOLCHAIN_CHECK_INTERFACE_H_

toolchain/check/name_scope.cpp

@@ -22,4 +22,38 @@ auto TryAsClassScope(Context& context, SemIR::NameScopeId scope_id)
   return {{.class_decl = *class_decl, .name_scope = &scope}};
 }
 
+auto TryAsInterfaceScope(Context& context, SemIR::NameScopeId scope_id)
+    -> std::optional<InterfaceScope> {
+  if (!scope_id.has_value()) {
+    return std::nullopt;
+  }
+  auto& scope = context.name_scopes().Get(scope_id);
+  if (!scope.inst_id().has_value()) {
+    return std::nullopt;
+  }
+  auto interface_decl =
+      context.insts().TryGetAs<SemIR::InterfaceDecl>(scope.inst_id());
+  if (!interface_decl) {
+    return std::nullopt;
+  }
+  return {{.interface_decl = *interface_decl, .name_scope = &scope}};
+}
+
+auto TryAsNamedConstraintScope(Context& context, SemIR::NameScopeId scope_id)
+    -> std::optional<NamedConstraintScope> {
+  if (!scope_id.has_value()) {
+    return std::nullopt;
+  }
+  auto& scope = context.name_scopes().Get(scope_id);
+  if (!scope.inst_id().has_value()) {
+    return std::nullopt;
+  }
+  auto constraint_decl =
+      context.insts().TryGetAs<SemIR::NamedConstraintDecl>(scope.inst_id());
+  if (!constraint_decl) {
+    return std::nullopt;
+  }
+  return {{.constraint_decl = *constraint_decl, .name_scope = &scope}};
+}
+
 }  // namespace Carbon::Check

toolchain/check/name_scope.h

@@ -7,6 +7,7 @@
 
 #include <optional>
 
+#include "common/concepts.h"
 #include "toolchain/check/context.h"
 #include "toolchain/sem_ir/ids.h"
 #include "toolchain/sem_ir/typed_insts.h"
@@ -23,6 +24,26 @@ struct ClassScope {
 auto TryAsClassScope(Context& context, SemIR::NameScopeId scope_id)
     -> std::optional<ClassScope>;
 
+struct InterfaceScope {
+  SemIR::InterfaceDecl interface_decl;
+  SemIR::NameScope* name_scope;
+};
+
+// If the specified name scope corresponds to a interface, returns the
+// corresponding interface declaration.
+auto TryAsInterfaceScope(Context& context, SemIR::NameScopeId scope_id)
+    -> std::optional<InterfaceScope>;
+
+struct NamedConstraintScope {
+  SemIR::NamedConstraintDecl constraint_decl;
+  SemIR::NameScope* name_scope;
+};
+
+// If the specified name scope corresponds to a named constraint, returns the
+// corresponding constraint declaration.
+auto TryAsNamedConstraintScope(Context& context, SemIR::NameScopeId scope_id)
+    -> std::optional<NamedConstraintScope>;
+
 }  // namespace Carbon::Check
 
 #endif  // CARBON_TOOLCHAIN_CHECK_NAME_SCOPE_H_

toolchain/check/scope_stack.h

@@ -105,6 +105,10 @@ class ScopeStack {
 
   // Returns the current scope, if it is of the specified kind. Otherwise,
   // returns nullopt.
+  //
+  // TODO: Consider adding TryAs*Scope functions in name_scope.h to replace use
+  // of this, as they could handle any NameScopeId instead of just the current
+  // one.
   template <typename InstT>
   auto GetCurrentScopeAs() -> std::optional<InstT> {
     auto inst_id = PeekInstId();