[torque] Clarify class annotations

As Nico pointed out in [1], it is a little strange that the pair of
annotations "@export @customCppClass" behaves similarly to the keyword
"extern": both indicate that the class is defined in a C++ file and
Torque generates only a base class template for it. In this change, I
explore a possible alternative which might be more consistent.

Removed annotations:
- @customCppClass, which required @export, instructed Torque to only
  generate a base class template instead of a full class.
- @customMap, which also required @export, instructed Torque to not emit
  code for setting up a unique Map instance for the class.

Added annotations:
- @generateUniqueMap, which requires extern, instructs Torque to emit
  code for setting up a unique Map instance for the class.
- @generateFactoryFunction, which requires extern, instructs Torque to
  emit a function for creating a class instance.

Subtracting two annotations and adding two others still leaves us with
way too many annotations, but the usage of "extern" becomes more
consistent and I think that the new opt-in annotations might be easier
to understand.

[1] https://docs.google.com/document/d/1q_gZLnXd4bGnCx3IUfbln46K3bSs9UHBGasy9McQtHI/edit

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

src/objects/contexts.tq

@@ -3,12 +3,10 @@
 // found in the LICENSE file.
 
 @abstract
-@export
-@customCppClass
 // We normally don't generate a BodyDescriptor for an abstact class, but here we
 // do since all context classes share the same BodyDescriptor.
 @generateBodyDescriptor
-class Context extends HeapObject {
+extern class Context extends HeapObject {
   macro GetScopeInfo(): ScopeInfo {
     return *ContextSlot(this, ContextSlot::SCOPE_INFO_INDEX);
   }

src/objects/descriptor-array.tq

@@ -14,9 +14,9 @@ struct DescriptorEntry {
   value: JSAny|Weak<Map>|AccessorInfo|AccessorPair|ClassPositions;
 }
 
-@export
-@customCppClass
-class DescriptorArray extends HeapObject {
+@generateBodyDescriptor
+@generateUniqueMap
+extern class DescriptorArray extends HeapObject {
   const number_of_all_descriptors: uint16;
   number_of_descriptors: uint16;
   raw_number_of_marked_descriptors: uint16;

src/objects/object-list-macros.h

@@ -96,11 +96,13 @@ class ZoneForwardList;
   V(CompilationCacheTable)                     \
   V(ConsString)                                \
   V(Constructor)                               \
+  V(Context)                                   \
   V(CoverageInfo)                              \
   V(ClosureFeedbackCellArray)                  \
   V(DataHandler)                               \
   V(DeoptimizationData)                        \
   V(DependentCode)                             \
+  V(DescriptorArray)                           \
   V(EmbedderDataArray)                         \
   V(EphemeronHashTable)                        \
   V(ExternalOneByteString)                     \
@@ -187,6 +189,7 @@ class ZoneForwardList;
   V(NumberWrapper)                             \
   V(ObjectHashSet)                             \
   V(ObjectHashTable)                           \
+  V(Oddball)                                   \
   V(OrderedHashMap)                            \
   V(OrderedHashSet)                            \
   V(OrderedNameDictionary)                     \
@@ -203,6 +206,7 @@ class ZoneForwardList;
   V(SeqOneByteString)                          \
   V(SeqString)                                 \
   V(SeqTwoByteString)                          \
+  V(SharedFunctionInfo)                        \
   V(SimpleNumberDictionary)                    \
   V(SlicedString)                              \
   V(SmallOrderedHashMap)                       \
@@ -223,6 +227,9 @@ class ZoneForwardList;
   V(TemplateList)                              \
   V(ThinString)                                \
   V(TransitionArray)                           \
+  V(UncompiledData)                            \
+  V(UncompiledDataWithPreparseData)            \
+  V(UncompiledDataWithoutPreparseData)         \
   V(Undetectable)                              \
   V(UniqueName)                                \
   IF_WASM(V, WasmArray)                        \

src/objects/oddball.tq

@@ -2,12 +2,10 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-@export
-@customCppClass
-@customMap  // Oddballs have one of multiple maps, depending on the kind.
+@generateBodyDescriptor
 @apiExposedInstanceTypeValue(0x43)
 @highestInstanceTypeWithinParentClassRange
-class Oddball extends PrimitiveHeapObject {
+extern class Oddball extends PrimitiveHeapObject {
   to_number_raw: float64;
   to_string: String;
   to_number: Number;

src/objects/shared-function-info.tq

@@ -44,10 +44,8 @@ bitfield struct SharedFunctionInfoFlags2 extends uint8 {
   has_static_private_methods_or_accessors: bool: 1 bit;
 }
 
-@export
-@customCppClass
-@customMap  // Just to place the map at the beginning of the roots array.
-class SharedFunctionInfo extends HeapObject {
+@generateBodyDescriptor
+extern class SharedFunctionInfo extends HeapObject {
   // function_data field is treated as a custom weak pointer. We visit this
   // 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
@@ -118,22 +116,22 @@ macro IsSharedFunctionInfoDontAdaptArguments(sfi: SharedFunctionInfo): bool {
 }
 
 @abstract
-@export
-@customCppClass
-class UncompiledData extends HeapObject {
+extern class UncompiledData extends HeapObject {
   inferred_name: String;
   start_position: int32;
   end_position: int32;
 }
 
-@export
-@customCppClass
-class UncompiledDataWithoutPreparseData extends UncompiledData {
+@generateBodyDescriptor
+@generateUniqueMap
+@generateFactoryFunction
+extern class UncompiledDataWithoutPreparseData extends UncompiledData {
 }
 
-@export
-@customCppClass
-class UncompiledDataWithPreparseData extends UncompiledData {
+@generateBodyDescriptor
+@generateUniqueMap
+@generateFactoryFunction
+extern class UncompiledDataWithPreparseData extends UncompiledData {
   preparse_data: PreparseData;
 }
 

src/torque/constants.h

@@ -102,6 +102,9 @@ static const char* const ANNOTATION_IF = "@if";
 static const char* const ANNOTATION_IFNOT = "@ifnot";
 static const char* const ANNOTATION_GENERATE_BODY_DESCRIPTOR =
     "@generateBodyDescriptor";
+static const char* const ANNOTATION_GENERATE_UNIQUE_MAP = "@generateUniqueMap";
+static const char* const ANNOTATION_GENERATE_FACTORY_FUNCTION =
+    "@generateFactoryFunction";
 static const char* const ANNOTATION_EXPORT = "@export";
 static const char* const ANNOTATION_DO_NOT_GENERATE_CAST = "@doNotGenerateCast";
 static const char* const ANNOTATION_USE_PARENT_TYPE_CHECKER =
@@ -149,14 +152,14 @@ enum class ClassFlag {
   kIsShape = 1 << 3,
   kHasSameInstanceTypeAsParent = 1 << 4,
   kGenerateCppClassDefinitions = 1 << 5,
-  kCustomCppClass = 1 << 6,
-  kHighestInstanceTypeWithinParent = 1 << 7,
-  kLowestInstanceTypeWithinParent = 1 << 8,
-  kUndefinedLayout = 1 << 9,
-  kGenerateBodyDescriptor = 1 << 10,
-  kExport = 1 << 11,
-  kDoNotGenerateCast = 1 << 12,
-  kCustomMap = 1 << 13,
+  kHighestInstanceTypeWithinParent = 1 << 6,
+  kLowestInstanceTypeWithinParent = 1 << 7,
+  kUndefinedLayout = 1 << 8,
+  kGenerateBodyDescriptor = 1 << 9,
+  kExport = 1 << 10,
+  kDoNotGenerateCast = 1 << 11,
+  kGenerateUniqueMap = 1 << 12,
+  kGenerateFactoryFunction = 1 << 13,
 };
 using ClassFlags = base::Flags<ClassFlag>;
 

src/torque/implementation-visitor.cc

@@ -4697,8 +4697,7 @@ void ImplementationVisitor::GenerateClassDefinitions(
           structs_used_in_classes.insert(*field_as_struct);
         }
       }
-      if (type->ShouldExport() && !type->IsAbstract() &&
-          !type->HasCustomMap()) {
+      if (type->ShouldGenerateFactoryFunction()) {
         std::string return_type = type->HandlifiedCppTypeName();
         std::string function_name = "New" + type->name();
         std::stringstream parameters;

src/torque/instance-type-generator.cc

@@ -460,20 +460,23 @@ void ImplementationVisitor::GenerateInstanceTypes(
       std::string instance_type_name =
           CapifyStringWithUnderscores(type->name()) + "_TYPE";
 
-      if (type->IsExtern()) continue;
-      torque_defined_class_list << "  V(" << upper_case_name << ") \\\n";
-
-      if (type->IsAbstract() || type->HasCustomMap()) continue;
-      torque_defined_map_csa_list << "  V(_, " << upper_case_name << "Map, "
-                                  << lower_case_name << "_map, "
-                                  << upper_case_name << ") \\\n";
-      torque_defined_map_root_list << "  V(Map, " << lower_case_name << "_map, "
-                                   << upper_case_name << "Map) \\\n";
-      std::stringstream& list = type->HasStaticSize()
-                                    ? torque_defined_fixed_instance_type_list
-                                    : torque_defined_varsize_instance_type_list;
-      list << "  V(" << instance_type_name << ", " << upper_case_name << ", "
-           << lower_case_name << ") \\\n";
+      if (!type->IsExtern()) {
+        torque_defined_class_list << "  V(" << upper_case_name << ") \\\n";
+      }
+
+      if (type->ShouldGenerateUniqueMap()) {
+        torque_defined_map_csa_list << "  V(_, " << upper_case_name << "Map, "
+                                    << lower_case_name << "_map, "
+                                    << upper_case_name << ") \\\n";
+        torque_defined_map_root_list << "  V(Map, " << lower_case_name
+                                     << "_map, " << upper_case_name
+                                     << "Map) \\\n";
+        std::stringstream& list =
+            type->HasStaticSize() ? torque_defined_fixed_instance_type_list
+                                  : torque_defined_varsize_instance_type_list;
+        list << "  V(" << instance_type_name << ", " << upper_case_name << ", "
+             << lower_case_name << ") \\\n";
+      }
     }
 
     header << "// Fully Torque-defined classes (both internal and exported).\n";

src/torque/torque-parser.cc

@@ -899,6 +899,7 @@ base::Optional<ParseResult> MakeClassDeclaration(
        ANNOTATION_DO_NOT_GENERATE_CPP_CLASS, ANNOTATION_CUSTOM_CPP_CLASS,
        ANNOTATION_CUSTOM_MAP, ANNOTATION_GENERATE_BODY_DESCRIPTOR,
        ANNOTATION_EXPORT, ANNOTATION_DO_NOT_GENERATE_CAST,
+       ANNOTATION_GENERATE_UNIQUE_MAP, ANNOTATION_GENERATE_FACTORY_FUNCTION,
        ANNOTATION_HIGHEST_INSTANCE_TYPE_WITHIN_PARENT,
        ANNOTATION_LOWEST_INSTANCE_TYPE_WITHIN_PARENT},
       {ANNOTATION_RESERVE_BITS_IN_INSTANCE_TYPE,
@@ -913,10 +914,16 @@ base::Optional<ParseResult> MakeClassDeclaration(
   bool do_not_generate_cpp_class =
       annotations.Contains(ANNOTATION_DO_NOT_GENERATE_CPP_CLASS);
   if (annotations.Contains(ANNOTATION_CUSTOM_CPP_CLASS)) {
-    flags |= ClassFlag::kCustomCppClass;
+    Error(
+        "@customCppClass is deprecated. Use 'extern' instead. "
+        "@generateBodyDescriptor, @generateUniqueMap, and "
+        "@generateFactoryFunction accomplish most of what '@export "
+        "@customCppClass' used to.");
   }
   if (annotations.Contains(ANNOTATION_CUSTOM_MAP)) {
-    flags |= ClassFlag::kCustomMap;
+    Error(
+        "@customMap is deprecated. Generating a unique map is opt-in now using "
+        "@generateUniqueMap.");
   }
   if (annotations.Contains(ANNOTATION_DO_NOT_GENERATE_CAST)) {
     flags |= ClassFlag::kDoNotGenerateCast;
@@ -924,6 +931,12 @@ base::Optional<ParseResult> MakeClassDeclaration(
   if (annotations.Contains(ANNOTATION_GENERATE_BODY_DESCRIPTOR)) {
     flags |= ClassFlag::kGenerateBodyDescriptor;
   }
+  if (annotations.Contains(ANNOTATION_GENERATE_UNIQUE_MAP)) {
+    flags |= ClassFlag::kGenerateUniqueMap;
+  }
+  if (annotations.Contains(ANNOTATION_GENERATE_FACTORY_FUNCTION)) {
+    flags |= ClassFlag::kGenerateFactoryFunction;
+  }
   if (annotations.Contains(ANNOTATION_EXPORT)) {
     flags |= ClassFlag::kExport;
   }

src/torque/type-visitor.cc

@@ -287,15 +287,23 @@ const ClassType* TypeVisitor::ComputeType(
     Error("Class \"", decl->name->value,
           "\" requires a layout but doesn't have one");
   }
-  if (flags & ClassFlag::kCustomCppClass) {
-    if (!(flags & ClassFlag::kExport)) {
-      Error("Only exported classes can have a custom C++ class.");
+  if (flags & ClassFlag::kGenerateUniqueMap) {
+    if (!(flags & ClassFlag::kExtern)) {
+      Error("No need to specify ", ANNOTATION_GENERATE_UNIQUE_MAP,
+            ", non-extern classes always have a unique map.");
     }
-    if (flags & ClassFlag::kExtern) {
-      Error("No need to specify ", ANNOTATION_CUSTOM_CPP_CLASS,
-            ", extern classes always have a custom C++ class.");
+    if (flags & ClassFlag::kAbstract) {
+      Error(ANNOTATION_ABSTRACT, " and ", ANNOTATION_GENERATE_UNIQUE_MAP,
+            " shouldn't be used together, because abstract classes are never "
+            "instantiated.");
     }
   }
+  if ((flags & ClassFlag::kGenerateFactoryFunction) &&
+      (flags & ClassFlag::kAbstract)) {
+    Error(ANNOTATION_ABSTRACT, " and ", ANNOTATION_GENERATE_FACTORY_FUNCTION,
+          " shouldn't be used together, because abstract classes are never "
+          "instantiated.");
+  }
   if (flags & ClassFlag::kExtern) {
     if (decl->generates) {
       bool enforce_tnode_type = true;

src/torque/types.h

@@ -691,11 +691,15 @@ class ClassType final : public AggregateType {
   bool ShouldGenerateCppClassDefinitions() const {
     return (flags_ & ClassFlag::kGenerateCppClassDefinitions) || !IsExtern();
   }
-  bool ShouldGenerateFullClassDefinition() const {
-    return !IsExtern() && !(flags_ & ClassFlag::kCustomCppClass);
+  bool ShouldGenerateFullClassDefinition() const { return !IsExtern(); }
+  bool ShouldGenerateUniqueMap() const {
+    return (flags_ & ClassFlag::kGenerateUniqueMap) ||
+           (!IsExtern() && !IsAbstract());
+  }
+  bool ShouldGenerateFactoryFunction() const {
+    return (flags_ & ClassFlag::kGenerateFactoryFunction) ||
+           (ShouldExport() && !IsAbstract());
   }
-  // Class with multiple or non-standard maps, do not auto-generate map.
-  bool HasCustomMap() const { return flags_ & ClassFlag::kCustomMap; }
   bool ShouldExport() const { return flags_ & ClassFlag::kExport; }
   bool IsShape() const { return flags_ & ClassFlag::kIsShape; }
   bool HasStaticSize() const;