[torque] Get rid of `weak` keyword

Torque allows a `weak` keyword on class field declarations. This keyword
is confusing, because it means two completely different things:

1. This field should be included in the weak fields section, meaning the
   field's offset should be in the range [kStartOfWeakFieldsOffset,
   kEndOfWeakFieldsOffset).
2. If a BodyDescriptor is generated for this class, then this field
   should be visited using *custom* weakness semantics
   (IterateCustomWeakPointers, not IterateMaybeObjectPointers).

I propose the following updated behavior, which I think is a bit more
reasonable:

1. To request that the generated BodyDescriptor use custom weakness
   semantics, use a new annotation @customWeakMarking.
2. The weak fields section includes all fields that can be a Weak<T>
   type, plus those annotated with @customWeakMarking.

These new rules require reordering fields in two classes which didn't
already have all of their strong fields adjacent.

Bug: v8:7793
Change-Id: Ic9d741986afa7fc1be3de044af5cae11a3c64d8c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3261968
Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
Cr-Commit-Position: refs/heads/main@{#77739}

src/objects/feedback-vector.tq

@@ -21,9 +21,9 @@ extern class FeedbackVector extends HeapObject {
   // Padding was necessary for GCMole.
   flags: FeedbackVectorFlags;
   shared_function_info: SharedFunctionInfo;
+  closure_feedback_cell_array: ClosureFeedbackCellArray;
   @if(V8_EXTERNAL_CODE_SPACE) maybe_optimized_code: Weak<CodeDataContainer>;
   @ifnot(V8_EXTERNAL_CODE_SPACE) maybe_optimized_code: Weak<Code>;
-  closure_feedback_cell_array: ClosureFeedbackCellArray;
   @cppRelaxedLoad raw_feedback_slots[length]: MaybeObject;
 }
 

src/objects/map.tq

@@ -73,8 +73,8 @@ extern class Map extends HeapObject {
   instance_descriptors: DescriptorArray;
   dependent_code: DependentCode;
   prototype_validity_cell: Smi|Cell;
-  weak transitions_or_prototype_info: Map|Weak<Map>|TransitionArray|
-      PrototypeInfo|Smi;
+  transitions_or_prototype_info: Map|Weak<Map>|TransitionArray|PrototypeInfo|
+      Smi;
 }
 
 @export

src/objects/objects-body-descriptors-inl.h

@@ -562,7 +562,7 @@ class PrototypeInfo::BodyDescriptor final : public BodyDescriptorBase {
                                  ObjectVisitor* v) {
     IteratePointers(obj, HeapObject::kHeaderSize, kObjectCreateMapOffset, v);
     IterateMaybeWeakPointer(obj, kObjectCreateMapOffset, v);
-    IteratePointers(obj, kObjectCreateMapOffset + kTaggedSize, object_size, v);
+    STATIC_ASSERT(kObjectCreateMapOffset + kTaggedSize == kHeaderSize);
   }
 
   static inline int SizeOf(Map map, HeapObject obj) {

src/objects/prototype-info.tq

@@ -23,8 +23,8 @@ extern class PrototypeInfo extends Struct {
   // is stored. Returns UNREGISTERED if this prototype has not been registered.
   registry_slot: Smi;
 
+  bit_field: SmiTagged<PrototypeInfoFlags>;
+
   // [object_create_map]: A field caching the map for Object.create(prototype).
   object_create_map: Weak<Map>|Undefined;
-
-  bit_field: SmiTagged<PrototypeInfoFlags>;
 }

src/objects/shared-function-info.tq

@@ -52,7 +52,7 @@ class SharedFunctionInfo extends HeapObject {
   // field as a weak pointer if there is aged bytecode. If there is no bytecode
   // or if the bytecode is young then we treat it as a strong pointer. This is
   // done to support flushing of bytecode.
-  weak function_data: Object;
+  @customWeakMarking function_data: Object;
   name_or_scope_info: String|NoSharedNameSentinel|ScopeInfo;
   outer_scope_info_or_feedback_metadata: HeapObject;
   script_or_debug_info: Script|DebugInfo|Undefined;

src/torque/ast.h

@@ -937,7 +937,7 @@ struct ClassFieldExpression {
   NameAndTypeExpression name_and_type;
   base::Optional<ClassFieldIndexInfo> index;
   std::vector<ConditionalAnnotation> conditions;
-  bool weak;
+  bool custom_weak_marking;
   bool const_qualified;
   FieldSynchronization read_synchronization;
   FieldSynchronization write_synchronization;

src/torque/constants.h

@@ -115,6 +115,8 @@ static const char* const ANNOTATION_CPP_RELAXED_LOAD = "@cppRelaxedLoad";
 static const char* const ANNOTATION_CPP_RELEASE_STORE = "@cppReleaseStore";
 // Generate C++ accessors with acquire load semantics.
 static const char* const ANNOTATION_CPP_ACQUIRE_LOAD = "@cppAcquireLoad";
+// Generate BodyDescriptor using IterateCustomWeakPointers.
+static const char* const ANNOTATION_CUSTOM_WEAK_MARKING = "@customWeakMarking";
 
 inline bool IsConstexprName(const std::string& name) {
   return name.substr(0, std::strlen(CONSTEXPR_TYPE_PREFIX)) ==

src/torque/implementation-visitor.cc

@@ -3728,7 +3728,18 @@ class FieldOffsetsGenerator {
     if (auto field_as_struct = field_type->StructSupertype()) {
       struct_contents = (*field_as_struct)->ClassifyContents();
     }
-    if (struct_contents == StructType::ClassificationFlag::kMixed) {
+    if ((struct_contents & StructType::ClassificationFlag::kStrongTagged) &&
+        (struct_contents & StructType::ClassificationFlag::kWeakTagged)) {
+      // It's okay for a struct to contain both strong and weak data. We'll just
+      // treat the whole thing as weak. This is required for DescriptorEntry.
+      struct_contents &= ~StructType::Classification(
+          StructType::ClassificationFlag::kStrongTagged);
+    }
+    bool struct_contains_tagged_fields =
+        (struct_contents & StructType::ClassificationFlag::kStrongTagged) ||
+        (struct_contents & StructType::ClassificationFlag::kWeakTagged);
+    if (struct_contains_tagged_fields &&
+        (struct_contents & StructType::ClassificationFlag::kUntagged)) {
       // We can't declare what section a struct goes in if it has multiple
       // categories of data within.
       Error(
@@ -3736,15 +3747,13 @@ class FieldOffsetsGenerator {
           "tagged and untagged data.")
           .Position(f.pos);
     }
-    // Currently struct-valued fields are only allowed to have tagged data; see
-    // TypeVisitor::VisitClassFieldsAndMethods.
-    if (field_type->IsSubtypeOf(TypeOracle::GetTaggedType()) ||
-        struct_contents == StructType::ClassificationFlag::kTagged) {
-      if (f.is_weak) {
-        return FieldSectionType::kWeakSection;
-      } else {
-        return FieldSectionType::kStrongSection;
-      }
+    if ((field_type->IsSubtypeOf(TypeOracle::GetStrongTaggedType()) ||
+         struct_contents == StructType::ClassificationFlag::kStrongTagged) &&
+        !f.custom_weak_marking) {
+      return FieldSectionType::kStrongSection;
+    } else if (field_type->IsSubtypeOf(TypeOracle::GetTaggedType()) ||
+               struct_contains_tagged_fields) {
+      return FieldSectionType::kWeakSection;
     } else {
       return FieldSectionType::kScalarSection;
     }

src/torque/torque-parser.cc

@@ -1974,7 +1974,8 @@ base::Optional<ParseResult> MakeClassField(ParseResultIterator* child_results) {
   AnnotationSet annotations(
       child_results,
       {ANNOTATION_CPP_RELAXED_STORE, ANNOTATION_CPP_RELAXED_LOAD,
-       ANNOTATION_CPP_RELEASE_STORE, ANNOTATION_CPP_ACQUIRE_LOAD},
+       ANNOTATION_CPP_RELEASE_STORE, ANNOTATION_CPP_ACQUIRE_LOAD,
+       ANNOTATION_CUSTOM_WEAK_MARKING},
       {ANNOTATION_IF, ANNOTATION_IFNOT});
   FieldSynchronization write_synchronization = FieldSynchronization::kNone;
   if (annotations.Contains(ANNOTATION_CPP_RELEASE_STORE)) {
@@ -2000,7 +2001,16 @@ base::Optional<ParseResult> MakeClassField(ParseResultIterator* child_results) {
     conditions.push_back(
         {*ifnot_condition, ConditionalAnnotationType::kNegative});
   }
-  auto weak = child_results->NextAs<bool>();
+  bool custom_weak_marking =
+      annotations.Contains(ANNOTATION_CUSTOM_WEAK_MARKING);
+  auto deprecated_weak = child_results->NextAs<bool>();
+  if (deprecated_weak) {
+    Error(
+        "The keyword 'weak' is deprecated. For a field that can contain a "
+        "normal weak pointer, use type Weak<T>. For a field that should be "
+        "marked in some custom way, use @customWeakMarking.");
+    custom_weak_marking = true;
+  }
   auto const_qualified = child_results->NextAs<bool>();
   auto name = child_results->NextAs<Identifier*>();
   auto optional = child_results->NextAs<bool>();
@@ -2025,7 +2035,7 @@ base::Optional<ParseResult> MakeClassField(ParseResultIterator* child_results) {
   return ParseResult{ClassFieldExpression{{name, type},
                                           index_info,
                                           std::move(conditions),
-                                          weak,
+                                          custom_weak_marking,
                                           const_qualified,
                                           read_synchronization,
                                           write_synchronization}};

src/torque/type-visitor.cc

@@ -427,8 +427,8 @@ void TypeVisitor::VisitClassFieldsAndMethods(
             "found type ",
             *field_type);
       }
-      if (field_expression.weak) {
-        ReportError("in-object properties cannot be weak");
+      if (field_expression.custom_weak_marking) {
+        ReportError("in-object properties cannot use @customWeakMarking");
       }
     }
     base::Optional<ClassFieldIndexInfo> array_length = field_expression.index;
@@ -438,7 +438,7 @@ void TypeVisitor::VisitClassFieldsAndMethods(
          array_length,
          {field_expression.name_and_type.name->value, field_type},
          class_offset.SingleValue(),
-         field_expression.weak,
+         field_expression.custom_weak_marking,
          field_expression.const_qualified,
          field_expression.read_synchronization,
          field_expression.write_synchronization});

src/torque/types.cc

@@ -432,8 +432,10 @@ StructType::Classification StructType::ClassifyContents() const {
   Classification result = ClassificationFlag::kEmpty;
   for (const Field& struct_field : fields()) {
     const Type* field_type = struct_field.name_and_type.type;
-    if (field_type->IsSubtypeOf(TypeOracle::GetTaggedType())) {
-      result |= ClassificationFlag::kTagged;
+    if (field_type->IsSubtypeOf(TypeOracle::GetStrongTaggedType())) {
+      result |= ClassificationFlag::kStrongTagged;
+    } else if (field_type->IsSubtypeOf(TypeOracle::GetTaggedType())) {
+      result |= ClassificationFlag::kWeakTagged;
     } else if (auto field_as_struct = field_type->StructSupertype()) {
       result |= (*field_as_struct)->ClassifyContents();
     } else {
@@ -618,13 +620,13 @@ void ComputeSlotKindsHelper(std::vector<ObjectSlotKind>* slots,
     } else {
       ObjectSlotKind kind;
       if (type->IsSubtypeOf(TypeOracle::GetObjectType())) {
-        if (field.is_weak) {
+        if (field.custom_weak_marking) {
           kind = ObjectSlotKind::kCustomWeakPointer;
         } else {
           kind = ObjectSlotKind::kStrongPointer;
         }
       } else if (type->IsSubtypeOf(TypeOracle::GetTaggedType())) {
-        DCHECK(!field.is_weak);
+        DCHECK(!field.custom_weak_marking);
         kind = ObjectSlotKind::kMaybeObjectPointer;
       } else {
         kind = ObjectSlotKind::kNoPointer;
@@ -985,8 +987,8 @@ std::ostream& operator<<(std::ostream& os, const NameAndType& name_and_type) {
 
 std::ostream& operator<<(std::ostream& os, const Field& field) {
   os << field.name_and_type;
-  if (field.is_weak) {
-    os << " (weak)";
+  if (field.custom_weak_marking) {
+    os << " (custom weak)";
   }
   return os;
 }

src/torque/types.h

@@ -226,7 +226,7 @@ struct Field {
   // because we don't support the struct field for on-heap layouts.
   base::Optional<size_t> offset;
 
-  bool is_weak;
+  bool custom_weak_marking;
   bool const_qualified;
   FieldSynchronization read_synchronization;
   FieldSynchronization write_synchronization;
@@ -618,9 +618,9 @@ class StructType final : public AggregateType {
 
   enum class ClassificationFlag {
     kEmpty = 0,
-    kTagged = 1 << 0,
-    kUntagged = 1 << 1,
-    kMixed = kTagged | kUntagged,
+    kStrongTagged = 1 << 0,
+    kWeakTagged = 1 << 1,
+    kUntagged = 1 << 2,
   };
   using Classification = base::Flags<ClassificationFlag>;