[torque] use LocationReference for local bindings

This is a first step towards allowing expressions for array sizes.

So far, local variable bindings used a VisitResult and a const flag.
This doesn't allow for local bindings to alias other things, like
heap references. While this is not generally a feature we need,
it will be helpful to create bindings when evaluating array sizes,
since we want to grant access to the preceding already initialized
object fields, but not to the whole object, which is not completely
initialized yet.

LocationReference already captures the notion of any readable and
assignable location, so it is a good fit to be used for local bindings.
The const attribute is no longer needed, since LocationReference already
has a notion of constness for stack ranges (that is,
LocationReference::Temporary vs LocationReference::VariableAccess).

Bug: v8:10004 v8:7793
Change-Id: Ibe0a43e898e5c2c10d6739e2496d92dda542e6cc
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1928852Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
Commit-Queue: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65117}

src/torque/implementation-visitor.cc

@@ -180,9 +180,8 @@ VisitResult ImplementationVisitor::InlineMacro(
   // than read operations.
   if (this_reference) {
     DCHECK(macro->IsMethod());
-    LocalValue this_value = LocalValue{!this_reference->IsVariableAccess(),
-                                       this_reference->GetVisitResult()};
-    parameter_bindings.Add(kThisParameterName, this_value, true);
+    parameter_bindings.Add(kThisParameterName, LocalValue{*this_reference},
+                           true);
   }
 
   size_t i = 0;
@@ -190,7 +189,10 @@ VisitResult ImplementationVisitor::InlineMacro(
     if (this_reference && i == signature.implicit_count) i++;
     const bool mark_as_used = signature.implicit_count > i;
     const Identifier* name = macro->parameter_names()[i++];
-    parameter_bindings.Add(name, LocalValue{true, arg}, mark_as_used);
+    parameter_bindings.Add(name,
+                           LocalValue{LocationReference::Temporary(
+                               arg, "parameter " + name->value)},
+                           mark_as_used);
   }
 
   DCHECK_EQ(label_blocks.size(), signature.labels.size());
@@ -391,8 +393,11 @@ std::string AddParameter(size_t i, Builtin* builtin,
   std::string external_name = "parameter" + std::to_string(i);
   parameters->Push(external_name);
   StackRange range = parameter_types->PushMany(LowerType(type));
-  parameter_bindings->Add(name, LocalValue{true, VisitResult(type, range)},
-                          mark_as_used);
+  parameter_bindings->Add(
+      name,
+      LocalValue{LocationReference::Temporary(VisitResult(type, range),
+                                              "parameter " + name->value)},
+      mark_as_used);
   return external_name;
 }
 
@@ -442,9 +447,11 @@ void ImplementationVisitor::Visit(Builtin* builtin) {
       parameters.Push("torque_arguments.length");
       const Type* arguments_type = TypeOracle::GetArgumentsType();
       StackRange range = parameter_types.PushMany(LowerType(arguments_type));
-      parameter_bindings.Add(
-          *signature.arguments_variable,
-          LocalValue{true, VisitResult(arguments_type, range)}, true);
+      parameter_bindings.Add(*signature.arguments_variable,
+                             LocalValue{LocationReference::Temporary(
+                                 VisitResult(arguments_type, range),
+                                 "parameter " + *signature.arguments_variable)},
+                             true);
     }
 
     for (size_t i = 0; i < signature.implicit_count; ++i) {
@@ -597,8 +604,11 @@ const Type* ImplementationVisitor::Visit(
     init_result =
         VisitResult(*type, assembler().TopRange(lowered_types.size()));
   }
-  block_bindings->Add(stmt->name,
-                      LocalValue{stmt->const_qualified, *init_result});
+  LocationReference ref = stmt->const_qualified
+                              ? LocationReference::Temporary(
+                                    *init_result, "const " + stmt->name->value)
+                              : LocationReference::VariableAccess(*init_result);
+  block_bindings->Add(stmt->name, LocalValue{std::move(ref)});
   return TypeOracle::GetVoidType();
 }
 
@@ -1145,8 +1155,10 @@ VisitResult ImplementationVisitor::Visit(TryLabelExpression* expr) {
   {
     BlockBindings<LocalValue> parameter_bindings(&ValueBindingsManager::Get());
     for (size_t i = 0; i < parameter_count; ++i) {
-      parameter_bindings.Add(expr->label_block->parameters.names[i],
-                             LocalValue{true, parameters[i]});
+      Identifier* name = expr->label_block->parameters.names[i];
+      parameter_bindings.Add(name,
+                             LocalValue{LocationReference::Temporary(
+                                 parameters[i], "parameter " + name->value)});
     }
 
     label_result = Visit(expr->label_block->body);
@@ -1930,8 +1942,12 @@ LocationReference ImplementationVisitor::GetLocationReference(
     const AggregateType* slice_type =
         AggregateType::cast(reference.heap_slice().type());
     Method* method = LookupMethod("AtIndex", slice_type, arguments, {});
+    // The reference has to be treated like a normal value when calling methods
+    // on the underlying slice implementation.
+    LocationReference slice_value = LocationReference::Temporary(
+        reference.GetVisitResult(), "slice as value");
     return LocationReference::HeapReference(
-        GenerateCall(method, reference, arguments, {}, false));
+        GenerateCall(method, std::move(slice_value), arguments, {}, false));
   } else {
     return LocationReference::ArrayAccess(GenerateFetchFromLocation(reference),
                                           index);
@@ -1951,11 +1967,12 @@ LocationReference ImplementationVisitor::GetLocationReference(
         ReportError("cannot have generic parameters on local name ",
                     expr->name);
       }
-      if ((*value)->is_const) {
-        return LocationReference::Temporary(
-            (*value)->value, "constant value " + expr->name->value);
+      const LocationReference& ref = (*value)->value;
+      if (ref.IsVariableAccess()) {
+        // Attach the binding to enable the never-assigned-to lint check.
+        return LocationReference::VariableAccess(ref.GetVisitResult(), *value);
       }
-      return LocationReference::VariableAccess((*value)->value, *value);
+      return ref;
     }
   }
 
@@ -2165,14 +2182,9 @@ VisitResult ImplementationVisitor::GenerateCall(
     }
   }
 
-  std::vector<VisitResult> converted_arguments;
-  StackRange argument_range = assembler().TopRange(0);
-  std::vector<std::string> constexpr_arguments;
-
-  size_t current = 0;
-  for (; current < callable->signature().implicit_count; ++current) {
-    std::string implicit_name =
-        callable->signature().parameter_names[current]->value;
+  std::vector<VisitResult> implicit_arguments;
+  for (size_t i = 0; i < callable->signature().implicit_count; ++i) {
+    std::string implicit_name = callable->signature().parameter_names[i]->value;
     base::Optional<Binding<LocalValue>*> val =
         TryLookupLocalValue(implicit_name);
     if (!val) {
@@ -2180,7 +2192,16 @@ VisitResult ImplementationVisitor::GenerateCall(
                   "' required for call to '", callable->ReadableName(),
                   "' is not defined");
     }
-    AddCallParameter(callable, (*val)->value,
+    implicit_arguments.push_back(GenerateFetchFromLocation((*val)->value));
+  }
+
+  std::vector<VisitResult> converted_arguments;
+  StackRange argument_range = assembler().TopRange(0);
+  std::vector<std::string> constexpr_arguments;
+
+  size_t current = 0;
+  for (; current < callable->signature().implicit_count; ++current) {
+    AddCallParameter(callable, implicit_arguments[current],
                      callable->signature().parameter_types.types[current],
                      &converted_arguments, &argument_range,
                      &constexpr_arguments);

src/torque/implementation-visitor.h

@@ -300,8 +300,7 @@ class BlockBindings {
 };
 
 struct LocalValue {
-  bool is_const;
-  VisitResult value;
+  LocationReference value;
 };
 
 struct LocalLabel {
@@ -321,7 +320,9 @@ template <>
 inline bool Binding<LocalValue>::CheckWritten() const {
   // Do the check only for non-const variables and non struct types.
   auto binding = *manager_->current_bindings_[name_];
-  return !binding->is_const && !binding->value.type()->IsStructType();
+  const LocationReference& ref = binding->value;
+  if (!ref.IsVariableAccess()) return false;
+  return !ref.GetVisitResult().type()->IsStructType();
 }
 template <>
 inline std::string Binding<LocalLabel>::BindingTypeString() const {