Add `addr self` binding support in destructors (#3117)

Refactors `CallDestructor` and `CallFunction` to both call a new method
`BindSelfIfPresent`, which includes support for binding `addr self` if
specified. Fixes the associated unit test.

Closes #2802.

explorer/interpreter/interpreter.cpp

@@ -182,8 +182,17 @@ class Interpreter {
                     std::optional<AllocationId> location_received)
       -> ErrorOr<Success>;
 
+  // Call the destructor method in `fun`, with any self argument bound to
+  // `receiver`.
   auto CallDestructor(Nonnull<const DestructorDeclaration*> fun,
-                      Nonnull<const Value*> receiver) -> ErrorOr<Success>;
+                      ExpressionResult receiver) -> ErrorOr<Success>;
+
+  // If the given method or destructor `decl` has a self argument, bind it to
+  // `receiver`.
+  void BindSelfIfPresent(Nonnull<const CallableDeclaration*> decl,
+                         ExpressionResult receiver, RuntimeScope& method_scope,
+                         BindingMap& generic_args,
+                         const SourceLocation& source_location);
 
   auto phase() const -> Phase { return phase_; }
 
@@ -848,39 +857,6 @@ auto Interpreter::Convert(Nonnull<const Value*> value,
   }
 }
 
-auto Interpreter::CallDestructor(Nonnull<const DestructorDeclaration*> fun,
-                                 Nonnull<const Value*> receiver)
-    -> ErrorOr<Success> {
-  const DestructorDeclaration& method = *fun;
-  CARBON_CHECK(method.is_method());
-  RuntimeScope method_scope(&heap_);
-  BindingMap generic_args;
-
-  // TODO: move this logic into PatternMatch, and call it here.
-  const auto* p = &method.self_pattern().value();
-  const auto* placeholder = dyn_cast<BindingPlaceholderValue>(p);
-  if (!placeholder) {
-    // TODO: Fix this, probably merging logic with CallFunction.
-    // https://github.com/carbon-language/carbon-lang/issues/2802
-    return ProgramError(fun->source_loc())
-           << "destructors currently don't support `addr self` bindings";
-  }
-  if (auto& value_node = placeholder->value_node()) {
-    if (value_node->expression_category() == ExpressionCategory::Value) {
-      method_scope.BindValue(*placeholder->value_node(), receiver);
-    } else {
-      CARBON_FATAL()
-          << "TODO: [self addr: Self*] destructors not implemented yet";
-    }
-  }
-  CARBON_CHECK(method.body().has_value())
-      << "Calling a method that's missing a body";
-
-  auto act = std::make_unique<StatementAction>(*method.body(), std::nullopt);
-  return todo_.Spawn(std::unique_ptr<Action>(std::move(act)),
-                     std::move(method_scope));
-}
-
 auto Interpreter::CallFunction(const CallExpression& call,
                                Nonnull<const Value*> fun,
                                Nonnull<const Value*> arg,
@@ -941,25 +917,9 @@ auto Interpreter::CallFunction(const CallExpression& call,
 
       // Bind the receiver to the `self` parameter, if there is one.
       if (const auto* method_val = dyn_cast<BoundMethodValue>(func_val)) {
-        CARBON_CHECK(function.is_method());
-        const auto* self_pattern = &function.self_pattern().value();
-        if (const auto* placeholder =
-                dyn_cast<BindingPlaceholderValue>(self_pattern)) {
-          // Immutable self with `[self: Self]`
-          // TODO: move this logic into PatternMatch
-          if (placeholder->value_node().has_value()) {
-            function_scope.BindValue(*placeholder->value_node(),
-                                     method_val->receiver());
-          }
-        } else {
-          // Mutable self with `[addr self: Self*]`
-          CARBON_CHECK(isa<AddrValue>(self_pattern));
-          bool success = PatternMatch(
-              self_pattern, ExpressionResult::Value(method_val->receiver()),
-              call.source_loc(), &function_scope, generic_args, trace_stream_,
-              this->arena_);
-          CARBON_CHECK(success) << "Failed to bind addr self";
-        }
+        BindSelfIfPresent(&function,
+                          ExpressionResult::Value(method_val->receiver()),
+                          function_scope, generic_args, call.source_loc());
       }
 
       CARBON_ASSIGN_OR_RETURN(
@@ -1013,6 +973,60 @@ auto Interpreter::CallFunction(const CallExpression& call,
   }
 }
 
+auto Interpreter::CallDestructor(Nonnull<const DestructorDeclaration*> fun,
+                                 ExpressionResult receiver)
+    -> ErrorOr<Success> {
+  const DestructorDeclaration& method = *fun;
+  CARBON_CHECK(method.is_method());
+
+  RuntimeScope method_scope(&heap_);
+  BindingMap generic_args;
+  BindSelfIfPresent(fun, receiver, method_scope, generic_args,
+                    SourceLocation::DiagnosticsIgnored());
+
+  CARBON_CHECK(method.body().has_value())
+      << "Calling a method that's missing a body";
+
+  auto act = std::make_unique<StatementAction>(*method.body(), std::nullopt);
+  return todo_.Spawn(std::unique_ptr<Action>(std::move(act)),
+                     std::move(method_scope));
+}
+
+void Interpreter::BindSelfIfPresent(Nonnull<const CallableDeclaration*> decl,
+                                    ExpressionResult receiver,
+                                    RuntimeScope& method_scope,
+                                    BindingMap& generic_args,
+                                    const SourceLocation& source_location) {
+  CARBON_CHECK(decl->is_method());
+  const auto* self_pattern = &decl->self_pattern().value();
+  if (const auto* placeholder =
+          dyn_cast<BindingPlaceholderValue>(self_pattern)) {
+    // Immutable self with `[self: Self]`
+    if (placeholder->value_node().has_value()) {
+      bool success =
+          PatternMatch(placeholder, receiver, source_location, &method_scope,
+                       generic_args, trace_stream_, this->arena_);
+      CARBON_CHECK(success) << "Failed to bind self";
+    }
+  } else {
+    // Mutable self with `[addr self: Self*]`
+    CARBON_CHECK(isa<AddrValue>(self_pattern));
+    ExpressionResult v = receiver;
+    // See if we need to make a LocationValue from the address provided in the
+    // ExpressionResult
+    if (receiver.value()->kind() != Value::Kind::LocationValue) {
+      CARBON_CHECK(receiver.expression_category() ==
+                   ExpressionCategory::Reference);
+      CARBON_CHECK(receiver.address().has_value());
+      v = ExpressionResult::Value(
+          arena_->New<LocationValue>(receiver.address().value()));
+    }
+    bool success = PatternMatch(self_pattern, v, source_location, &method_scope,
+                                generic_args, trace_stream_, this->arena_);
+    CARBON_CHECK(success) << "Failed to bind addr self";
+  }
+}
+
 // Returns true if the format string is okay to pass to formatv. This only
 // supports `{{` and `{N}` as special syntax.
 static auto ValidateFormatString(SourceLocation source_loc,
@@ -2395,7 +2409,9 @@ auto Interpreter::StepDestroy() -> ErrorOr<Success> {
       if (act.pos() == 0) {
         // Run the destructor, if there is one.
         if (auto destructor = class_decl.destructor()) {
-          return CallDestructor(*destructor, class_obj);
+          return CallDestructor(
+              *destructor, ExpressionResult::Reference(
+                               class_obj, destroy_act.location()->address()));
         } else {
           return todo_.RunAgain();
         }

explorer/testdata/destructor/fail_addr.carbon → explorer/testdata/destructor/self_addr.carbon

@@ -10,7 +10,6 @@ class A {
   destructor[addr self: Self*] {
     self->n += 1;
     Print("DESTRUCTOR A {0}", self->n);
-  // CHECK:STDERR: RUNTIME ERROR: fail_addr.carbon:[[@LINE+1]]: destructors currently don't support `addr self` bindings
   }
 
   var n: i32;
@@ -20,3 +19,6 @@ fn Main() -> i32 {
   var a: A = {.n = 2};
   return 0;
 }
+
+// CHECK:STDOUT: DESTRUCTOR A 3
+// CHECK:STDOUT: result: 0

explorer/testdata/trace/phase_execution.carbon

@@ -109,6 +109,8 @@ fn Main() -> i32 {
 // CHECK:STDOUT: <[] stack-pop:  ValueExpressionAction pos: 1 `{}.(interface ImplicitAs(T = class T).Convert)` results: [`bound_method<Convert>`]  (phase_execution.carbon:10)
 // CHECK:STDOUT: ->> step ExpressionAction pos: 1 `{}.(interface ImplicitAs(T = class T).Convert)()` results: [`bound_method<Convert>`]  scope: [] (phase_execution.carbon:10) --->
 // CHECK:STDOUT: -() calling function: bound_method<Convert>
+// CHECK:STDOUT: === match pattern `Placeholder<self>`
+// CHECK:STDOUT:     from value expression with value `{}`
 // CHECK:STDOUT: === match pattern `()`
 // CHECK:STDOUT:     from value expression with value `()`
 // CHECK:STDOUT: >[] stack-push: ScopeAction pos: 0  scope: [`self: Self`: `{}`] (None)