Keep parameters in scope throughout the entity that they parameterize. (#3671)

Previously, we created scopes for implicit parameter lists and tuple
patterns, but that meant that bindings went out of scope too soon. We
now keep them in scope until the end of the enclosing declaration. This
is accomplished by pushing a scope for parameters when we handle a name
that might have them, and then popping the scope again if it turns out
that there were no parameters.

For a case such as:

```carbon
fn A(T:! type).B(U:! type).F(x: T, y: U) {
  var z: T;
}
```

... we now have the following scopes in the stack:

-   A parameter scope containing `T`.
-   A class scope for `A(T:! type)`.
-   A parameter scope containing `U`.
-   A class scope for `A(T:! type).B(U:! type)`.
-   A parameter scope containing `x: T` and `y: U`.
-   A function body scope containing `z: T`.

The innermost scope when check processes a declaration of a function,
class, or similar is now often a parameter scope rather than the
enclosing scope in which the class or function is declared, so the
target scope is now passed explicitly into the modifier checking code
that wants to inspect that enclosing scope.

toolchain/check/context.h

@@ -180,6 +180,13 @@ class Context {
   // lexical_lookup_.
   auto PopScope() -> void;
 
+  // Pops the top scope from scope_stack_ if it contains no names.
+  auto PopScopeIfEmpty() -> void {
+    if (scope_stack_.back().names.empty()) {
+      PopScope();
+    }
+  }
+
   // Pops scopes until we return to the specified scope index.
   auto PopToScope(ScopeIndex index) -> void;
 
@@ -193,36 +200,29 @@ class Context {
     return current_scope().scope_id;
   }
 
-  auto GetCurrentScopeParseNode() const -> Parse::NodeId {
-    auto current_scope_inst_id = current_scope().scope_inst_id;
-    if (!current_scope_inst_id.is_valid()) {
-      return Parse::NodeId::Invalid;
-    }
-    return sem_ir_->insts().GetParseNode(current_scope_inst_id);
+  // Returns the instruction associated with the current scope, or Invalid if
+  // there is no such instruction, such as for a block scope.
+  auto current_scope_inst_id() const -> SemIR::InstId {
+    return current_scope().scope_inst_id;
   }
 
-  // Returns true if currently at file scope.
-  auto at_file_scope() const -> bool { return scope_stack_.size() == 1; }
-
-  // Returns true if the current scope is of the specified kind.
-  template <typename InstT>
-  auto CurrentScopeIs() -> bool {
-    auto current_scope_inst_id = current_scope().scope_inst_id;
-    if (!current_scope_inst_id.is_valid()) {
-      return false;
+  auto GetCurrentScopeParseNode() const -> Parse::NodeId {
+    auto inst_id = current_scope_inst_id();
+    if (!inst_id.is_valid()) {
+      return Parse::NodeId::Invalid;
     }
-    return sem_ir_->insts().Get(current_scope_inst_id).kind() == InstT::Kind;
+    return sem_ir_->insts().GetParseNode(inst_id);
   }
 
   // Returns the current scope, if it is of the specified kind. Otherwise,
   // returns nullopt.
   template <typename InstT>
   auto GetCurrentScopeAs() -> std::optional<InstT> {
-    auto current_scope_inst_id = current_scope().scope_inst_id;
-    if (!current_scope_inst_id.is_valid()) {
+    auto inst_id = current_scope_inst_id();
+    if (!inst_id.is_valid()) {
       return std::nullopt;
     }
-    return insts().Get(current_scope_inst_id).TryAs<InstT>();
+    return insts().TryGetAs<InstT>(inst_id);
   }
 
   // If there is no `returned var` in scope, sets the given instruction to be

toolchain/check/decl_name_stack.cpp

@@ -23,6 +23,9 @@ auto DeclNameStack::MakeUnqualifiedName(Parse::NodeId parse_node,
 
 auto DeclNameStack::PushScopeAndStartName() -> void {
   decl_name_stack_.push_back(MakeEmptyNameContext());
+
+  // Create a scope for any parameters introduced in this name.
+  context_->PushScope();
 }
 
 auto DeclNameStack::FinishName() -> NameContext {
@@ -139,6 +142,26 @@ auto DeclNameStack::ApplyNameQualifierTo(NameContext& name_context,
   }
 }
 
+// Push a scope corresponding to a name qualifier. For example, for
+//
+//   fn Class(T:! type).F(n: i32)
+//
+// we will push the scope for `Class(T:! type)` between the scope containing the
+// declaration of `T` and the scope containing the declaration of `n`.
+static auto PushNameQualifierScope(Context& context,
+                                   SemIR::InstId scope_inst_id,
+                                   SemIR::NameScopeId scope_id,
+                                   bool has_error = false) -> void {
+  // If the qualifier has no parameters, we don't need to keep around a
+  // parameter scope.
+  context.PopScopeIfEmpty();
+
+  context.PushScope(scope_inst_id, scope_id, has_error);
+
+  // Enter a parameter scope in case the qualified name itself has parameters.
+  context.PushScope();
+}
+
 auto DeclNameStack::UpdateScopeIfNeeded(NameContext& name_context) -> void {
   // This will only be reached for resolved instructions. We update the target
   // scope based on the resolved type.
@@ -150,7 +173,8 @@ auto DeclNameStack::UpdateScopeIfNeeded(NameContext& name_context) -> void {
       if (class_info.is_defined()) {
         name_context.state = NameContext::State::Resolved;
         name_context.target_scope_id = class_info.scope_id;
-        context_->PushScope(name_context.resolved_inst_id, class_info.scope_id);
+        PushNameQualifierScope(*context_, name_context.resolved_inst_id,
+                               class_info.scope_id);
       } else {
         name_context.state = NameContext::State::ResolvedNonScope;
       }
@@ -160,8 +184,8 @@ auto DeclNameStack::UpdateScopeIfNeeded(NameContext& name_context) -> void {
       auto scope_id = resolved_inst.As<SemIR::Namespace>().name_scope_id;
       name_context.state = NameContext::State::Resolved;
       name_context.target_scope_id = scope_id;
-      context_->PushScope(name_context.resolved_inst_id, scope_id,
-                          context_->name_scopes().Get(scope_id).has_error);
+      PushNameQualifierScope(*context_, name_context.resolved_inst_id, scope_id,
+                             context_->name_scopes().Get(scope_id).has_error);
       break;
     }
     default:

toolchain/check/handle_binding_pattern.cpp

@@ -127,6 +127,9 @@ auto HandleAnyBindingPattern(Context& context, Parse::NodeId parse_node,
       auto param_id =
           context.AddInst({name_node, SemIR::Param{cast_type_id, name_id}});
       auto bind_id = context.AddInst(make_bind_name(cast_type_id, param_id));
+      // TODO: Bindings should come into scope immediately in other contexts
+      // too.
+      context.AddNameToLookup(name_id, bind_id);
       context.node_stack().Push(parse_node, bind_id);
       break;
     }

toolchain/check/handle_class.cpp

@@ -36,7 +36,8 @@ static auto BuildClassDecl(Context& context, Parse::AnyClassDeclId parse_node)
       .PopAndDiscardSoloParseNode<Parse::NodeKind::ClassIntroducer>();
 
   // Process modifiers.
-  CheckAccessModifiersOnDecl(context, Lex::TokenKind::Class);
+  CheckAccessModifiersOnDecl(context, Lex::TokenKind::Class,
+                             name_context.target_scope_id);
   LimitModifiersOnDecl(context,
                        KeywordModifierSet::Class | KeywordModifierSet::Access,
                        Lex::TokenKind::Class);

toolchain/check/handle_function.cpp

@@ -37,32 +37,17 @@ auto HandleReturnType(Context& context, Parse::ReturnTypeId parse_node)
   return true;
 }
 
-static auto DiagnoseModifiers(Context& context) -> KeywordModifierSet {
-  Lex::TokenKind decl_kind = Lex::TokenKind::Fn;
-  CheckAccessModifiersOnDecl(context, decl_kind);
+static auto DiagnoseModifiers(Context& context,
+                              SemIR::NameScopeId target_scope_id)
+    -> KeywordModifierSet {
+  const Lex::TokenKind decl_kind = Lex::TokenKind::Fn;
+  CheckAccessModifiersOnDecl(context, decl_kind, target_scope_id);
   LimitModifiersOnDecl(context,
                        KeywordModifierSet::Access | KeywordModifierSet::Method |
                            KeywordModifierSet::Interface,
                        decl_kind);
-  // Rules for abstract, virtual, and impl, which are only allowed in classes.
-  if (auto class_decl = context.GetCurrentScopeAs<SemIR::ClassDecl>()) {
-    auto inheritance_kind =
-        context.classes().Get(class_decl->class_id).inheritance_kind;
-    if (inheritance_kind == SemIR::Class::Final) {
-      ForbidModifiersOnDecl(context, KeywordModifierSet::Virtual, decl_kind,
-                            " in a non-abstract non-base `class` definition",
-                            context.GetCurrentScopeParseNode());
-    }
-    if (inheritance_kind != SemIR::Class::Abstract) {
-      ForbidModifiersOnDecl(context, KeywordModifierSet::Abstract, decl_kind,
-                            " in a non-abstract `class` definition",
-                            context.GetCurrentScopeParseNode());
-    }
-  } else {
-    ForbidModifiersOnDecl(context, KeywordModifierSet::Method, decl_kind,
-                          " outside of a class");
-  }
-  RequireDefaultFinalOnlyInInterfaces(context, decl_kind);
+  CheckMethodModifiersOnFunction(context, target_scope_id);
+  RequireDefaultFinalOnlyInInterfaces(context, decl_kind, target_scope_id);
 
   return context.decl_state_stack().innermost().modifier_set;
 }
@@ -117,7 +102,7 @@ static auto BuildFunctionDecl(Context& context,
       .PopAndDiscardSoloParseNode<Parse::NodeKind::FunctionIntroducer>();
 
   // Process modifiers.
-  auto modifiers = DiagnoseModifiers(context);
+  auto modifiers = DiagnoseModifiers(context, name_context.target_scope_id);
   if (!!(modifiers & KeywordModifierSet::Access)) {
     context.TODO(context.decl_state_stack().innermost().saw_access_modifier,
                  "access modifier");
@@ -261,14 +246,6 @@ auto HandleFunctionDefinitionStart(Context& context,
           param_id, IncompleteTypeInFunctionParam,
           context.sem_ir().StringifyType(param.type_id()));
     });
-
-    if (auto fn_param = param.TryAs<SemIR::AnyBindName>()) {
-      context.AddNameToLookup(
-          context.bind_names().Get(fn_param->bind_name_id).name_id, param_id);
-    } else {
-      CARBON_FATAL() << "Unexpected kind of parameter in function definition "
-                     << param;
-    }
   }
 
   context.node_stack().Push(parse_node, function_id);

toolchain/check/handle_interface.cpp

@@ -36,7 +36,8 @@ static auto BuildInterfaceDecl(Context& context,
       .PopAndDiscardSoloParseNode<Parse::NodeKind::InterfaceIntroducer>();
 
   // Process modifiers.
-  CheckAccessModifiersOnDecl(context, Lex::TokenKind::Interface);
+  CheckAccessModifiersOnDecl(context, Lex::TokenKind::Interface,
+                             name_context.target_scope_id);
   LimitModifiersOnDecl(context, KeywordModifierSet::Access,
                        Lex::TokenKind::Interface);
 

toolchain/check/handle_let.cpp

@@ -31,8 +31,12 @@ auto HandleLetDecl(Context& context, Parse::LetDeclId parse_node) -> bool {
   context.node_stack()
       .PopAndDiscardSoloParseNode<Parse::NodeKind::LetIntroducer>();
   // Process declaration modifiers.
-  CheckAccessModifiersOnDecl(context, Lex::TokenKind::Let);
-  RequireDefaultFinalOnlyInInterfaces(context, Lex::TokenKind::Let);
+  // TODO: For a qualified `let` declaration, this should use the target scope
+  // of the name introduced in the declaration. See #2590.
+  CheckAccessModifiersOnDecl(context, Lex::TokenKind::Let,
+                             context.current_scope_id());
+  RequireDefaultFinalOnlyInInterfaces(context, Lex::TokenKind::Let,
+                                      context.current_scope_id());
   LimitModifiersOnDecl(
       context, KeywordModifierSet::Access | KeywordModifierSet::Interface,
       Lex::TokenKind::Let);

toolchain/check/handle_pattern_list.cpp

@@ -9,7 +9,6 @@ namespace Carbon::Check {
 auto HandleImplicitParamListStart(Context& context,
                                   Parse::ImplicitParamListStartId parse_node)
     -> bool {
-  context.PushScope();
   context.node_stack().Push(parse_node);
   context.ParamOrArgStart();
   return true;
@@ -28,18 +27,6 @@ auto HandleImplicitParamList(Context& context,
 
 auto HandleTuplePatternStart(Context& context,
                              Parse::TuplePatternStartId parse_node) -> bool {
-  // A tuple pattern following an implicit parameter list shares the same
-  // scope.
-  //
-  // TODO: For a declaration like
-  //
-  //   fn A(T:! type).B(U:! T).C(x: X(U)) { ... }
-  //
-  // ... all the earlier parameter should be in scope in the later parameter
-  // lists too.
-  if (!context.node_stack().PeekIs<Parse::NodeKind::ImplicitParamList>()) {
-    context.PushScope();
-  }
   context.node_stack().Push(parse_node);
   context.ParamOrArgStart();
   return true;
@@ -54,7 +41,6 @@ auto HandlePatternListComma(Context& context,
 auto HandleTuplePattern(Context& context, Parse::TuplePatternId parse_node)
     -> bool {
   auto refs_id = context.ParamOrArgEnd(Parse::NodeKind::TuplePatternStart);
-  context.PopScope();
   context.node_stack()
       .PopAndDiscardSoloParseNode<Parse::NodeKind::TuplePatternStart>();
   context.node_stack().Push(parse_node, refs_id);

toolchain/check/handle_variable.cpp

@@ -82,7 +82,10 @@ auto HandleVariableDecl(Context& context, Parse::VariableDeclId parse_node)
       .PopAndDiscardSoloParseNode<Parse::NodeKind::VariableIntroducer>();
 
   // Process declaration modifiers.
-  CheckAccessModifiersOnDecl(context, Lex::TokenKind::Var);
+  // TODO: For a qualified `var` declaration, this should use the target scope
+  // of the name introduced in the declaration. See #2590.
+  CheckAccessModifiersOnDecl(context, Lex::TokenKind::Var,
+                             context.current_scope_id());
   LimitModifiersOnDecl(context, KeywordModifierSet::Access,
                        Lex::TokenKind::Var);
   auto modifiers = context.decl_state_stack().innermost().modifier_set;

toolchain/check/modifiers.cpp

@@ -62,16 +62,41 @@ auto ForbidModifiersOnDecl(Context& context, KeywordModifierSet forbidden,
   s.modifier_set = s.modifier_set & ~forbidden;
 }
 
-auto CheckAccessModifiersOnDecl(Context& context, Lex::TokenKind decl_kind)
-    -> void {
-  if (context.at_file_scope()) {
+// Returns the instruction that owns the given scope, or Invalid if the scope is
+// not associated with an instruction.
+auto GetScopeInstId(Context& context, SemIR::NameScopeId scope_id)
+    -> SemIR::InstId {
+  if (!scope_id.is_valid()) {
+    return SemIR::InstId::Invalid;
+  }
+  return context.name_scopes().Get(scope_id).inst_id;
+}
+
+// Returns the instruction that owns the given scope, or Invalid if the scope is
+// not associated with an instruction.
+auto GetScopeInst(Context& context, SemIR::NameScopeId scope_id)
+    -> std::optional<SemIR::Inst> {
+  auto inst_id = GetScopeInstId(context, scope_id);
+  if (!inst_id.is_valid()) {
+    return std::nullopt;
+  }
+  return context.insts().Get(inst_id);
+}
+
+auto CheckAccessModifiersOnDecl(Context& context, Lex::TokenKind decl_kind,
+                                SemIR::NameScopeId target_scope_id) -> void {
+  auto target = GetScopeInst(context, target_scope_id);
+  if (target && target->Is<SemIR::Namespace>()) {
+    // TODO: This assumes that namespaces can only be declared at file scope. If
+    // we add support for non-file-scope namespaces, we will need to check the
+    // parents of the target scope to determine whether we're at file scope.
     ForbidModifiersOnDecl(
         context, KeywordModifierSet::Protected, decl_kind,
         " at file scope, `protected` is only allowed on class members");
     return;
   }
 
-  if (context.CurrentScopeIs<SemIR::ClassDecl>()) {
+  if (target && target->Is<SemIR::ClassDecl>()) {
     // Both `private` and `protected` allowed in a class definition.
     return;
   }
@@ -84,9 +109,41 @@ auto CheckAccessModifiersOnDecl(Context& context, Lex::TokenKind decl_kind)
       ", `private` is only allowed on class members and at file scope");
 }
 
+// Rules for abstract, virtual, and impl, which are only allowed in classes.
+auto CheckMethodModifiersOnFunction(Context& context,
+                                    SemIR::NameScopeId target_scope_id)
+    -> void {
+  const Lex::TokenKind decl_kind = Lex::TokenKind::Fn;
+  auto target_id = GetScopeInstId(context, target_scope_id);
+  if (target_id.is_valid()) {
+    if (auto class_decl =
+            context.insts().TryGetAs<SemIR::ClassDecl>(target_id)) {
+      auto inheritance_kind =
+          context.classes().Get(class_decl->class_id).inheritance_kind;
+      if (inheritance_kind == SemIR::Class::Final) {
+        ForbidModifiersOnDecl(context, KeywordModifierSet::Virtual, decl_kind,
+                              " in a non-abstract non-base `class` definition",
+                              context.insts().GetParseNode(target_id));
+      }
+      if (inheritance_kind != SemIR::Class::Abstract) {
+        ForbidModifiersOnDecl(context, KeywordModifierSet::Abstract, decl_kind,
+                              " in a non-abstract `class` definition",
+                              context.insts().GetParseNode(target_id));
+      }
+      return;
+    }
+  }
+
+  ForbidModifiersOnDecl(context, KeywordModifierSet::Method, decl_kind,
+                        " outside of a class");
+}
+
 auto RequireDefaultFinalOnlyInInterfaces(Context& context,
-                                         Lex::TokenKind decl_kind) -> void {
-  if (context.CurrentScopeIs<SemIR::InterfaceDecl>()) {
+                                         Lex::TokenKind decl_kind,
+                                         SemIR::NameScopeId target_scope_id)
+    -> void {
+  auto target = GetScopeInst(context, target_scope_id);
+  if (target && target->Is<SemIR::InterfaceDecl>()) {
     // Both `default` and `final` allowed in an interface definition.
     return;
   }

toolchain/check/modifiers.h

@@ -9,10 +9,21 @@
 
 namespace Carbon::Check {
 
-// Reports a diagnostic (using `decl_name`) if access control modifiers on this
-// are not allowed, and updates the declaration state in `context`.
-auto CheckAccessModifiersOnDecl(Context& context, Lex::TokenKind decl_kind)
-    -> void;
+// Reports a diagnostic if access control modifiers on this are not allowed for
+// a declaration in `target_scope_id`, and updates the declaration state in
+// `context`.
+//
+// `target_scope_id` may be Invalid for a declaration in a block scope.
+auto CheckAccessModifiersOnDecl(Context& context, Lex::TokenKind decl_kind,
+                                SemIR::NameScopeId target_scope_id) -> void;
+
+// Reports a diagnostic if the method function modifiers `abstract`, `virtual`,
+// or `impl` are present but not permitted on a function declaration in
+// `target_scope_id`.
+//
+// `target_scope_id` may be Invalid for a declaration in a block scope.
+auto CheckMethodModifiersOnFunction(Context& context,
+                                    SemIR::NameScopeId target_scope_id) -> void;
 
 // Reports a diagnostic (using `decl_kind`) if modifiers on this declaration are
 // not in `allowed`. Updates the declaration state in
@@ -32,8 +43,12 @@ auto ForbidModifiersOnDecl(Context& context, KeywordModifierSet forbidden,
 // Report a diagonostic if `default` and `final` modifiers are used on
 // declarations where they are not allowed. Right now they are only allowed
 // inside interfaces.
+//
+// `target_scope_id` may be Invalid for a declaration in a block scope.
 auto RequireDefaultFinalOnlyInInterfaces(Context& context,
-                                         Lex::TokenKind decl_kind) -> void;
+                                         Lex::TokenKind decl_kind,
+                                         SemIR::NameScopeId target_scope_id)
+    -> void;
 
 }  // namespace Carbon::Check
 

toolchain/check/testdata/class/fail_todo_generic_method.carbon

@@ -0,0 +1,53 @@
+// 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
+
+// CHECK:STDERR: fail_todo_generic_method.carbon:[[@LINE+3]]:1: ERROR: Semantics TODO: `generic class`.
+// CHECK:STDERR: class Class(T:! type) {
+// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~
+class Class(T:! type) {
+  var a: T;
+  fn F[self: Self](n: T);
+}
+
+// CHECK:STDERR: fail_todo_generic_method.carbon:[[@LINE+9]]:1: ERROR: Duplicate name being declared in the same scope.
+// CHECK:STDERR: fn Class(T:! type).F[self: Self](n: T) {}
+// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+// CHECK:STDERR: fail_todo_generic_method.carbon:[[@LINE-8]]:1: Name is previously declared here.
+// CHECK:STDERR: class Class(T:! type) {
+// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~
+// CHECK:STDERR: fail_todo_generic_method.carbon:[[@LINE+3]]:19: ERROR: `fn` declarations must either end with a `;` or have a `{ ... }` block for a definition.
+// CHECK:STDERR: fn Class(T:! type).F[self: Self](n: T) {}
+// CHECK:STDERR:                   ^
+fn Class(T:! type).F[self: Self](n: T) {}
+
+// CHECK:STDOUT: --- fail_todo_generic_method.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %Class: type = class_type @Class [template]
+// CHECK:STDOUT:   %.1: type = unbound_element_type Class, T [symbolic]
+// CHECK:STDOUT:   %.2: type = struct_type {.a: T} [symbolic]
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace {.Class = %Class.decl} [template]
+// CHECK:STDOUT:   %Class.decl = class_decl @Class, (<unexpected instref inst+1>, <unexpected instref inst+2>)
+// CHECK:STDOUT:   %.loc24: <function> = fn_decl @.1 [template]
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @Class {
+// CHECK:STDOUT:   %T.ref: type = name_ref T, <unexpected instref inst+2> [symbolic = <unexpected instref inst+2>]
+// CHECK:STDOUT:   %.loc11: <unbound element of class Class> = field_decl a, element0 [template]
+// CHECK:STDOUT:   %F: <function> = fn_decl @F [template]
+// CHECK:STDOUT:
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT:   .a = %.loc11
+// CHECK:STDOUT:   .F = %F
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @F[%self: Class](%n: T);
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @.1(%T: type);
+// CHECK:STDOUT:

toolchain/check/testdata/function/declaration/fail_param_in_type.carbon

@@ -0,0 +1,20 @@
+// 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
+
+// CHECK:STDERR: fail_param_in_type.carbon:[[@LINE+3]]:23: ERROR: Array bound is not a constant.
+// CHECK:STDERR: fn F(n: i32, a: [i32; n]*);
+// CHECK:STDERR:                       ^
+fn F(n: i32, a: [i32; n]*);
+
+// CHECK:STDOUT: --- fail_param_in_type.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace {.F = %F} [template]
+// CHECK:STDOUT:   %F: <function> = fn_decl @F [template]
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @F(%n: i32, %a: <error>);
+// CHECK:STDOUT:

toolchain/check/testdata/function/declaration/fail_param_redecl.carbon

@@ -0,0 +1,23 @@
+// 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
+
+// CHECK:STDERR: fail_param_redecl.carbon:[[@LINE+6]]:14: ERROR: Duplicate name being declared in the same scope.
+// CHECK:STDERR: fn F(n: i32, n: i32);
+// CHECK:STDERR:              ^
+// CHECK:STDERR: fail_param_redecl.carbon:[[@LINE+3]]:6: Name is previously declared here.
+// CHECK:STDERR: fn F(n: i32, n: i32);
+// CHECK:STDERR:      ^
+fn F(n: i32, n: i32);
+
+// CHECK:STDOUT: --- fail_param_redecl.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace {.F = %F} [template]
+// CHECK:STDOUT:   %F: <function> = fn_decl @F [template]
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @F(%n.loc13_6: i32, %n.loc13_14: i32);
+// CHECK:STDOUT:

toolchain/check/testdata/function/declaration/param_same_name.carbon

@@ -0,0 +1,22 @@
+// 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
+
+fn F(a: i32);
+
+fn G(a: i32);
+
+// CHECK:STDOUT: --- param_same_name.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace {.F = %F, .G = %G} [template]
+// CHECK:STDOUT:   %F: <function> = fn_decl @F [template]
+// CHECK:STDOUT:   %G: <function> = fn_decl @G [template]
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @F(%a: i32);
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @G(%a: i32);
+// CHECK:STDOUT:

toolchain/check/testdata/function/definition/fail_param_name_conflict.carbon

@@ -1,26 +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
-
-// CHECK:STDERR: fail_param_name_conflict.carbon:[[@LINE+6]]:16: ERROR: Duplicate name being declared in the same scope.
-// CHECK:STDERR: fn Bar(a: i32, a: i32) {}
-// CHECK:STDERR:                ^
-// CHECK:STDERR: fail_param_name_conflict.carbon:[[@LINE+3]]:8: Name is previously declared here.
-// CHECK:STDERR: fn Bar(a: i32, a: i32) {}
-// CHECK:STDERR:        ^
-fn Bar(a: i32, a: i32) {}
-
-// CHECK:STDOUT: --- fail_param_name_conflict.carbon
-// CHECK:STDOUT:
-// CHECK:STDOUT: file {
-// CHECK:STDOUT:   package: <namespace> = namespace {.Bar = %Bar} [template]
-// CHECK:STDOUT:   %Bar: <function> = fn_decl @Bar [template]
-// CHECK:STDOUT: }
-// CHECK:STDOUT:
-// CHECK:STDOUT: fn @Bar(%a.loc13_8: i32, %a.loc13_16: i32) {
-// CHECK:STDOUT: !entry:
-// CHECK:STDOUT:   return
-// CHECK:STDOUT: }
-// CHECK:STDOUT:

toolchain/check/testdata/function/definition/same_param_name.carbon

@@ -1,27 +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
-
-fn Foo(a: i32) {}
-fn Bar(a: i32) {}
-
-// CHECK:STDOUT: --- same_param_name.carbon
-// CHECK:STDOUT:
-// CHECK:STDOUT: file {
-// CHECK:STDOUT:   package: <namespace> = namespace {.Foo = %Foo, .Bar = %Bar} [template]
-// CHECK:STDOUT:   %Foo: <function> = fn_decl @Foo [template]
-// CHECK:STDOUT:   %Bar: <function> = fn_decl @Bar [template]
-// CHECK:STDOUT: }
-// CHECK:STDOUT:
-// CHECK:STDOUT: fn @Foo(%a: i32) {
-// CHECK:STDOUT: !entry:
-// CHECK:STDOUT:   return
-// CHECK:STDOUT: }
-// CHECK:STDOUT:
-// CHECK:STDOUT: fn @Bar(%a: i32) {
-// CHECK:STDOUT: !entry:
-// CHECK:STDOUT:   return
-// CHECK:STDOUT: }
-// CHECK:STDOUT:

toolchain/check/testdata/function/generic/fail_todo_param_in_type.carbon

@@ -0,0 +1,20 @@
+// 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
+
+// CHECK:STDERR: fail_todo_param_in_type.carbon:[[@LINE+3]]:24: ERROR: Semantics TODO: `symbolic array bound`.
+// CHECK:STDERR: fn F(N:! i32, a: [i32; N]*);
+// CHECK:STDERR:                        ^
+fn F(N:! i32, a: [i32; N]*);
+
+// CHECK:STDOUT: --- fail_todo_param_in_type.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace {.F = %F} [template]
+// CHECK:STDOUT:   %F: <function> = fn_decl @F [template]
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @F(%N: i32, %a: <error>);
+// CHECK:STDOUT:

toolchain/check/testdata/function/generic/type_param_scope.carbon

@@ -0,0 +1,27 @@
+// 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
+
+fn F(T:! type, n: T) -> T {
+  let m: T = n;
+  return m;
+}
+
+// CHECK:STDOUT: --- type_param_scope.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace {.F = %F} [template]
+// CHECK:STDOUT:   %F: <function> = fn_decl @F [template]
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @F(%T: type, %n: T) -> T {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   %T.ref: type = name_ref T, %T [symbolic = %T]
+// CHECK:STDOUT:   %n.ref: T = name_ref n, %n
+// CHECK:STDOUT:   %m: T = bind_name m, %n.ref
+// CHECK:STDOUT:   %m.ref: T = name_ref m, %m
+// CHECK:STDOUT:   return %m.ref
+// CHECK:STDOUT: }
+// CHECK:STDOUT:

toolchain/check/testdata/let/fail_duplicate_decl.carbon

@@ -4,20 +4,22 @@
 //
 // AUTOUPDATE
 
-fn F(a: i32) {
+fn F() {
+  let a: i32 = 1;
   // CHECK:STDERR: fail_duplicate_decl.carbon:[[@LINE+6]]:7: ERROR: Duplicate name being declared in the same scope.
+  // CHECK:STDERR:   let a: i32 = 2;
+  // CHECK:STDERR:       ^
+  // CHECK:STDERR: fail_duplicate_decl.carbon:[[@LINE-4]]:7: Name is previously declared here.
   // CHECK:STDERR:   let a: i32 = 1;
   // CHECK:STDERR:       ^
-  // CHECK:STDERR: fail_duplicate_decl.carbon:[[@LINE-4]]:6: Name is previously declared here.
-  // CHECK:STDERR: fn F(a: i32) {
-  // CHECK:STDERR:      ^
-  let a: i32 = 1;
+  let a: i32 = 2;
 }
 
 // CHECK:STDOUT: --- fail_duplicate_decl.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {
 // CHECK:STDOUT:   %.1: i32 = int_literal 1 [template]
+// CHECK:STDOUT:   %.2: i32 = int_literal 2 [template]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
@@ -25,10 +27,12 @@ fn F(a: i32) {
 // CHECK:STDOUT:   %F: <function> = fn_decl @F [template]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @F(%a.loc7: i32) {
+// CHECK:STDOUT: fn @F() {
 // CHECK:STDOUT: !entry:
-// CHECK:STDOUT:   %.loc14: i32 = int_literal 1 [template = constants.%.1]
-// CHECK:STDOUT:   %a.loc14: i32 = bind_name a, %.loc14
+// CHECK:STDOUT:   %.loc8: i32 = int_literal 1 [template = constants.%.1]
+// CHECK:STDOUT:   %a.loc8: i32 = bind_name a, %.loc8
+// CHECK:STDOUT:   %.loc15: i32 = int_literal 2 [template = constants.%.2]
+// CHECK:STDOUT:   %a.loc15: i32 = bind_name a, %.loc15
 // CHECK:STDOUT:   return
 // CHECK:STDOUT: }
 // CHECK:STDOUT:

toolchain/check/testdata/let/shadowed_decl.carbon

@@ -0,0 +1,31 @@
+// 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
+
+fn F(a: i32) -> i32 {
+  let a: i32 = 1;
+  // TODO: Reject this due to ambiguity.
+  return a;
+}
+
+// CHECK:STDOUT: --- shadowed_decl.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %.1: i32 = int_literal 1 [template]
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace {.F = %F} [template]
+// CHECK:STDOUT:   %F: <function> = fn_decl @F [template]
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @F(%a.loc7: i32) -> i32 {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   %.loc8: i32 = int_literal 1 [template = constants.%.1]
+// CHECK:STDOUT:   %a.loc8: i32 = bind_name a, %.loc8
+// CHECK:STDOUT:   %a.ref: i32 = name_ref a, %a.loc8
+// CHECK:STDOUT:   return %a.ref
+// CHECK:STDOUT: }
+// CHECK:STDOUT: