[torque] ensure array lengths are const

To ensure good error messages, we do create bindings even for non-const
fields but then add a new error message mechanism when accessing such
a binding.

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

src/objects/debug-objects.tq

@@ -66,6 +66,6 @@ struct CoverageInfoSlot {
 // not contain any HeapObject fields.
 @generateCppClass
 extern class CoverageInfo extends HeapObject {
-  slot_count: int32;
+  const slot_count: int32;
   slots[slot_count]: CoverageInfoSlot;
 }

src/objects/descriptor-array.tq

@@ -17,7 +17,7 @@ struct DescriptorEntry {
 }
 
 extern class DescriptorArray extends HeapObject {
-  number_of_all_descriptors: uint16;
+  const number_of_all_descriptors: uint16;
   number_of_descriptors: uint16;
   raw_number_of_marked_descriptors: uint16;
   filler16_bits: uint16;

src/objects/fixed-array.tq

@@ -4,7 +4,7 @@
 
 @abstract
 extern class FixedArrayBase extends HeapObject {
-  length: Smi;
+  const length: Smi;
 }
 
 extern class FixedArray extends FixedArrayBase { objects[length]: Object; }
@@ -29,9 +29,9 @@ extern class TemplateList extends FixedArray {
 }
 
 extern class WeakArrayList extends HeapObject {
-  capacity: Smi;
+  const capacity: Smi;
   length: Smi;
-  objects[length]: MaybeObject;
+  objects[capacity]: MaybeObject;
 }
 
 extern operator '.length_intptr' macro LoadAndUntagFixedArrayBaseLength(

src/objects/ordered-hash-table.tq

@@ -27,7 +27,7 @@ const kSmallOrderedHashSetMap: Map = SmallOrderedHashSetMapConstant();
 extern class SmallOrderedHashSet extends SmallOrderedHashTable {
   number_of_elements: uint8;
   number_of_deleted_elements: uint8;
-  number_of_buckets: uint8;
+  const number_of_buckets: uint8;
   @if(TAGGED_SIZE_8_BYTES) padding[5]: uint8;
   @ifnot(TAGGED_SIZE_8_BYTES) padding[1]: uint8;
   data_table[Convert<intptr>(number_of_buckets) * kSmallOrderedHashTableLoadFactor]:
@@ -66,7 +66,7 @@ const kSmallOrderedHashMapMap: Map = SmallOrderedHashMapMapConstant();
 extern class SmallOrderedHashMap extends SmallOrderedHashTable {
   number_of_elements: uint8;
   number_of_deleted_elements: uint8;
-  number_of_buckets: uint8;
+  const number_of_buckets: uint8;
   @if(TAGGED_SIZE_8_BYTES) padding[5]: uint8;
   @ifnot(TAGGED_SIZE_8_BYTES) padding[1]: uint8;
   data_table[Convert<intptr>(number_of_buckets) * kSmallOrderedHashTableLoadFactor]:
@@ -104,7 +104,7 @@ extern class SmallOrderedNameDictionary extends SmallOrderedHashTable {
   hash: int32;
   number_of_elements: uint8;
   number_of_deleted_elements: uint8;
-  number_of_buckets: uint8;
+  const number_of_buckets: uint8;
   padding: uint8;
   data_table[Convert<intptr>(number_of_buckets) * kSmallOrderedHashTableLoadFactor]:
       NameDictionaryEntry;

src/objects/string.tq

@@ -6,7 +6,7 @@
 @generateCppClass
 @reserveBitsInInstanceType(6)
 extern class String extends Name {
-  length: int32;
+  const length: int32;
 }
 
 @generateCppClass

src/torque/implementation-visitor.cc

@@ -1345,7 +1345,7 @@ void ImplementationVisitor::InitializeClass(
 
 VisitResult ImplementationVisitor::GenerateArrayLength(
     Expression* array_length, Namespace* nspace,
-    const std::map<std::string, LocationReference>& bindings) {
+    const std::map<std::string, LocalValue>& bindings) {
   StackScope stack_scope(this);
   CurrentSourcePosition::Scope pos_scope(array_length->pos);
   // Switch to the namespace where the class was declared.
@@ -1369,11 +1369,15 @@ VisitResult ImplementationVisitor::GenerateArrayLength(VisitResult object,
 
   StackScope stack_scope(this);
   const ClassType* class_type = *object.type()->ClassSupertype();
-  std::map<std::string, LocationReference> bindings;
+  std::map<std::string, LocalValue> bindings;
   for (Field f : class_type->ComputeAllFields()) {
     if (f.index) break;
     bindings.insert(
-        {f.name_and_type.name, GenerateFieldReference(object, f, class_type)});
+        {f.name_and_type.name,
+         f.const_qualified
+             ? LocalValue{GenerateFieldReference(object, f, class_type)}
+             : LocalValue(
+                   "Non-const fields cannot be used for array lengths.")});
   }
   return stack_scope.Yield(
       GenerateArrayLength(*field.index, class_type->nspace(), bindings));
@@ -1385,13 +1389,18 @@ VisitResult ImplementationVisitor::GenerateArrayLength(
   DCHECK(field.index);
 
   StackScope stack_scope(this);
-  std::map<std::string, LocationReference> bindings;
+  std::map<std::string, LocalValue> bindings;
   for (Field f : class_type->ComputeAllFields()) {
     if (f.index) break;
     const std::string& fieldname = f.name_and_type.name;
     VisitResult value = initializer_results.field_value_map.at(fieldname);
-    bindings.insert({fieldname, LocationReference::Temporary(
-                                    value, "initial field " + fieldname)});
+    bindings.insert(
+        {fieldname,
+         f.const_qualified
+             ? LocalValue{LocationReference::Temporary(
+                   value, "initial field " + fieldname)}
+             : LocalValue(
+                   "Non-const fields cannot be used for array lengths.")});
   }
   return stack_scope.Yield(
       GenerateArrayLength(*field.index, class_type->nspace(), bindings));
@@ -2085,12 +2094,7 @@ LocationReference ImplementationVisitor::GetLocationReference(
         ReportError("cannot have generic parameters on local name ",
                     expr->name);
       }
-      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 ref;
+      return (*value)->GetLocationReference(*value);
     }
   }
 
@@ -2376,7 +2380,8 @@ VisitResult ImplementationVisitor::GenerateCall(
                   "' required for call to '", callable->ReadableName(),
                   "' is not defined");
     }
-    implicit_arguments.push_back(GenerateFetchFromLocation((*val)->value));
+    implicit_arguments.push_back(
+        GenerateFetchFromLocation((*val)->GetLocationReference(*val)));
   }
 
   std::vector<VisitResult> converted_arguments;

src/torque/implementation-visitor.h

@@ -23,7 +23,7 @@ namespace torque {
 
 template <typename T>
 class Binding;
-struct LocalValue;
+class LocalValue;
 class ImplementationVisitor;
 
 // LocationReference is the representation of an l-value, so a value that might
@@ -345,8 +345,32 @@ class BlockBindings {
   std::vector<std::unique_ptr<Binding<T>>> bindings_;
 };
 
-struct LocalValue {
-  LocationReference value;
+class LocalValue {
+ public:
+  explicit LocalValue(LocationReference reference)
+      : value(std::move(reference)) {}
+  explicit LocalValue(std::string inaccessible_explanation)
+      : inaccessible_explanation(std::move(inaccessible_explanation)) {}
+
+  LocationReference GetLocationReference(Binding<LocalValue>* binding) {
+    if (value) {
+      const LocationReference& ref = *value;
+      if (ref.IsVariableAccess()) {
+        // Attach the binding to enable the never-assigned-to lint check.
+        return LocationReference::VariableAccess(ref.GetVisitResult(), binding);
+      }
+      return ref;
+    } else {
+      Error("Cannot access ", binding->name(), ": ", inaccessible_explanation)
+          .Throw();
+    }
+  }
+
+  bool IsAccessible() const { return value.has_value(); }
+
+ private:
+  base::Optional<LocationReference> value;
+  std::string inaccessible_explanation;
 };
 
 struct LocalLabel {
@@ -366,7 +390,8 @@ 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_];
-  const LocationReference& ref = binding->value;
+  if (!binding->IsAccessible()) return false;
+  const LocationReference& ref = binding->GetLocationReference(binding);
   if (!ref.IsVariableAccess()) return false;
   return !ref.GetVisitResult().type()->StructSupertype();
 }
@@ -424,7 +449,7 @@ class ImplementationVisitor {
       const LayoutForInitialization& layout);
   VisitResult GenerateArrayLength(
       Expression* array_length, Namespace* nspace,
-      const std::map<std::string, LocationReference>& bindings);
+      const std::map<std::string, LocalValue>& bindings);
   VisitResult GenerateArrayLength(VisitResult object, const Field& field);
   VisitResult GenerateArrayLength(const ClassType* class_type,
                                   const InitializerResults& initializer_results,
@@ -440,6 +465,7 @@ class ImplementationVisitor {
   VisitResult Visit(StructExpression* decl);
 
   LocationReference GetLocationReference(Expression* location);
+  LocationReference LookupLocalValue(const std::string& name);
   LocationReference GetLocationReference(IdentifierExpression* expr);
   LocationReference GetLocationReference(DereferenceExpression* expr);
   LocationReference GetLocationReference(FieldAccessExpression* expr);

test/torque/test-torque.tq

@@ -1148,7 +1148,7 @@ namespace test {
   class InternalClassWithStructElements extends HeapObject {
     dummy1: int32;
     dummy2: int32;
-    count: Smi;
+    const count: Smi;
     data: Smi;
     object: Object;
     entries[count]: Smi;