Clarify and simplify handling of lvalues (#956)

- Rename `PointerValue` to `LValue` to reflect how it's actually used. We can introduce a `PointerValue` type when we add support for actual pointer values.
- Remove support for pattern assignment. It's unclear if Carbon will support this, and even if we do, it raises questions that should first be addressed in a language proposal, like "is the left-hand side of `(x, y) = (1, 2)` an lvalue, or a pattern, or both, or something else entirely?"

executable_semantics/interpreter/action.h

@@ -132,8 +132,7 @@ class Action {
 };
 
 // An Action which implements evaluation of an Expression to produce an
-// lvalue. The result be expressed as a PointerValue which points to the
-// Expression's value.
+// LValue.
 class LValAction : public Action {
  public:
   explicit LValAction(Nonnull<const Expression*> expression)

executable_semantics/interpreter/interpreter.cpp

@@ -291,57 +291,6 @@ auto Interpreter::PatternMatch(Nonnull<const Value*> p, Nonnull<const Value*> v,
   }
 }
 
-void Interpreter::PatternAssignment(Nonnull<const Value*> pat,
-                                    Nonnull<const Value*> val,
-                                    SourceLocation source_loc) {
-  switch (pat->kind()) {
-    case Value::Kind::PointerValue:
-      heap_.Write(cast<PointerValue>(*pat).value(), val, source_loc);
-      break;
-    case Value::Kind::TupleValue: {
-      switch (val->kind()) {
-        case Value::Kind::TupleValue: {
-          const auto& pat_tup = cast<TupleValue>(*pat);
-          const auto& val_tup = cast<TupleValue>(*val);
-          if (pat_tup.elements().size() != val_tup.elements().size()) {
-            FATAL_RUNTIME_ERROR(source_loc)
-                << "arity mismatch in tuple pattern assignment:\n  pattern: "
-                << pat_tup << "\n  value: " << val_tup;
-          }
-          for (size_t i = 0; i < pat_tup.elements().size(); ++i) {
-            PatternAssignment(pat_tup.elements()[i], val_tup.elements()[i],
-                              source_loc);
-          }
-          break;
-        }
-        default:
-          FATAL() << "expected a tuple value on right-hand-side, not " << *val;
-      }
-      break;
-    }
-    case Value::Kind::AlternativeValue: {
-      switch (val->kind()) {
-        case Value::Kind::AlternativeValue: {
-          const auto& pat_alt = cast<AlternativeValue>(*pat);
-          const auto& val_alt = cast<AlternativeValue>(*val);
-          CHECK(val_alt.choice_name() == pat_alt.choice_name() &&
-                val_alt.alt_name() == pat_alt.alt_name())
-              << "internal error in pattern assignment";
-          PatternAssignment(&pat_alt.argument(), &val_alt.argument(),
-                            source_loc);
-          break;
-        }
-        default:
-          FATAL() << "expected an alternative in left-hand-side, not " << *val;
-      }
-      break;
-    }
-    default:
-      CHECK(ValueEqual(pat, val, source_loc))
-          << "internal error in pattern assignment";
-  }
-}
-
 void Interpreter::StepLvalue() {
   Action& act = todo_.CurrentAction();
   const Expression& exp = cast<LValAction>(act).expression();
@@ -355,7 +304,7 @@ void Interpreter::StepLvalue() {
       // -> { {E(x) :: C, E, F} :: S, H}
       Address pointer =
           GetFromEnv(exp.source_loc(), cast<IdentifierExpression>(exp).name());
-      Nonnull<const Value*> v = arena_->New<PointerValue>(pointer);
+      Nonnull<const Value*> v = arena_->New<LValue>(pointer);
       return todo_.FinishAction(v);
     }
     case ExpressionKind::FieldAccessExpression: {
@@ -367,10 +316,10 @@ void Interpreter::StepLvalue() {
       } else {
         //    { v :: [].f :: C, E, F} :: S, H}
         // -> { { &v.f :: C, E, F} :: S, H }
-        Address aggregate = cast<PointerValue>(*act.results()[0]).value();
+        Address aggregate = cast<LValue>(*act.results()[0]).address();
         Address field = aggregate.SubobjectAddress(
             cast<FieldAccessExpression>(exp).field());
-        return todo_.FinishAction(arena_->New<PointerValue>(field));
+        return todo_.FinishAction(arena_->New<LValue>(field));
       }
     }
     case ExpressionKind::IndexExpression: {
@@ -386,26 +335,14 @@ void Interpreter::StepLvalue() {
       } else {
         //    { v :: [][i] :: C, E, F} :: S, H}
         // -> { { &v[i] :: C, E, F} :: S, H }
-        Address aggregate = cast<PointerValue>(*act.results()[0]).value();
+        Address aggregate = cast<LValue>(*act.results()[0]).address();
         std::string f =
             std::to_string(cast<IntValue>(*act.results()[1]).value());
         Address field = aggregate.SubobjectAddress(f);
-        return todo_.FinishAction(arena_->New<PointerValue>(field));
-      }
-    }
-    case ExpressionKind::TupleLiteral: {
-      if (act.pos() <
-          static_cast<int>(cast<TupleLiteral>(exp).fields().size())) {
-        //    { { vk :: (f1=v1,..., fk=[],fk+1=ek+1,...) :: C, E, F} :: S,
-        //    H}
-        // -> { { ek+1 :: (f1=v1,..., fk=vk, fk+1=[],...) :: C, E, F} :: S,
-        // H}
-        return todo_.Spawn(std::make_unique<LValAction>(
-            cast<TupleLiteral>(exp).fields()[act.pos()]));
-      } else {
-        return todo_.FinishAction(arena_->New<TupleValue>(act.results()));
+        return todo_.FinishAction(arena_->New<LValue>(field));
       }
     }
+    case ExpressionKind::TupleLiteral:
     case ExpressionKind::StructLiteral:
     case ExpressionKind::StructTypeLiteral:
     case ExpressionKind::IntLiteral:
@@ -431,7 +368,7 @@ auto Interpreter::Convert(Nonnull<const Value*> value,
   switch (value->kind()) {
     case Value::Kind::IntValue:
     case Value::Kind::FunctionValue:
-    case Value::Kind::PointerValue:
+    case Value::Kind::LValue:
     case Value::Kind::BoolValue:
     case Value::Kind::NominalClassValue:
     case Value::Kind::AlternativeValue:
@@ -912,9 +849,10 @@ void Interpreter::StepStmt() {
       } else {
         //    { { v :: (a = []) :: C, E, F} :: S, H}
         // -> { { C, E, F} :: S, H(a := v)}
-        auto pat = act.results()[0];
-        auto val = Convert(act.results()[1], &assign.lhs().static_type());
-        PatternAssignment(pat, val, stmt.source_loc());
+        const auto& lval = cast<LValue>(*act.results()[0]);
+        Nonnull<const Value*> rval =
+            Convert(act.results()[1], &assign.lhs().static_type());
+        heap_.Write(lval.address(), rval, stmt.source_loc());
         return todo_.FinishAction();
       }
     }

executable_semantics/interpreter/interpreter.h

@@ -75,9 +75,6 @@ class Interpreter {
   auto EvalPrim(Operator op, const std::vector<Nonnull<const Value*>>& args,
                 SourceLocation source_loc) -> Nonnull<const Value*>;
 
-  void PatternAssignment(Nonnull<const Value*> pat, Nonnull<const Value*> val,
-                         SourceLocation source_loc);
-
   // Returns the result of converting `value` to type `destination_type`.
   auto Convert(Nonnull<const Value*> value,
                Nonnull<const Value*> destination_type) const

executable_semantics/interpreter/type_checker.cpp

@@ -111,7 +111,7 @@ static auto IsConcreteType(Nonnull<const Value*> value) -> bool {
   switch (value->kind()) {
     case Value::Kind::IntValue:
     case Value::Kind::FunctionValue:
-    case Value::Kind::PointerValue:
+    case Value::Kind::LValue:
     case Value::Kind::BoolValue:
     case Value::Kind::StructValue:
     case Value::Kind::NominalClassValue:
@@ -349,7 +349,7 @@ auto TypeChecker::ArgumentDeduction(SourceLocation source_loc, TypeEnv deduced,
     case Value::Kind::IntValue:
     case Value::Kind::BoolValue:
     case Value::Kind::FunctionValue:
-    case Value::Kind::PointerValue:
+    case Value::Kind::LValue:
     case Value::Kind::StructValue:
     case Value::Kind::NominalClassValue:
     case Value::Kind::AlternativeValue:
@@ -412,7 +412,7 @@ auto TypeChecker::Substitute(TypeEnv dict, Nonnull<const Value*> type)
     case Value::Kind::IntValue:
     case Value::Kind::BoolValue:
     case Value::Kind::FunctionValue:
-    case Value::Kind::PointerValue:
+    case Value::Kind::LValue:
     case Value::Kind::StructValue:
     case Value::Kind::NominalClassValue:
     case Value::Kind::AlternativeValue:

executable_semantics/interpreter/value.cpp

@@ -186,8 +186,8 @@ void Value::Print(llvm::raw_ostream& out) const {
     case Value::Kind::FunctionValue:
       out << "fun<" << cast<FunctionValue>(*this).declaration().name() << ">";
       break;
-    case Value::Kind::PointerValue:
-      out << "ptr<" << cast<PointerValue>(*this).value() << ">";
+    case Value::Kind::LValue:
+      out << "ptr<" << cast<LValue>(*this).address() << ">";
       break;
     case Value::Kind::BoolType:
       out << "Bool";
@@ -425,7 +425,7 @@ auto ValueEqual(Nonnull<const Value*> v1, Nonnull<const Value*> v2,
     case Value::Kind::BindingPlaceholderValue:
     case Value::Kind::AlternativeConstructorValue:
     case Value::Kind::ContinuationValue:
-    case Value::Kind::PointerValue:
+    case Value::Kind::LValue:
       // TODO: support pointer comparisons once we have a clearer distinction
       // between pointers and lvalues.
       FATAL() << "ValueEqual does not support this kind of value: " << *v1;

executable_semantics/interpreter/value.h

@@ -36,7 +36,7 @@ class Value {
   enum class Kind {
     IntValue,
     FunctionValue,
-    PointerValue,
+    LValue,
     BoolValue,
     StructValue,
     NominalClassValue,
@@ -132,17 +132,17 @@ class FunctionValue : public Value {
   Nonnull<const FunctionDeclaration*> declaration_;
 };
 
-// A pointer value.
-class PointerValue : public Value {
+// The value of a location in memory.
+class LValue : public Value {
  public:
-  explicit PointerValue(Address value)
-      : Value(Kind::PointerValue), value_(std::move(value)) {}
+  explicit LValue(Address value)
+      : Value(Kind::LValue), value_(std::move(value)) {}
 
   static auto classof(const Value* value) -> bool {
-    return value->kind() == Kind::PointerValue;
+    return value->kind() == Kind::LValue;
   }
 
-  auto value() const -> const Address& { return value_; }
+  auto address() const -> const Address& { return value_; }
 
  private:
   Address value_;

executable_semantics/testdata/tuple/assign.carbon

@@ -1,19 +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
-//
-// RUN: executable_semantics %s 2>&1 | \
-// RUN:   FileCheck --match-full-lines --allow-unused-prefixes=false %s
-// RUN: executable_semantics --trace %s 2>&1 | \
-// RUN:   FileCheck --match-full-lines --allow-unused-prefixes %s
-// AUTOUPDATE: executable_semantics %s
-// CHECK: result: 0
-
-package ExecutableSemanticsTest api;
-
-fn Main() -> i32 {
-  var x: auto = 0;
-  var y: auto = 1;
-  (x, y) = (5, -5);
-  return x + y;
-}