[torque] Lint errors for unused macros

This CL adds lint errors for unused Torque macros. To prevent lots of
noisy warnings, the check is rather narrow. Macros declared as "extern"
or marked with "@export" are ignored. Also macros starting with "Convert",
"Cast" or "FromConstexpr" are not checked.

Drive-by: Removing some unused macros.

Bug: v8:7793
Change-Id: Ie0d2e445f8882a9b0ebbda45876b342abf341248
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1645312
Commit-Queue: Simon Zünd <szuend@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#62092}

src/builtins/array-map.tq

@@ -220,19 +220,6 @@ namespace array_map {
     return vector.CreateJSArray(len);
   }
 
-  // Bails out if the slow path needs to be taken.
-  // It's useful to structure it this way, because the consequences of
-  // using the slow path on species creation are interesting to the caller.
-  macro FastMapSpeciesCreate(implicit context: Context)(
-      receiver: JSReceiver, length: Number): JSArray labels Bailout {
-    if (IsArraySpeciesProtectorCellInvalid()) goto Bailout;
-    Cast<FastJSArray>(receiver) otherwise Bailout;
-    const smiLength = Cast<Smi>(length) otherwise Bailout;
-    const newMap: Map =
-        LoadJSArrayElementsMap(PACKED_SMI_ELEMENTS, LoadNativeContext(context));
-    return AllocateJSArray(PACKED_SMI_ELEMENTS, newMap, smiLength, smiLength);
-  }
-
   // https://tc39.github.io/ecma262/#sec-array.prototype.map
   transitioning javascript builtin
   ArrayMap(implicit context: Context)(receiver: Object, ...arguments): Object {

src/builtins/array.tq

@@ -66,24 +66,5 @@ namespace array {
     elements.objects[k] = Hole;
   }
 
-  macro CopyArrayElement(
-      elements: FixedArray, newElements: FixedArray, from: Smi, to: Smi): void {
-    const e: Object = elements.objects[from];
-    newElements.objects[to] = e;
-  }
-
-  macro CopyArrayElement(
-      elements: FixedDoubleArray, newElements: FixedDoubleArray, from: Smi,
-      to: Smi): void {
-    try {
-      const floatValue: float64 = LoadDoubleWithHoleCheck(elements, from)
-          otherwise FoundHole;
-      newElements.floats[to] = floatValue;
-    }
-    label FoundHole {
-      StoreArrayHole(newElements, to);
-    }
-  }
-
   extern macro SetPropertyLength(implicit context: Context)(Object, Number);
 }

src/builtins/base.tq

@@ -289,11 +289,6 @@ extern class JSObject extends JSReceiver {
   @noVerifier elements: FixedArrayBase;
 }
 
-macro NewJSObject(
-    map: Map, properties: FixedArrayBase | Smi,
-    elements: FixedArrayBase): JSObject {
-  return new JSObject{map, properties_or_hash: properties, elements};
-}
 macro NewJSObject(implicit context: Context)(): JSObject {
   const objectFunction: JSFunction = GetObjectFunction();
   const map: Map = Cast<Map>(objectFunction.prototype_or_initial_map)
@@ -409,20 +404,6 @@ macro NewJSArray(implicit context: Context)(): JSArray {
   };
 }
 
-struct HoleIterator {
-  Next(): Object labels _NoMore {
-    return Hole;
-  }
-}
-
-macro NewJSArray(implicit context: Context)(_map: Map, length: Smi): JSArray {
-  const map = GetFastPackedSmiElementsJSArrayMap();
-  const i = HoleIterator{};
-  const elements = new FixedArray{map, length, objects: ...i};
-  return new
-  JSArray{map, properties_or_hash: kEmptyFixedArray, elements, length};
-}
-
 // A HeapObject with a JSArray map, and either fast packed elements, or fast
 // holey elements when the global NoElementsProtector is not invalidated.
 transient type FastJSArray extends JSArray;
@@ -2403,10 +2384,6 @@ extern operator '.floats[]=' macro StoreFixedDoubleArrayElement(
     FixedDoubleArray, intptr, float64): void;
 extern operator '.floats[]=' macro StoreFixedDoubleArrayElementSmi(
     FixedDoubleArray, Smi, float64): void;
-operator '.floats[]=' macro StoreFixedDoubleArrayElementSmi(
-    a: FixedDoubleArray, i: Smi, n: Number): void {
-  StoreFixedDoubleArrayElementSmi(a, i, Convert<float64>(n));
-}
 operator '[]=' macro StoreFixedDoubleArrayDirect(
     a: FixedDoubleArray, i: Smi, v: Number) {
   a.floats[i] = Convert<float64>(v);
@@ -2896,19 +2873,6 @@ macro BranchIfFastJSArrayForRead(o: Object, context: Context):
   BranchIf<FastJSArrayForRead>(o) otherwise True, False;
 }
 
-macro BranchIfNotFastJSArray(o: Object, context: Context): never
-    labels True, False {
-  BranchIfNot<FastJSArray>(o) otherwise True, False;
-}
-
-macro BranchIfFastJSArrayForCopy(o: Object, context: Context): never
-    labels True, False {
-  // Long-term, it's likely not a good idea to have this slow-path test here,
-  // since it fundamentally breaks the type system.
-  GotoIfForceSlowPath() otherwise False;
-  BranchIf<FastJSArrayForCopy>(o) otherwise True, False;
-}
-
 @export
 macro IsFastJSArrayWithNoCustomIteration(context: Context, o: Object): bool {
   return Is<FastJSArrayWithNoCustomIteration>(o);

src/builtins/bigint.tq

@@ -44,6 +44,9 @@ namespace bigint {
       MutableBigInt, intptr, uintptr): void;
   extern macro CodeStubAssembler::LoadBigIntDigit(BigIntBase, intptr): uintptr;
 
+  @export  // Silence unused warning.
+           // TODO(szuend): Remove @export once macros that are only used in
+           //               asserts are no longer detected as unused.
   macro IsCanonicalized(bigint: BigIntBase): bool {
     const length = ReadBigIntLength(bigint);
 
@@ -58,10 +61,6 @@ namespace bigint {
     return sign == kPositiveSign ? kNegativeSign : kPositiveSign;
   }
 
-  macro WriteBigIntSign(bigint: MutableBigInt, sign: uint32): void {
-    WriteBigIntSignAndLength(bigint, sign, ReadBigIntLength(bigint));
-  }
-
   macro AllocateEmptyBigInt(implicit context: Context)(
       sign: uint32, length: intptr): MutableBigInt {
     if (length > kBigIntMaxLength) {

src/builtins/internal-coverage.tq

@@ -28,6 +28,8 @@ namespace internal_coverage {
     return UnsafeCast<CoverageInfo>(debugInfo.coverage_info);
   }
 
+  @export  // Silence unused warning on release builds. SlotCount is only used
+           // in an assert. TODO(szuend): Remove once macros and asserts work.
   macro SlotCount(coverageInfo: CoverageInfo): Smi {
     assert(kFirstSlotIndex == 0);  // Otherwise we'd have to consider it below.
     assert(kFirstSlotIndex == (coverageInfo.length & kSlotIndexCountMask));

src/builtins/typed-array.tq

@@ -76,18 +76,6 @@ namespace typed_array {
   type LoadFn = builtin(Context, JSTypedArray, Smi) => Object;
   type StoreFn = builtin(Context, JSTypedArray, Smi, Object) => Object;
 
-  // These UnsafeCast specializations are necessary becuase there is no
-  // way to definitively test whether an Object is a Torque function
-  // with a specific signature, and the default UnsafeCast implementation
-  // would try to check this through an assert(Is<>), so the test
-  // is bypassed in this specialization.
-  UnsafeCast<LoadFn>(implicit context: Context)(o: Object): LoadFn {
-    return %RawDownCast<LoadFn>(o);
-  }
-  UnsafeCast<StoreFn>(implicit context: Context)(o: Object): StoreFn {
-    return %RawDownCast<StoreFn>(o);
-  }
-
   // AttachedJSTypedArray guards that the array's buffer is not detached.
   transient type AttachedJSTypedArray extends JSTypedArray;
 

src/torque/declarable.h

@@ -312,16 +312,23 @@ class Macro : public Callable {
     return Callable::ShouldBeInlined();
   }
 
+  void SetUsed() { used_ = true; }
+  bool IsUsed() const { return used_; }
+
  protected:
   Macro(Declarable::Kind kind, std::string external_name,
         std::string readable_name, const Signature& signature,
         bool transitioning, base::Optional<Statement*> body)
       : Callable(kind, std::move(external_name), std::move(readable_name),
-                 signature, transitioning, body) {
+                 signature, transitioning, body),
+        used_(false) {
     if (signature.parameter_types.var_args) {
       ReportError("Varargs are not supported for macros.");
     }
   }
+
+ private:
+  bool used_;
 };
 
 class ExternMacro : public Macro {

src/torque/implementation-visitor.cc

@@ -979,10 +979,13 @@ const Type* ImplementationVisitor::Visit(AssertStatement* stmt) {
         for (Identifier* label : call->labels) {
           LabelBindingsManager::Get().TryLookup(label->value);
         }
+        // TODO(szuend): In case the call expression resolves to a macro
+        //               callable, mark the macro as used as well.
       } else if (auto call = CallMethodExpression::DynamicCast(expression)) {
         for (Identifier* label : call->labels) {
           LabelBindingsManager::Get().TryLookup(label->value);
         }
+        // TODO(szuend): Mark the underlying macro as used.
       }
     });
   }
@@ -2127,6 +2130,7 @@ VisitResult ImplementationVisitor::GenerateCall(
     if (is_tailcall) {
       ReportError("can't tail call a macro");
     }
+    macro->SetUsed();
     if (return_type->IsConstexpr()) {
       DCHECK_EQ(0, arguments.labels.size());
       std::stringstream result;
@@ -3552,6 +3556,33 @@ void ImplementationVisitor::GenerateCSATypes(
   WriteFile(output_directory + "/" + file_name + ".h", h_contents.str());
 }
 
+void ReportAllUnusedMacros() {
+  for (const auto& declarable : GlobalContext::AllDeclarables()) {
+    if (!declarable->IsMacro() || declarable->IsExternMacro()) continue;
+
+    Macro* macro = Macro::cast(declarable.get());
+    if (macro->IsUsed()) continue;
+
+    if (macro->IsTorqueMacro() && TorqueMacro::cast(macro)->IsExportedToCSA()) {
+      continue;
+    }
+
+    std::vector<std::string> ignored_prefixes = {"Convert<", "Cast<",
+                                                 "FromConstexpr<"};
+    const std::string name = macro->ReadableName();
+    const bool ignore =
+        std::any_of(ignored_prefixes.begin(), ignored_prefixes.end(),
+                    [&name](const std::string& prefix) {
+                      return StringStartsWith(name, prefix);
+                    });
+
+    if (!ignore) {
+      Lint("Macro '", macro->ReadableName(), "' is never used.")
+          .Position(macro->IdentifierPosition());
+    }
+  }
+}
+
 }  // namespace torque
 }  // namespace internal
 }  // namespace v8

src/torque/implementation-visitor.h

@@ -677,6 +677,8 @@ class ImplementationVisitor {
   bool is_dry_run_;
 };
 
+void ReportAllUnusedMacros();
+
 }  // namespace torque
 }  // namespace internal
 }  // namespace v8

src/torque/torque-compiler.cc

@@ -76,6 +76,8 @@ void CompileCurrentAst(TorqueCompilerOptions options) {
 
   implementation_visitor.VisitAllDeclarables();
 
+  ReportAllUnusedMacros();
+
   implementation_visitor.GenerateBuiltinDefinitions(output_directory);
   implementation_visitor.GenerateClassFieldOffsets(output_directory);
   implementation_visitor.GeneratePrintDefinitions(output_directory);

test/torque/test-torque.tq

@@ -11,11 +11,7 @@ namespace test {
     }
   }
 
-  macro ElementsKindTestHelper2(kind: constexpr ElementsKind): bool {
+  macro ElementsKindTestHelper2(kind: constexpr ElementsKind): constexpr bool {
-    return ((kind == UINT8_ELEMENTS) || (kind == UINT16_ELEMENTS));
-  }
-
-  macro ElementsKindTestHelper3(kind: constexpr ElementsKind): constexpr bool {
     return ((kind == UINT8_ELEMENTS) || (kind == UINT16_ELEMENTS));
   }
 
@@ -48,10 +44,10 @@ namespace test {
 
   @export
   macro TestConstexprReturn() {
-    check(FromConstexpr<bool>(ElementsKindTestHelper3(UINT8_ELEMENTS)));
+    check(FromConstexpr<bool>(ElementsKindTestHelper2(UINT8_ELEMENTS)));
-    check(FromConstexpr<bool>(ElementsKindTestHelper3(UINT16_ELEMENTS)));
+    check(FromConstexpr<bool>(ElementsKindTestHelper2(UINT16_ELEMENTS)));
-    check(!FromConstexpr<bool>(ElementsKindTestHelper3(UINT32_ELEMENTS)));
+    check(!FromConstexpr<bool>(ElementsKindTestHelper2(UINT32_ELEMENTS)));
-    check(FromConstexpr<bool>(!ElementsKindTestHelper3(UINT32_ELEMENTS)));
+    check(FromConstexpr<bool>(!ElementsKindTestHelper2(UINT32_ELEMENTS)));
   }
 
   @export
@@ -344,6 +340,7 @@ namespace test {
   Foo(TestStructA) {
     goto Foo(TestStruct2());
   }
+  @export  // Silence unused warning.
   macro CallTestStructInLabel(implicit context: Context)() {
     try {
       TestStructInLabel() otherwise Foo;
@@ -787,7 +784,7 @@ namespace test {
   @export
   macro TestNew(implicit context: Context)() {
     const f: JSArray = NewJSArray();
-    assert(f.IsEmpty());
+    check(f.IsEmpty());
     f.length = 0;
   }
 
@@ -812,15 +809,15 @@ namespace test {
   macro TestStructConstructor(implicit context: Context)() {
     // Test default constructor
     let a: TestOuter = TestOuter{a: 5, b: TestInner{x: 6, y: 7}, c: 8};
-    assert(a.a == 5);
+    check(a.a == 5);
-    assert(a.b.x == 6);
+    check(a.b.x == 6);
-    assert(a.b.y == 7);
+    check(a.b.y == 7);
-    assert(a.c == 8);
+    check(a.c == 8);
     a.b.x = 1;
-    assert(a.b.x == 1);
+    check(a.b.x == 1);
     a.b.SetX(2);
-    assert(a.b.x == 2);
+    check(a.b.x == 2);
-    assert(a.b.GetX() == 2);
+    check(a.b.GetX() == 2);
   }
 
   class InternalClass {
@@ -862,6 +859,9 @@ namespace test {
     let y = StructWithConst{a: Null, b: 1};
     y.a = Undefined;
     const _copy = x;
+
+    check(x.TestMethod1() == 1);
+    check(x.TestMethod2() == Null);
   }
 
   struct TestIterator {
@@ -924,5 +924,4 @@ namespace test {
     const v2 = (box.unrelated == 0) ? box.value : box.value;
     StaticAssert(WordEqual(v1, v2));
   }
-
 }

test/unittests/torque/torque-unittest.cc

@@ -129,6 +129,7 @@ TEST(Torque, ClassDefinition) {
       i: uintptr;
     }
 
+    @export
     macro TestClassWithAllTypesLoadsAndStores(
         t: TestClassWithAllTypes, r: RawPtr, v1: int8, v2: uint8, v3: int16,
         v4: uint16, v5: int32, v6: uint32, v7: intptr, v8: uintptr) {
@@ -210,13 +211,13 @@ TEST(Torque, ConditionalFields) {
 
 TEST(Torque, ConstexprLetBindingDoesNotCrash) {
   ExpectFailingCompilation(
-      R"(macro FooBar() { let foo = 0; check(foo >= 0); })",
+      R"(@export macro FooBar() { let foo = 0; check(foo >= 0); })",
       HasSubstr("Use 'const' instead of 'let' for variable 'foo'"));
 }
 
 TEST(Torque, DoubleUnderScorePrefixIllegalForIdentifiers) {
   ExpectFailingCompilation(R"(
-    macro Foo() {
+    @export macro Foo() {
       let __x;
     }
   )",
@@ -225,7 +226,7 @@ TEST(Torque, DoubleUnderScorePrefixIllegalForIdentifiers) {
 
 TEST(Torque, UnusedLetBindingLintError) {
   ExpectFailingCompilation(R"(
-    macro Foo(y: Smi) {
+    @export macro Foo(y: Smi) {
       let x: Smi = y;
     }
   )",
@@ -234,7 +235,7 @@ TEST(Torque, UnusedLetBindingLintError) {
 
 TEST(Torque, UnderscorePrefixSilencesUnusedWarning) {
   ExpectSuccessfulCompilation(R"(
-    macro Foo(y: Smi) {
+    @export macro Foo(y: Smi) {
       let _x: Smi = y;
     }
   )");
@@ -242,7 +243,7 @@ TEST(Torque, UnderscorePrefixSilencesUnusedWarning) {
 
 TEST(Torque, UsingUnderscorePrefixedIdentifierError) {
   ExpectFailingCompilation(R"(
-    macro Foo(y: Smi) {
+    @export macro Foo(y: Smi) {
       let _x: Smi = y;
       check(_x == y);
     }
@@ -252,39 +253,39 @@ TEST(Torque, UsingUnderscorePrefixedIdentifierError) {
 
 TEST(Torque, UnusedArgumentLintError) {
   ExpectFailingCompilation(R"(
-    macro Foo(x: Smi) {}
+    @export macro Foo(x: Smi) {}
   )",
                            HasSubstr("Variable 'x' is never used."));
 }
 
 TEST(Torque, UsingUnderscorePrefixedArgumentSilencesWarning) {
   ExpectSuccessfulCompilation(R"(
-    macro Foo(_y: Smi) {}
+    @export macro Foo(_y: Smi) {}
   )");
 }
 
 TEST(Torque, UnusedLabelLintError) {
   ExpectFailingCompilation(R"(
-    macro Foo() labels Bar {}
+    @export macro Foo() labels Bar {}
   )",
                            HasSubstr("Label 'Bar' is never used."));
 }
 
 TEST(Torque, UsingUnderScorePrefixLabelSilencesWarning) {
   ExpectSuccessfulCompilation(R"(
-    macro Foo() labels _Bar {}
+    @export macro Foo() labels _Bar {}
   )");
 }
 
 TEST(Torque, NoUnusedWarningForImplicitArguments) {
   ExpectSuccessfulCompilation(R"(
-    macro Foo(implicit c: Context, r: JSReceiver)() {}
+    @export macro Foo(implicit c: Context, r: JSReceiver)() {}
   )");
 }
 
 TEST(Torque, NoUnusedWarningForVariablesOnlyUsedInAsserts) {
   ExpectSuccessfulCompilation(R"(
-    macro Foo(x: bool) {
+    @export macro Foo(x: bool) {
       assert(x);
     }
   )");
@@ -297,7 +298,7 @@ TEST(Torque, ImportNonExistentFile) {
 
 TEST(Torque, LetShouldBeConstLintError) {
   ExpectFailingCompilation(R"(
-    macro Foo(y: Smi): Smi {
+    @export macro Foo(y: Smi): Smi {
       let x: Smi = y;
       return x;
     })",
@@ -307,7 +308,7 @@ TEST(Torque, LetShouldBeConstLintError) {
 TEST(Torque, LetShouldBeConstIsSkippedForStructs) {
   ExpectSuccessfulCompilation(R"(
     struct Foo{ a: Smi; }
-    macro Bar(x: Smi): Foo {
+    @export macro Bar(x: Smi): Foo {
       let foo = Foo{a: x};
       return foo;
     }