[torque] Add a way to specify that a class field is optional

Currently, some ScopeInfo fields are defined as indexed fields with a
length of either one or zero, because the field might be present or it
might not. Based on comments in https://crrev.com/c/v8/v8/+/2601880 ,
this strategy is not sustainable and we need a better way to represent
optional fields so that we don't have to pass zero when accessing their
only element. This change is a proposal to fix that problem.

Syntax:

I'm proposing using a question mark because TypeScript does, and Torque
syntax looks somewhat like TypeScript. I don't feel strongly about this
though, and I'm open to other suggestions.
  field_name?[condition_expression]: FieldType;

Internal Torque compiler representation:

Internally, I've updated the Torque compiler to still treat these fields
as indexed, but with an extra flag saying they're optional. When getting
a LocationReference for a field access expression on an optional field,
Torque produces a Slice like it would for any other indexed field and
subsequently calls AtIndex(0) to get a Reference.

AtIndex can crash the process if the index is out of bounds (which is
good), so some other parts of the Torque compiler need minor adjustments
so that it doesn't take references to optional fields unless it actually
needs them.

Initialization:

This proposal doesn't include any changes to initialization logic, so an
optional field can still be initialized using '...' and an iterator.
Perhaps we could introduce an Optional<T> struct for prettier
initialization in a future change.

Bug: v8:7793
Change-Id: I37649495f4c259e685261f53e4cf2859da66a31f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2706306
Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73018}

src/builtins/torque-internal.tq

@@ -304,6 +304,15 @@ extern macro StaticAssert(bool, constexpr string);
 // field x from object o is `(&o.x).length`.
 intrinsic %IndexedFieldLength<T: type>(o: T, f: constexpr string);
 
+// If field x is defined as optional, then &o.x returns a reference to the field
+// or crashes the program (unreachable) if the field is not present. Usually
+// that's the most convenient behavior, but in rare cases such as the
+// implementation of the dot operator, we may instead need to get a Slice to the
+// optional field, which is either length zero or one depending on whether the
+// field is present. This intrinsic provides Slices for both indexed fields
+// (equivalent to &o.x) and optional fields.
+intrinsic %FieldSlice<T: type>(o: T, f: constexpr string);
+
 }  // namespace torque_internal
 
 // Indicates that an array-field should not be initialized.

src/objects/scope-info.cc

@@ -580,7 +580,7 @@ Handle<ScopeInfo> ScopeInfo::RecreateWithBlockList(
       isolate, kVariablePartIndex, *original, kVariablePartIndex,
       scope_info->LocalsBlockListIndex() - kVariablePartIndex,
       WriteBarrierMode::UPDATE_WRITE_BARRIER);
-  scope_info->set_locals_block_list(0, *blocklist);
+  scope_info->set_locals_block_list(*blocklist);
   scope_info->CopyElements(
       isolate, scope_info->LocalsBlockListIndex() + 1, *original,
       scope_info->LocalsBlockListIndex(),
@@ -700,12 +700,12 @@ bool ScopeInfo::HasSharedFunctionName() const {
 void ScopeInfo::SetFunctionName(Object name) {
   DCHECK(HasFunctionName());
   DCHECK(name.IsString() || name == SharedFunctionInfo::kNoSharedNameSentinel);
-  set_function_variable_info_name(0, name);
+  set_function_variable_info_name(name);
 }
 
 void ScopeInfo::SetInferredFunctionName(String name) {
   DCHECK(HasInferredFunctionName());
-  set_inferred_function_name(0, name);
+  set_inferred_function_name(name);
 }
 
 bool ScopeInfo::HasOuterScopeInfo() const {
@@ -736,19 +736,19 @@ bool ScopeInfo::HasLocalsBlockList() const {
 
 StringSet ScopeInfo::LocalsBlockList() const {
   DCHECK(HasLocalsBlockList());
-  return StringSet::cast(locals_block_list(0));
+  return StringSet::cast(locals_block_list());
 }
 
 bool ScopeInfo::HasContext() const { return ContextLength() > 0; }
 
 Object ScopeInfo::FunctionName() const {
   DCHECK(HasFunctionName());
-  return function_variable_info_name(0);
+  return function_variable_info_name();
 }
 
 Object ScopeInfo::InferredFunctionName() const {
   DCHECK(HasInferredFunctionName());
-  return inferred_function_name(0);
+  return inferred_function_name();
 }
 
 String ScopeInfo::FunctionDebugName() const {
@@ -766,29 +766,29 @@ String ScopeInfo::FunctionDebugName() const {
 
 int ScopeInfo::StartPosition() const {
   DCHECK(HasPositionInfo());
-  return position_info_start(0);
+  return position_info_start();
 }
 
 int ScopeInfo::EndPosition() const {
   DCHECK(HasPositionInfo());
-  return position_info_end(0);
+  return position_info_end();
 }
 
 void ScopeInfo::SetPositionInfo(int start, int end) {
   DCHECK(HasPositionInfo());
   DCHECK_LE(start, end);
-  set_position_info_start(0, start);
-  set_position_info_end(0, end);
+  set_position_info_start(start);
+  set_position_info_end(end);
 }
 
 ScopeInfo ScopeInfo::OuterScopeInfo() const {
   DCHECK(HasOuterScopeInfo());
-  return ScopeInfo::cast(outer_scope_info(0));
+  return ScopeInfo::cast(outer_scope_info());
 }
 
 SourceTextModuleInfo ScopeInfo::ModuleDescriptorInfo() const {
   DCHECK(scope_type() == MODULE_SCOPE);
-  return SourceTextModuleInfo::cast(module_info(0));
+  return SourceTextModuleInfo::cast(module_info());
 }
 
 String ScopeInfo::ContextLocalName(int var) const {
@@ -846,7 +846,7 @@ int ScopeInfo::ModuleIndex(String name, VariableMode* mode,
   DCHECK_NOT_NULL(init_flag);
   DCHECK_NOT_NULL(maybe_assigned_flag);
 
-  int module_vars_count = module_variable_count(0);
+  int module_vars_count = module_variable_count();
   int entry = ModuleVariablesIndex();
   for (int i = 0; i < module_vars_count; ++i) {
     String var_name = String::cast(get(entry + kModuleVariableNameOffset));
@@ -895,7 +895,7 @@ int ScopeInfo::ContextSlotIndex(ScopeInfo scope_info, String name,
 
 int ScopeInfo::SavedClassVariableContextLocalIndex() const {
   if (HasSavedClassVariableIndexBit::decode(Flags())) {
-    int index = saved_class_variable_info(0);
+    int index = saved_class_variable_info();
     return index - Context::MIN_CONTEXT_SLOTS;
   }
   return -1;
@@ -904,7 +904,7 @@ int ScopeInfo::SavedClassVariableContextLocalIndex() const {
 int ScopeInfo::ReceiverContextSlotIndex() const {
   if (ReceiverVariableBits::decode(Flags()) ==
       VariableAllocationInfo::CONTEXT) {
-    return receiver_info(0);
+    return receiver_info();
   }
   return -1;
 }
@@ -914,7 +914,7 @@ int ScopeInfo::FunctionContextSlotIndex(String name) const {
   if (FunctionVariableBits::decode(Flags()) ==
           VariableAllocationInfo::CONTEXT &&
       FunctionName() == name) {
-    return function_variable_info_context_or_stack_slot_index(0);
+    return function_variable_info_context_or_stack_slot_index();
   }
   return -1;
 }

src/objects/scope-info.tq

@@ -121,50 +121,51 @@ extern class ScopeInfo extends FixedArrayBase {
   // If the scope is a class scope and it has static private methods that
   // may be accessed directly or through eval, one slot is reserved to hold
   // the context slot index for the class variable.
-  saved_class_variable_info[flags.has_saved_class_variable_index ? 1 : 0]: Smi;
+  saved_class_variable_info?[flags.has_saved_class_variable_index]: Smi;
 
   // If the scope binds a "this" value, one slot is reserved to hold the
   // context or stack slot index for the variable.
-  receiver_info[
-      flags.receiver_variable ==
-          FromConstexpr<VariableAllocationInfo>(VariableAllocationInfo::STACK)
-      || flags.receiver_variable ==
-          FromConstexpr<VariableAllocationInfo>(VariableAllocationInfo::CONTEXT)
-      ? 1 : 0]: Smi;
+  receiver_info?[
+    flags.receiver_variable ==
+        FromConstexpr<VariableAllocationInfo>(VariableAllocationInfo::STACK) ||
+    flags.receiver_variable ==
+        FromConstexpr<VariableAllocationInfo>(VariableAllocationInfo::CONTEXT)
+  ]: Smi;
 
   // If the scope belongs to a named function expression this part contains
   // information about the function variable. It always occupies two array
   // slots:  a. The name of the function variable.
   //         b. The context or stack slot index for the variable.
-  function_variable_info[flags.function_variable != FromConstexpr<VariableAllocationInfo>(VariableAllocationInfo::NONE) ? 1 : 0]:
-      FunctionVariableInfo;
+  function_variable_info?
+      [flags.function_variable !=
+       FromConstexpr<VariableAllocationInfo>(VariableAllocationInfo::NONE)]:
+          FunctionVariableInfo;
 
-  inferred_function_name[flags.has_inferred_function_name ? 1 : 0]: String|
-      Undefined;
+  inferred_function_name?[flags.has_inferred_function_name]: String|Undefined;
 
   // Contains two slots with a) the startPosition and b) the endPosition if
   // the scope belongs to a function or script.
-  position_info[flags.scope_type == ScopeType::FUNCTION_SCOPE ||
-                flags.scope_type == ScopeType::SCRIPT_SCOPE ||
-                flags.scope_type == ScopeType::EVAL_SCOPE ||
-                flags.scope_type == ScopeType::MODULE_SCOPE
-                ? 1 : 0]: PositionInfo;
+  position_info?
+      [flags.scope_type == ScopeType::FUNCTION_SCOPE ||
+       flags.scope_type == ScopeType::SCRIPT_SCOPE ||
+       flags.scope_type == ScopeType::EVAL_SCOPE ||
+       flags.scope_type == ScopeType::MODULE_SCOPE]: PositionInfo;
 
-  outer_scope_info[flags.has_outer_scope_info ? 1 : 0]: ScopeInfo|TheHole;
+  outer_scope_info?[flags.has_outer_scope_info]: ScopeInfo|TheHole;
 
   // List of stack allocated local variables. Used by debug evaluate to properly
   // abort variable lookup when a name clashes with a stack allocated local that
   // can't be materialized.
-  locals_block_list[flags.has_locals_block_list ? 1 : 0]: HashTable;
+  locals_block_list?[flags.has_locals_block_list]: HashTable;
 
   // For a module scope, this part contains the SourceTextModuleInfo, the
   // number of MODULE-allocated variables, and the metadata of those
   // variables.  For non-module scopes it is empty.
-  module_info[flags.scope_type == ScopeType::MODULE_SCOPE ? 1 : 0]:
-      SourceTextModuleInfo;
-  const module_variable_count[flags.scope_type == ScopeType::MODULE_SCOPE ? 1 : 0]:
-      Smi;
-  module_variables[flags.scope_type == ScopeType::MODULE_SCOPE ? module_variable_count[0] : 0]:
+  module_info?
+      [flags.scope_type == ScopeType::MODULE_SCOPE]: SourceTextModuleInfo;
+  const module_variable_count?
+      [flags.scope_type == ScopeType::MODULE_SCOPE]: Smi;
+  module_variables[flags.scope_type == ScopeType::MODULE_SCOPE ? module_variable_count : 0]:
       ModuleVariable;
 }
 

src/torque/ast.h

@@ -924,9 +924,18 @@ struct Annotation {
   base::Optional<AnnotationParameter> param;
 };
 
+struct ClassFieldIndexInfo {
+  // The expression that can compute how many items are in the indexed field.
+  Expression* expr;
+
+  // Whether the field was declared as optional, meaning it can only hold zero
+  // or one values, and thus should not require an index expression to access.
+  bool optional;
+};
+
 struct ClassFieldExpression {
   NameAndTypeExpression name_and_type;
-  base::Optional<Expression*> index;
+  base::Optional<ClassFieldIndexInfo> index;
   std::vector<ConditionalAnnotation> conditions;
   bool weak;
   bool const_qualified;

src/torque/implementation-visitor.cc

@@ -1381,10 +1381,19 @@ InitializerResults ImplementationVisitor::VisitInitializerResults(
 }
 
 LocationReference ImplementationVisitor::GenerateFieldReference(
-    VisitResult object, const Field& field, const ClassType* class_type) {
+    VisitResult object, const Field& field, const ClassType* class_type,
+    bool treat_optional_as_indexed) {
   if (field.index.has_value()) {
-    return LocationReference::HeapSlice(
+    LocationReference slice = LocationReference::HeapSlice(
         GenerateCall(class_type->GetSliceMacroName(field), {{object}, {}}));
+    if (field.index->optional && !treat_optional_as_indexed) {
+      // This field was declared using optional syntax, so any reference to it
+      // is implicitly a reference to the first item.
+      return GenerateReferenceToItemInHeapSlice(
+          slice, {TypeOracle::GetConstInt31Type(), "0"});
+    } else {
+      return slice;
+    }
   }
   DCHECK(field.offset.has_value());
   StackRange result_range = assembler().TopRange(0);
@@ -1481,18 +1490,25 @@ VisitResult ImplementationVisitor::GenerateArrayLength(VisitResult object,
     if (field.name_and_type.name == f.name_and_type.name) {
       before_current = false;
     }
+    // We can't generate field references eagerly here, because some preceding
+    // fields might be optional, and attempting to get a reference to an
+    // optional field can crash the program if the field isn't present.
+    // Instead, we use the lazy form of LocalValue to only generate field
+    // references if they are used in the length expression.
     bindings.insert(
         {f.name_and_type.name,
          f.const_qualified
              ? (before_current
-                    ? LocalValue{GenerateFieldReference(object, f, class_type)}
+                    ? LocalValue{[=]() {
+                        return GenerateFieldReference(object, f, class_type);
+                      }}
                     : LocalValue("Array lengths may only refer to fields "
                                  "defined earlier"))
              : LocalValue(
                    "Non-const fields cannot be used for array lengths.")});
   }
   return stack_scope.Yield(
-      GenerateArrayLength(*field.index, class_type->nspace(), bindings));
+      GenerateArrayLength(field.index->expr, class_type->nspace(), bindings));
 }
 
 VisitResult ImplementationVisitor::GenerateArrayLength(
@@ -1515,7 +1531,7 @@ VisitResult ImplementationVisitor::GenerateArrayLength(
                    "Non-const fields cannot be used for array lengths.")});
   }
   return stack_scope.Yield(
-      GenerateArrayLength(*field.index, class_type->nspace(), bindings));
+      GenerateArrayLength(field.index->expr, class_type->nspace(), bindings));
 }
 
 LayoutForInitialization ImplementationVisitor::GenerateLayoutForInitialization(
@@ -2284,22 +2300,27 @@ LocationReference ImplementationVisitor::GetLocationReference(
   LocationReference reference = GetLocationReference(expr->array);
   VisitResult index = Visit(expr->index);
   if (reference.IsHeapSlice()) {
-    Arguments arguments{{index}, {}};
-    const StructType* slice_type =
-        *reference.heap_slice().type()->StructSupertype();
-    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, std::move(slice_value), arguments, {}, false));
+    return GenerateReferenceToItemInHeapSlice(reference, index);
   } else {
     return LocationReference::ArrayAccess(GenerateFetchFromLocation(reference),
                                           index);
   }
 }
 
+LocationReference ImplementationVisitor::GenerateReferenceToItemInHeapSlice(
+    LocationReference slice, VisitResult index) {
+  DCHECK(slice.IsHeapSlice());
+  Arguments arguments{{index}, {}};
+  const StructType* slice_type = *slice.heap_slice().type()->StructSupertype();
+  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(slice.GetVisitResult(), "slice as value");
+  return LocationReference::HeapReference(
+      GenerateCall(method, std::move(slice_value), arguments, {}, false));
+}
+
 LocationReference ImplementationVisitor::GetLocationReference(
     IdentifierExpression* expr) {
   if (expr->namespace_qualification.empty()) {
@@ -2997,6 +3018,21 @@ VisitResult ImplementationVisitor::GenerateCall(
       assembler().Emit(MakeLazyNodeInstruction{getter, return_type,
                                                constexpr_arguments_for_getter});
       return VisitResult(return_type, assembler().TopRange(1));
+    } else if (intrinsic->ExternalName() == "%FieldSlice") {
+      const Type* type = specialization_types[0];
+      const ClassType* class_type = ClassType::DynamicCast(type);
+      if (!class_type) {
+        ReportError("%FieldSlice must take a class type parameter");
+      }
+      const Field& field =
+          class_type->LookupField(StringLiteralUnquote(constexpr_arguments[0]));
+      LocationReference ref = GenerateFieldReference(
+          VisitResult(type, argument_range), field, class_type,
+          /*treat_optional_as_indexed=*/true);
+      if (!ref.IsHeapSlice()) {
+        ReportError("%FieldSlice expected an indexed or optional field");
+      }
+      return ref.heap_slice();
     } else {
       assembler().Emit(CallIntrinsicInstruction{intrinsic, specialization_types,
                                                 constexpr_arguments});
@@ -3885,7 +3921,7 @@ base::Optional<std::vector<Field>> GetOrderedUniqueIndexFields(
   std::set<std::string> index_names;
   for (const Field& field : type.ComputeAllFields()) {
     if (field.index) {
-      auto name_and_type = ExtractSimpleFieldArraySize(type, *field.index);
+      auto name_and_type = ExtractSimpleFieldArraySize(type, field.index->expr);
       if (!name_and_type) {
         return base::nullopt;
       }
@@ -4004,7 +4040,7 @@ void CppClassGenerator::GenerateClass() {
     for (const Field& field : type_->ComputeAllFields()) {
       if (field.index) {
         auto index_name_and_type =
-            *ExtractSimpleFieldArraySize(*type_, *field.index);
+            *ExtractSimpleFieldArraySize(*type_, field.index->expr);
         size_t field_size = 0;
         std::tie(field_size, std::ignore) = field.GetFieldSizeInformation();
         hdr_ << "    size += " << index_name_and_type.name << " * "
@@ -4123,7 +4159,7 @@ void GenerateBoundsDCheck(std::ostream& os, const std::string& index,
   os << "  DCHECK_GE(" << index << ", 0);\n";
   std::string length_expression;
   if (base::Optional<NameAndType> array_length =
-          ExtractSimpleFieldArraySize(*type, *f.index)) {
+          ExtractSimpleFieldArraySize(*type, f.index->expr)) {
     length_expression = "this ->" + array_length->name + "()";
   } else {
     // The length is element 2 in the flattened field slice.
@@ -4164,7 +4200,7 @@ void CppClassGenerator::GenerateFieldAccessors(
     return;
   }
 
-  bool indexed = class_field.index.has_value();
+  bool indexed = class_field.index && !class_field.index->optional;
   std::string type_name = GetTypeNameForAccessor(innermost_field);
   bool can_contain_heap_objects = CanContainHeapObjects(field_type);
 
@@ -4288,9 +4324,10 @@ void CppClassGenerator::EmitLoadFieldStatement(
 
   std::string offset = field_offset;
   if (class_field.index) {
-    GenerateBoundsDCheck(inl_, "i", type_, class_field);
-    inl_ << "  int offset = " << field_offset << " + i * " << class_field_size
-         << ";\n";
+    const char* index = class_field.index->optional ? "0" : "i";
+    GenerateBoundsDCheck(inl_, index, type_, class_field);
+    inl_ << "  int offset = " << field_offset << " + " << index << " * "
+         << class_field_size << ";\n";
     offset = "offset";
   }
 
@@ -4353,9 +4390,10 @@ void CppClassGenerator::EmitStoreFieldStatement(
 
   std::string offset = field_offset;
   if (class_field.index) {
-    GenerateBoundsDCheck(inl_, "i", type_, class_field);
-    inl_ << "  int offset = " << field_offset << " + i * " << class_field_size
-         << ";\n";
+    const char* index = class_field.index->optional ? "0" : "i";
+    GenerateBoundsDCheck(inl_, index, type_, class_field);
+    inl_ << "  int offset = " << field_offset << " + " << index << " * "
+         << class_field_size << ";\n";
     offset = "offset";
   }
 

src/torque/implementation-visitor.h

@@ -361,6 +361,8 @@ class LocalValue {
       : value(std::move(reference)) {}
   explicit LocalValue(std::string inaccessible_explanation)
       : inaccessible_explanation(std::move(inaccessible_explanation)) {}
+  explicit LocalValue(std::function<LocationReference()> lazy)
+      : lazy(std::move(lazy)) {}
 
   LocationReference GetLocationReference(Binding<LocalValue>* binding) {
     if (value) {
@@ -370,16 +372,19 @@ class LocalValue {
         return LocationReference::VariableAccess(ref.GetVisitResult(), binding);
       }
       return ref;
+    } else if (lazy) {
+      return (*lazy)();
     } else {
       Error("Cannot access ", binding->name(), ": ", inaccessible_explanation)
           .Throw();
     }
   }
 
-  bool IsAccessible() const { return value.has_value(); }
+  bool IsAccessibleNonLazy() const { return value.has_value(); }
 
  private:
   base::Optional<LocationReference> value;
+  base::Optional<std::function<LocationReference()>> lazy;
   std::string inaccessible_explanation;
 };
 
@@ -400,7 +405,7 @@ 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_];
-  if (!binding->IsAccessible()) return false;
+  if (!binding->IsAccessibleNonLazy()) return false;
   const LocationReference& ref = binding->GetLocationReference(binding);
   if (!ref.IsVariableAccess()) return false;
   return !ref.GetVisitResult().type()->StructSupertype();
@@ -469,9 +474,9 @@ class ImplementationVisitor {
   InitializerResults VisitInitializerResults(
       const ClassType* class_type,
       const std::vector<NameAndExpression>& expressions);
-  LocationReference GenerateFieldReference(VisitResult object,
-                                           const Field& field,
-                                           const ClassType* class_type);
+  LocationReference GenerateFieldReference(
+      VisitResult object, const Field& field, const ClassType* class_type,
+      bool treat_optional_as_indexed = false);
   LocationReference GenerateFieldReferenceForInit(
       VisitResult object, const Field& field,
       const LayoutForInitialization& layout);
@@ -502,6 +507,8 @@ class ImplementationVisitor {
       bool ignore_stuct_field_constness = false,
       base::Optional<SourcePosition> pos = {});
   LocationReference GetLocationReference(ElementAccessExpression* expr);
+  LocationReference GenerateReferenceToItemInHeapSlice(LocationReference slice,
+                                                       VisitResult index);
 
   VisitResult GenerateFetchFromLocation(const LocationReference& reference);
 

src/torque/torque-parser.cc

@@ -1976,10 +1976,27 @@ base::Optional<ParseResult> MakeClassField(ParseResultIterator* child_results) {
   auto weak = child_results->NextAs<bool>();
   auto const_qualified = child_results->NextAs<bool>();
   auto name = child_results->NextAs<Identifier*>();
+  auto optional = child_results->NextAs<bool>();
   auto index = child_results->NextAs<base::Optional<Expression*>>();
+  if (optional && !index) {
+    Error(
+        "Fields using optional specifier must also provide an expression "
+        "indicating the condition for whether the field is present");
+  }
+  base::Optional<ClassFieldIndexInfo> index_info;
+  if (index) {
+    if (optional) {
+      // Internally, an optional field is just an indexed field where the count
+      // is zero or one.
+      index = MakeNode<ConditionalExpression>(
+          *index, MakeNode<NumberLiteralExpression>(1),
+          MakeNode<NumberLiteralExpression>(0));
+    }
+    index_info = ClassFieldIndexInfo{*index, optional};
+  }
   auto type = child_results->NextAs<TypeExpression*>();
   return ParseResult{ClassFieldExpression{{name, type},
-                                          index,
+                                          index_info,
                                           std::move(conditions),
                                           weak,
                                           const_qualified,
@@ -2257,7 +2274,8 @@ struct TorqueGrammar : Grammar {
   // Result: ClassFieldExpression
   Symbol classField = {
       Rule({annotations, CheckIf(Token("weak")), CheckIf(Token("const")), &name,
-            optionalArraySpecifier, Token(":"), &type, Token(";")},
+            CheckIf(Token("?")), optionalArraySpecifier, Token(":"), &type,
+            Token(";")},
            MakeClassField)};
 
   // Result: StructFieldExpression

src/torque/type-visitor.cc

@@ -420,7 +420,7 @@ void TypeVisitor::VisitClassFieldsAndMethods(
         ReportError("in-object properties cannot be weak");
       }
     }
-    base::Optional<Expression*> array_length = field_expression.index;
+    base::Optional<ClassFieldIndexInfo> array_length = field_expression.index;
     const Field& field = class_type->RegisterField(
         {field_expression.name_and_type.name->pos,
          class_type,
@@ -440,7 +440,8 @@ void TypeVisitor::VisitClassFieldsAndMethods(
       field.ValidateAlignment(class_offset +
                               field_size * ResidueClass::Unknown());
 
-      if (auto literal = NumberLiteralExpression::DynamicCast(*field.index)) {
+      if (auto literal =
+              NumberLiteralExpression::DynamicCast(field.index->expr)) {
         size_t value = static_cast<size_t>(literal->number);
         if (value != literal->number) {
           Error("non-integral array length").Position(field.pos);

src/torque/types.cc

@@ -743,12 +743,16 @@ void ClassType::GenerateAccessors() {
       continue;
     }
 
+    // An explicit index is only used for indexed fields not marked as optional.
+    // Optional fields implicitly load or store item zero.
+    bool use_index = field.index && !field.index->optional;
+
     // Load accessor
     std::string load_macro_name = "Load" + this->name() + camel_field_name;
     Signature load_signature;
     load_signature.parameter_names.push_back(MakeNode<Identifier>("o"));
     load_signature.parameter_types.types.push_back(this);
-    if (field.index) {
+    if (use_index) {
       load_signature.parameter_names.push_back(MakeNode<Identifier>("i"));
       load_signature.parameter_types.types.push_back(
           TypeOracle::GetIntPtrType());
@@ -758,7 +762,7 @@ void ClassType::GenerateAccessors() {
 
     Expression* load_expression =
         MakeFieldAccessExpression(parameter, field.name_and_type.name);
-    if (field.index) {
+    if (use_index) {
       load_expression =
           MakeNode<ElementAccessExpression>(load_expression, index);
     }
@@ -773,7 +777,7 @@ void ClassType::GenerateAccessors() {
       Signature store_signature;
       store_signature.parameter_names.push_back(MakeNode<Identifier>("o"));
       store_signature.parameter_types.types.push_back(this);
-      if (field.index) {
+      if (use_index) {
         store_signature.parameter_names.push_back(MakeNode<Identifier>("i"));
         store_signature.parameter_types.types.push_back(
             TypeOracle::GetIntPtrType());
@@ -785,7 +789,7 @@ void ClassType::GenerateAccessors() {
       store_signature.return_type = TypeOracle::GetVoidType();
       Expression* store_expression =
           MakeFieldAccessExpression(parameter, field.name_and_type.name);
-      if (field.index) {
+      if (use_index) {
         store_expression =
             MakeNode<ElementAccessExpression>(store_expression, index);
       }
@@ -806,23 +810,23 @@ void ClassType::GenerateSliceAccessor(size_t field_index) {
   //
   // If the field has a known offset (in this example, 16):
   // FieldSliceClassNameFieldName(o: ClassName) {
-  //     return torque_internal::unsafe::New{Const,Mutable}Slice<FieldType>(
-  //     object: o,
-  //     offset: 16,
-  //     length: torque_internal::%IndexedFieldLength<ClassName>(
-  //                 o, "field_name")
+  //   return torque_internal::unsafe::New{Const,Mutable}Slice<FieldType>(
+  //     /*object:*/ o,
+  //     /*offset:*/ 16,
+  //     /*length:*/ torque_internal::%IndexedFieldLength<ClassName>(
+  //                     o, "field_name")
   //   );
   // }
   //
   // If the field has an unknown offset, and the previous field is named p, and
   // an item in the previous field has size 4:
   // FieldSliceClassNameFieldName(o: ClassName) {
-  //   const previous = &o.p;
-  //     return torque_internal::unsafe::New{Const,Mutable}Slice<FieldType>(
-  //     object: o,
-  //     offset: previous.offset + 4 * previous.length,
-  //     length: torque_internal::%IndexedFieldLength<ClassName>(
-  //                 o, "field_name")
+  //   const previous = %FieldSlice<ClassName>(o, "p");
+  //   return torque_internal::unsafe::New{Const,Mutable}Slice<FieldType>(
+  //     /*object:*/ o,
+  //     /*offset:*/ previous.offset + 4 * previous.length,
+  //     /*length:*/ torque_internal::%IndexedFieldLength<ClassName>(
+  //                     o, "field_name")
   //   );
   // }
   const Field& field = fields_[field_index];
@@ -849,14 +853,14 @@ void ClassType::GenerateSliceAccessor(size_t field_index) {
     const Field* previous = GetFieldPreceding(field_index);
     DCHECK_NOT_NULL(previous);
 
-    // o.p
-    Expression* previous_expression =
-        MakeFieldAccessExpression(parameter, previous->name_and_type.name);
-
-    // &o.p
-    previous_expression = MakeCallExpression("&", {previous_expression});
+    // %FieldSlice<ClassName>(o, "p")
+    Expression* previous_expression = MakeCallExpression(
+        MakeIdentifierExpression({"torque_internal"}, "%FieldSlice",
+                                 {MakeNode<PrecomputedTypeExpression>(this)}),
+        {parameter, MakeNode<StringLiteralExpression>(
+                        StringLiteralQuote(previous->name_and_type.name))});
 
-    // const previous = &o.p;
+    // const previous = %FieldSlice<ClassName>(o, "p");
     Statement* define_previous =
         MakeConstDeclarationStatement("previous", previous_expression);
     statements.push_back(define_previous);
@@ -896,10 +900,10 @@ void ClassType::GenerateSliceAccessor(size_t field_index) {
                       StringLiteralQuote(field.name_and_type.name))});
 
   // torque_internal::unsafe::New{Const,Mutable}Slice<FieldType>(
-  //   object: o,
-  //   offset: <<offset_expression>>,
-  //   length: torque_internal::%IndexedFieldLength<ClassName>(
-  //               o, "field_name")
+  //   /*object:*/ o,
+  //   /*offset:*/ <<offset_expression>>,
+  //   /*length:*/ torque_internal::%IndexedFieldLength<ClassName>(
+  //                   o, "field_name")
   // )
   IdentifierExpression* new_struct = MakeIdentifierExpression(
       {"torque_internal", "unsafe"},

src/torque/types.h

@@ -215,7 +215,7 @@ struct Field {
 
   SourcePosition pos;
   const AggregateType* aggregate;
-  base::Optional<Expression*> index;
+  base::Optional<ClassFieldIndexInfo> index;
   NameAndType name_and_type;
 
   // The byte offset of this field from the beginning of the containing class or