Prevent generic bindings in binding types (#2789)

Adds TODO #2788 for the bigger refactoring.

Fixes #2783

explorer/interpreter/type_checker.cpp

@@ -4131,7 +4131,7 @@ auto TypeChecker::TypeCheckWhereClause(Nonnull<WhereClause*> clause,
 }
 
 auto TypeChecker::TypeCheckPattern(
-    Nonnull<Pattern*> p, bool require_irrefutable,
+    Nonnull<Pattern*> p, PatternRequirements requirements,
     std::optional<Nonnull<const Value*>> expected, ImplScope& impl_scope,
     ExpressionCategory enclosing_expression_category) -> ErrorOr<Success> {
   if (trace_stream_->is_enabled()) {
@@ -4141,16 +4141,25 @@ auto TypeChecker::TypeCheckPattern(
     }
     *trace_stream_ << "\n";
   }
-  if (require_irrefutable) {
-    switch (p->kind()) {
-      case PatternKind::AutoPattern:
-      case PatternKind::ExpressionPattern:
+  switch (p->kind()) {
+    case PatternKind::AutoPattern:
+    case PatternKind::ExpressionPattern:
+      if (requirements == PatternRequirements::Irrefutable) {
         return ProgramError(p->source_loc())
                << "An irrefutable pattern is required, but `" << *p
                << "` is refutable.";
-      default:
-        break;
-    }
+      }
+      break;
+    case PatternKind::BindingPattern:
+    case PatternKind::GenericBinding:
+      if (requirements == PatternRequirements::BindingType) {
+        return ProgramError(p->source_loc())
+               << "Binding types cannot contain bindings, but `" << *p
+               << "` is a binding.";
+      }
+      break;
+    default:
+      break;
   }
   switch (p->kind()) {
     case PatternKind::AutoPattern: {
@@ -4160,15 +4169,9 @@ auto TypeChecker::TypeCheckPattern(
     }
     case PatternKind::BindingPattern: {
       auto& binding = cast<BindingPattern>(*p);
-      if (!VisitNestedPatterns(binding.type(), [](const Pattern& pattern) {
-            return !isa<BindingPattern>(pattern);
-          })) {
-        return ProgramError(binding.type().source_loc())
-               << "the type of a binding pattern cannot contain bindings";
-      }
       CARBON_RETURN_IF_ERROR(TypeCheckPattern(
-          &binding.type(), /*require_irrefutable=*/false, expected, impl_scope,
-          enclosing_expression_category));
+          &binding.type(), PatternRequirements::BindingType, expected,
+          impl_scope, enclosing_expression_category));
       Nonnull<const Value*> type = &binding.type().value();
       // Convert to a type.
       // TODO: Convert the pattern before interpreting it rather than doing
@@ -4246,7 +4249,7 @@ auto TypeChecker::TypeCheckPattern(
         if (expected) {
           expected_field_type = cast<TupleType>(**expected).elements()[i];
         }
-        CARBON_RETURN_IF_ERROR(TypeCheckPattern(field, require_irrefutable,
+        CARBON_RETURN_IF_ERROR(TypeCheckPattern(field, requirements,
                                                 expected_field_type, impl_scope,
                                                 enclosing_expression_category));
         if (trace_stream_->is_enabled()) {
@@ -4292,8 +4295,8 @@ auto TypeChecker::TypeCheckPattern(
           Substitute(choice_type.bindings(),
                      *(*signature)->parameters_static_type()));
       CARBON_RETURN_IF_ERROR(TypeCheckPattern(
-          &alternative.arguments(), require_irrefutable, parameter_type,
-          impl_scope, enclosing_expression_category));
+          &alternative.arguments(), requirements, parameter_type, impl_scope,
+          enclosing_expression_category));
       alternative.set_static_type(&choice_type);
       alternative.set_value(arena_->New<AlternativeValue>(
           &choice_type, *signature,
@@ -4313,9 +4316,9 @@ auto TypeChecker::TypeCheckPattern(
     case PatternKind::VarPattern: {
       auto& var_pattern = cast<VarPattern>(*p);
 
-      CARBON_RETURN_IF_ERROR(TypeCheckPattern(
-          &var_pattern.pattern(), require_irrefutable, expected, impl_scope,
-          var_pattern.expression_category()));
+      CARBON_RETURN_IF_ERROR(
+          TypeCheckPattern(&var_pattern.pattern(), requirements, expected,
+                           impl_scope, var_pattern.expression_category()));
       var_pattern.set_static_type(&var_pattern.pattern().static_type());
       var_pattern.set_value(&var_pattern.pattern().value());
       return Success();
@@ -4326,9 +4329,9 @@ auto TypeChecker::TypeCheckPattern(
       if (expected) {
         expected_ptr = arena_->New<PointerType>(expected.value());
       }
-      CARBON_RETURN_IF_ERROR(TypeCheckPattern(
-          &addr_pattern.binding(), require_irrefutable, expected_ptr,
-          impl_scope, enclosing_expression_category));
+      CARBON_RETURN_IF_ERROR(
+          TypeCheckPattern(&addr_pattern.binding(), requirements, expected_ptr,
+                           impl_scope, enclosing_expression_category));
 
       if (const auto* inner_binding_type =
               dyn_cast<PointerType>(&addr_pattern.binding().static_type())) {
@@ -4447,7 +4450,7 @@ auto TypeChecker::TypeCheckStmt(Nonnull<Statement*> s,
         // TODO: Should user-defined conversions be permitted in `match`
         // statements? When would we run them? See #1283.
         CARBON_RETURN_IF_ERROR(
-            TypeCheckPattern(&clause.pattern(), /*require_irrefutable=*/false,
+            TypeCheckPattern(&clause.pattern(), PatternRequirements::None,
                              &match.expression().static_type(), clause_scope,
                              ExpressionCategory::Value));
         if (expected_type.has_value()) {
@@ -4501,7 +4504,7 @@ auto TypeChecker::TypeCheckStmt(Nonnull<Statement*> s,
       const Value& rhs = for_stmt.loop_target().static_type();
       if (rhs.kind() == Value::Kind::StaticArrayType) {
         CARBON_RETURN_IF_ERROR(TypeCheckPattern(
-            &for_stmt.variable_declaration(), /*require_irrefutable=*/true,
+            &for_stmt.variable_declaration(), PatternRequirements::Irrefutable,
             &cast<StaticArrayType>(rhs).element_type(), inner_impl_scope,
             ExpressionCategory::Reference));
         CARBON_RETURN_IF_ERROR(ExpectExactType(
@@ -4548,7 +4551,7 @@ auto TypeChecker::TypeCheckStmt(Nonnull<Statement*> s,
         init_type = &var.init().static_type();
       }
       CARBON_RETURN_IF_ERROR(
-          TypeCheckPattern(&var.pattern(), /*require_irrefutable=*/true,
+          TypeCheckPattern(&var.pattern(), PatternRequirements::Irrefutable,
                            init_type, var_scope, var.expression_category()));
       CARBON_RETURN_IF_ERROR(ExpectCompleteType(
           var.source_loc(), "type of variable", &var.pattern().static_type()));
@@ -4755,23 +4758,23 @@ auto TypeChecker::DeclareCallableDeclaration(Nonnull<CallableDeclaration*> f,
   std::vector<Nonnull<const ImplBinding*>> impl_bindings;
   // Bring the deduced parameters into scope.
   for (Nonnull<GenericBinding*> deduced : f->deduced_parameters()) {
-    CARBON_RETURN_IF_ERROR(
-        TypeCheckPattern(deduced, /*require_irrefutable=*/true, std::nullopt,
-                         function_scope, ExpressionCategory::Value));
+    CARBON_RETURN_IF_ERROR(TypeCheckPattern(
+        deduced, PatternRequirements::Irrefutable, std::nullopt, function_scope,
+        ExpressionCategory::Value));
     CollectAndNumberGenericBindingsInPattern(deduced, all_bindings);
     CollectImplBindingsInPattern(deduced, impl_bindings);
   }
   // Type check the receiver pattern.
   if (f->is_method()) {
     CARBON_RETURN_IF_ERROR(TypeCheckPattern(
-        &f->self_pattern(), /*require_irrefutable=*/true, std::nullopt,
+        &f->self_pattern(), PatternRequirements::Irrefutable, std::nullopt,
         function_scope, ExpressionCategory::Value));
     CollectAndNumberGenericBindingsInPattern(&f->self_pattern(), all_bindings);
     CollectImplBindingsInPattern(&f->self_pattern(), impl_bindings);
   }
   // Type check the parameter pattern.
   CARBON_RETURN_IF_ERROR(TypeCheckPattern(
-      &f->param_pattern(), /*require_irrefutable=*/true, std::nullopt,
+      &f->param_pattern(), PatternRequirements::Irrefutable, std::nullopt,
       function_scope, ExpressionCategory::Value));
   CollectImplBindingsInPattern(&f->param_pattern(), impl_bindings);
 
@@ -4933,7 +4936,7 @@ auto TypeChecker::DeclareClassDeclaration(Nonnull<ClassDeclaration*> class_decl,
   if (class_decl->type_params().has_value()) {
     Nonnull<TuplePattern*> type_params = *class_decl->type_params();
     CARBON_RETURN_IF_ERROR(
-        TypeCheckPattern(type_params, /*require_irrefutable=*/true,
+        TypeCheckPattern(type_params, PatternRequirements::Irrefutable,
                          std::nullopt, class_scope, ExpressionCategory::Value));
     CollectAndNumberGenericBindingsInPattern(type_params, bindings);
     if (trace_stream_->is_enabled()) {
@@ -5102,9 +5105,9 @@ auto TypeChecker::DeclareMixinDeclaration(Nonnull<MixinDeclaration*> mixin_decl,
   ImplScope mixin_scope(scope_info.innermost_scope);
 
   if (mixin_decl->params().has_value()) {
-    CARBON_RETURN_IF_ERROR(
-        TypeCheckPattern(*mixin_decl->params(), /*require_irrefutable=*/true,
-                         std::nullopt, mixin_scope, ExpressionCategory::Value));
+    CARBON_RETURN_IF_ERROR(TypeCheckPattern(
+        *mixin_decl->params(), PatternRequirements::Irrefutable, std::nullopt,
+        mixin_scope, ExpressionCategory::Value));
     if (trace_stream_->is_enabled()) {
       *trace_stream_ << mixin_scope;
     }
@@ -5123,7 +5126,7 @@ auto TypeChecker::DeclareMixinDeclaration(Nonnull<MixinDeclaration*> mixin_decl,
 
   // Process the Self parameter.
   CARBON_RETURN_IF_ERROR(
-      TypeCheckPattern(mixin_decl->self(), /*require_irrefutable=*/true,
+      TypeCheckPattern(mixin_decl->self(), PatternRequirements::Irrefutable,
                        std::nullopt, mixin_scope, ExpressionCategory::Value));
 
   ScopeInfo mixin_scope_info = ScopeInfo::ForNonClassScope(&mixin_scope);
@@ -5233,8 +5236,8 @@ auto TypeChecker::DeclareConstraintTypeDeclaration(
   std::vector<Nonnull<const GenericBinding*>> bindings = scope_info.bindings;
   if (constraint_decl->params().has_value()) {
     CARBON_RETURN_IF_ERROR(TypeCheckPattern(
-        *constraint_decl->params(), /*require_irrefutable=*/true, std::nullopt,
-        constraint_scope, ExpressionCategory::Value));
+        *constraint_decl->params(), PatternRequirements::Irrefutable,
+        std::nullopt, constraint_scope, ExpressionCategory::Value));
     if (trace_stream_->is_enabled()) {
       *trace_stream_ << constraint_scope;
     }
@@ -5608,8 +5611,8 @@ auto TypeChecker::DeclareImplDeclaration(Nonnull<ImplDeclaration*> impl_decl,
   for (Nonnull<GenericBinding*> deduced : impl_decl->deduced_parameters()) {
     generic_bindings.push_back(deduced);
     CARBON_RETURN_IF_ERROR(
-        TypeCheckPattern(deduced, /*require_irrefutable=*/true, std::nullopt,
-                         impl_scope, ExpressionCategory::Value));
+        TypeCheckPattern(deduced, PatternRequirements::Irrefutable,
+                         std::nullopt, impl_scope, ExpressionCategory::Value));
     CollectImplBindingsInPattern(deduced, impl_bindings);
   }
   impl_decl->set_impl_bindings(impl_bindings);
@@ -5802,9 +5805,9 @@ auto TypeChecker::DeclareChoiceDeclaration(Nonnull<ChoiceDeclaration*> choice,
   std::vector<Nonnull<const GenericBinding*>> bindings = scope_info.bindings;
   if (choice->type_params().has_value()) {
     Nonnull<TuplePattern*> type_params = *choice->type_params();
-    CARBON_RETURN_IF_ERROR(TypeCheckPattern(
-        type_params, /*require_irrefutable=*/false, std::nullopt, choice_scope,
-        ExpressionCategory::Value));
+    CARBON_RETURN_IF_ERROR(
+        TypeCheckPattern(type_params, PatternRequirements::None, std::nullopt,
+                         choice_scope, ExpressionCategory::Value));
     CollectAndNumberGenericBindingsInPattern(type_params, bindings);
     if (trace_stream_->is_enabled()) {
       *trace_stream_ << choice_scope;
@@ -6128,7 +6131,7 @@ auto TypeChecker::DeclareDeclaration(Nonnull<Declaration*> d,
                << "Expected expression for variable type";
       }
       CARBON_RETURN_IF_ERROR(TypeCheckPattern(
-          &var.binding(), /*require_irrefutable=*/true, std::nullopt,
+          &var.binding(), PatternRequirements::Irrefutable, std::nullopt,
           *scope_info.innermost_scope, var.expression_category()));
       CARBON_RETURN_IF_ERROR(ExpectCompleteType(
           var.source_loc(), "type of variable", &var.binding().static_type()));

explorer/interpreter/type_checker.h

@@ -37,6 +37,21 @@ using GlobalMembersMap =
 
 class TypeChecker {
  public:
+  // Requirements for TypeCheckPattern.
+  enum class PatternRequirements {
+    None,
+
+    // The pattern must be fully irrefutable.
+    Irrefutable,
+
+    // A special case for types of bindings, which must be either `auto` or a
+    // valid expression.
+    // TODO: `auto` should be refactored from a pattern to an expression.
+    // Once that's done, this may be removable.
+    // https://github.com/carbon-language/carbon-lang/issues/2788
+    BindingType,
+  };
+
   explicit TypeChecker(Nonnull<Arena*> arena,
                        Nonnull<TraceStream*> trace_stream,
                        Nonnull<llvm::raw_ostream*> print_stream)
@@ -196,7 +211,7 @@ class TypeChecker {
   // Implicit conversions from `expected` to the pattern's type are permitted.
   //
   // `impl_scope` is extended with all implementations implied by the pattern.
-  auto TypeCheckPattern(Nonnull<Pattern*> p, bool require_irrefutable,
+  auto TypeCheckPattern(Nonnull<Pattern*> p, PatternRequirements requirements,
                         std::optional<Nonnull<const Value*>> expected,
                         ImplScope& impl_scope,
                         ExpressionCategory enclosing_expression_category)

explorer/testdata/var/local/fail_generic_binding_in_type.carbon

@@ -0,0 +1,15 @@
+// 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
+// RUN: %{not} %{explorer-run}
+// RUN: %{not} %{explorer-run-trace}
+
+package ExplorerTest api;
+
+fn Main() -> i32 {
+  // CHECK:STDERR: COMPILATION ERROR: {{.*}}/explorer/testdata/var/local/fail_generic_binding_in_type.carbon:[[@LINE+1]]: Binding types cannot contain bindings, but `T:! type` is a binding.
+  var x: (T:! type) = 1;
+  return 1;
+}

explorer/testdata/var/local/fail_nested_binding_in_type.carbon

@@ -9,7 +9,7 @@
 package ExplorerTest api;
 
 fn Main() -> i32 {
-  // CHECK:STDERR: COMPILATION ERROR: {{.*}}/explorer/testdata/var/local/fail_nested_binding_in_type.carbon:[[@LINE+1]]: the type of a binding pattern cannot contain bindings
+  // CHECK:STDERR: COMPILATION ERROR: {{.*}}/explorer/testdata/var/local/fail_nested_binding_in_type.carbon:[[@LINE+1]]: Binding types cannot contain bindings, but `T: type` is a binding.
   var x: (T: type) = 1;
   return 1;
 }