[torque] Generate more detailed errors when instantiating generics

Currently it's pretty easy to write Torque code that generates an error
in some common generic function such as Convert<To: type, From: type>,
and unless your change is very small, it can be hard to figure out what
part of it caused that macro specialization. This CL updates the Torque
compiler to emit some extra information about the stack of code
positions that caused a specialization of a macro or builtin, similar to
what Clang does for C++ templates. Obviously there might be multiple
places that require a particular specialization, but we only report the
first one that caused the specialization to be created.

Bug: v8:7793
Change-Id: I4c0fbf1fd437d0eb0d7d5002baef7a5361aea5ee
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1911019
Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#64987}

src/torque/declarable.h

@@ -133,6 +133,28 @@ class Declarable {
     return static_cast<const x*>(declarable);                 \
   }
 
+// Information about what code caused a specialization to exist. This is used
+// for error reporting.
+struct SpecializationRequester {
+  // The position of the expression that caused this specialization.
+  SourcePosition position;
+  // The Scope which contains the expression that caused this specialization.
+  // It may in turn also be within a specialization, which allows us to print
+  // the stack of requesters when an error occurs.
+  Scope* scope;
+  // The name of the specialization.
+  std::string name;
+
+  static SpecializationRequester None() {
+    return {SourcePosition::Invalid(), nullptr, ""};
+  }
+
+  bool IsNone() const {
+    return position == SourcePosition::Invalid() && scope == nullptr &&
+           name == "";
+  }
+};
+
 class Scope : public Declarable {
  public:
   DECLARE_DECLARABLE_BOILERPLATE(Scope, scope)
@@ -174,8 +196,20 @@ class Scope : public Declarable {
     return declarable;
   }
 
+  const SpecializationRequester& GetSpecializationRequester() const {
+    return requester_;
+  }
+  void SetSpecializationRequester(const SpecializationRequester& requester) {
+    requester_ = requester;
+  }
+
  private:
   std::unordered_map<std::string, std::vector<Declarable*>> declarations_;
+
+  // If this Scope was created for specializing a generic type or callable,
+  // then {requester_} refers to the place that caused the specialization so we
+  // can construct useful error messages.
+  SpecializationRequester requester_ = SpecializationRequester::None();
 };
 
 class Namespace : public Scope {

src/torque/declaration-visitor.cc

@@ -294,10 +294,14 @@ Callable* DeclarationVisitor::SpecializeImplicit(
                 " with types <", key.specialized_types, "> declared at ",
                 key.generic->Position());
   }
+  SpecializationRequester requester{CurrentSourcePosition::Get(),
+                                    CurrentScope::Get(), ""};
   CurrentScope::Scope generic_scope(key.generic->ParentScope());
   Callable* result = Specialize(key, key.generic->declaration(), base::nullopt,
                                 body, CurrentSourcePosition::Get());
   result->SetIsUserDefined(false);
+  requester.name = result->ReadableName();
+  result->SetSpecializationRequester(requester);
   CurrentScope::Scope callable_scope(result);
   DeclareSpecializedTypes(key);
   return result;

src/torque/source-positions.h

@@ -38,6 +38,9 @@ struct LineAndColumn {
   bool operator==(const LineAndColumn& other) const {
     return line == other.line && column == other.column;
   }
+  bool operator!=(const LineAndColumn& other) const {
+    return !(*this == other);
+  }
 };
 
 struct SourcePosition {
@@ -66,6 +69,7 @@ struct SourcePosition {
   bool operator==(const SourcePosition& pos) const {
     return source == pos.source && start == pos.start && end == pos.end;
   }
+  bool operator!=(const SourcePosition& pos) const { return !(*this == pos); }
 };
 
 DECLARE_CONTEXTUAL_VARIABLE(CurrentSourceFile, SourceId);

src/torque/type-oracle.cc

@@ -37,16 +37,24 @@ const Type* TypeOracle::GetGenericTypeInstance(GenericType* generic_type,
   if (auto specialization = generic_type->GetSpecialization(arg_types)) {
     return *specialization;
   } else {
-    CurrentScope::Scope generic_scope(generic_type->ParentScope());
-    auto type = TypeVisitor::ComputeType(generic_type->declaration(),
-                                         {{generic_type, arg_types}});
+    const Type* type = nullptr;
+    // AddSpecialization can raise an error, which should be reported in the
+    // scope of the code requesting the specialization, not the generic type's
+    // parent scope, hence the following block.
+    {
+      v8::internal::torque::Scope* requester_scope = CurrentScope::Get();
+      CurrentScope::Scope generic_scope(generic_type->ParentScope());
+      type = TypeVisitor::ComputeType(generic_type->declaration(),
+                                      {{generic_type, arg_types}},
+                                      requester_scope);
+    }
     generic_type->AddSpecialization(arg_types, type);
     return type;
   }
 }
 
 // static
-Namespace* TypeOracle::CreateGenericTypeInstatiationNamespace() {
+Namespace* TypeOracle::CreateGenericTypeInstantiationNamespace() {
   Get().generic_type_instantiation_namespaces_.push_back(
       std::make_unique<Namespace>(GENERIC_TYPE_INSTANTIATION_NAMESPACE_STRING));
   return Get().generic_type_instantiation_namespaces_.back().get();

src/torque/type-oracle.h

@@ -277,7 +277,7 @@ class TypeOracle : public ContextualClass<TypeOracle> {
 
   static size_t FreshTypeId() { return Get().next_type_id_++; }
 
-  static Namespace* CreateGenericTypeInstatiationNamespace();
+  static Namespace* CreateGenericTypeInstantiationNamespace();
 
  private:
   const Type* GetBuiltinType(const std::string& name) {

src/torque/type-visitor.cc

@@ -16,11 +16,16 @@ namespace internal {
 namespace torque {
 
 const Type* TypeVisitor::ComputeType(TypeDeclaration* decl,
-                                     MaybeSpecializationKey specialized_from) {
+                                     MaybeSpecializationKey specialized_from,
+                                     Scope* specialization_requester) {
+  SourcePosition requester_position = CurrentSourcePosition::Get();
   CurrentSourcePosition::Scope scope(decl->pos);
   Scope* current_scope = CurrentScope::Get();
   if (specialized_from) {
-    current_scope = TypeOracle::CreateGenericTypeInstatiationNamespace();
+    current_scope = TypeOracle::CreateGenericTypeInstantiationNamespace();
+    current_scope->SetSpecializationRequester(
+        {requester_position, specialization_requester,
+         Type::ComputeName(decl->name->value, specialized_from)});
   }
   CurrentScope::Scope new_current_scope_scope(current_scope);
   if (specialized_from) {

src/torque/type-visitor.h

@@ -14,6 +14,8 @@ namespace v8 {
 namespace internal {
 namespace torque {
 
+class Scope;
+
 class TypeVisitor {
  public:
   static TypeVector ComputeTypeVector(const std::vector<TypeExpression*>& v) {
@@ -39,7 +41,8 @@ class TypeVisitor {
   friend class TypeOracle;
   static const Type* ComputeType(
       TypeDeclaration* decl,
-      MaybeSpecializationKey specialized_from = base::nullopt);
+      MaybeSpecializationKey specialized_from = base::nullopt,
+      Scope* specialization_requester = nullptr);
   static const AbstractType* ComputeType(
       AbstractTypeDeclaration* decl, MaybeSpecializationKey specialized_from);
   static const Type* ComputeType(TypeAliasDeclaration* decl,

src/torque/types.h

@@ -136,6 +136,9 @@ class V8_EXPORT_PRIVATE Type : public TypeBase {
   static base::Optional<const Type*> MatchUnaryGeneric(const Type* type,
                                                        GenericType* generic);
 
+  static std::string ComputeName(const std::string& basename,
+                                 MaybeSpecializationKey specialized_from);
+
  protected:
   Type(TypeBase::Kind kind, const Type* parent,
        MaybeSpecializationKey specialized_from = base::nullopt);
@@ -149,9 +152,6 @@ class V8_EXPORT_PRIVATE Type : public TypeBase {
   virtual std::string GetGeneratedTNodeTypeNameImpl() const = 0;
   virtual std::string SimpleNameImpl() const = 0;
 
-  static std::string ComputeName(const std::string& basename,
-                                 MaybeSpecializationKey specialized_from);
-
  private:
   bool IsAbstractName(const std::string& name) const;
 

src/torque/utils.cc

@@ -9,6 +9,7 @@
 
 #include "src/base/logging.h"
 #include "src/torque/ast.h"
+#include "src/torque/declarable.h"
 #include "src/torque/utils.h"
 
 namespace v8 {
@@ -130,10 +131,32 @@ MessageBuilder::MessageBuilder(const std::string& message,
     position = CurrentSourcePosition::Get();
   }
   message_ = TorqueMessage{message, position, kind};
+  if (CurrentScope::HasScope()) {
+    // Traverse the parent scopes to find one that was created to represent a
+    // specialization of something generic. If we find one, then log it and
+    // continue walking the scope tree of the code that requested that
+    // specialization. This allows us to collect the stack of locations that
+    // caused a specialization.
+    Scope* scope = CurrentScope::Get();
+    while (scope) {
+      SpecializationRequester requester = scope->GetSpecializationRequester();
+      if (!requester.IsNone()) {
+        extra_messages_.push_back(
+            {"Note: in specialization " + requester.name + " requested here",
+             requester.position, kind});
+        scope = requester.scope;
+      } else {
+        scope = scope->ParentScope();
+      }
+    }
+  }
 }
 
 void MessageBuilder::Report() const {
   TorqueMessages::Get().push_back(message_);
+  for (const auto& message : extra_messages_) {
+    TorqueMessages::Get().push_back(message);
+  }
 }
 
 [[noreturn]] void MessageBuilder::Throw() const {

src/torque/utils.h

@@ -66,6 +66,7 @@ class V8_EXPORT_PRIVATE MessageBuilder {
   void Report() const;
 
   TorqueMessage message_;
+  std::vector<TorqueMessage> extra_messages_;
 };
 
 // Used for throwing exceptions. Retrieve TorqueMessage from the contextual

test/unittests/torque/torque-unittest.cc

@@ -94,13 +94,58 @@ void ExpectSuccessfulCompilation(std::string source) {
 }
 
 template <class T>
-void ExpectFailingCompilation(
-    std::string source, ::testing::PolymorphicMatcher<T> message_pattern) {
+using MatcherVector =
+    std::vector<std::pair<::testing::PolymorphicMatcher<T>, LineAndColumn>>;
+
+template <class T>
+void ExpectFailingCompilation(std::string source,
+                              MatcherVector<T> message_patterns) {
   TorqueCompilerResult result = TestCompileTorque(std::move(source));
   ASSERT_FALSE(result.messages.empty());
-  EXPECT_THAT(result.messages[0].message, message_pattern);
+  EXPECT_GE(result.messages.size(), message_patterns.size());
+  size_t limit = message_patterns.size();
+  if (result.messages.size() < limit) {
+    limit = result.messages.size();
+  }
+  for (size_t i = 0; i < limit; ++i) {
+    EXPECT_THAT(result.messages[i].message, message_patterns[i].first);
+    if (message_patterns[i].second != LineAndColumn::Invalid()) {
+      base::Optional<SourcePosition> actual = result.messages[i].position;
+      EXPECT_TRUE(actual.has_value());
+      EXPECT_EQ(actual->start, message_patterns[i].second);
+    }
+  }
 }
 
+template <class T>
+void ExpectFailingCompilation(
+    std::string source, ::testing::PolymorphicMatcher<T> message_pattern) {
+  ExpectFailingCompilation(
+      source, MatcherVector<T>{{message_pattern, LineAndColumn::Invalid()}});
+}
+
+int CountPreludeLines() {
+  static int result = -1;
+  if (result == -1) {
+    std::string prelude(kTestTorquePrelude);
+    result = static_cast<int>(std::count(prelude.begin(), prelude.end(), '\n'));
+  }
+  return result;
+}
+
+using SubstrWithPosition =
+    std::pair<::testing::PolymorphicMatcher<
+                  ::testing::internal::HasSubstrMatcher<std::string>>,
+              LineAndColumn>;
+
+SubstrWithPosition SubstrTester(const std::string& message, int line, int col) {
+  // Change line and column from 1-based to 0-based.
+  return {::testing::HasSubstr(message),
+          LineAndColumn{line + CountPreludeLines() - 1, col - 1}};
+}
+
+using SubstrVector = std::vector<SubstrWithPosition>;
+
 }  // namespace
 
 TEST(Torque, Prelude) { ExpectSuccessfulCompilation(""); }
@@ -358,6 +403,87 @@ TEST(Torque, GenericAbstractType) {
                            HasSubstr("cannot find suitable callable"));
 }
 
+TEST(Torque, SpecializationRequesters) {
+  ExpectFailingCompilation(
+      R"(
+    macro A<T: type extends HeapObject>() {}
+    macro B<T: type>() {
+      A<T>();
+    }
+    macro C<T: type>() {
+      B<T>();
+    }
+    macro D() {
+      C<Smi>();
+    }
+  )",
+      SubstrVector{
+          SubstrTester("cannot find suitable callable", 4, 7),
+          SubstrTester("Note: in specialization B<Smi> requested here", 7, 7),
+          SubstrTester("Note: in specialization C<Smi> requested here", 10,
+                       7)});
+
+  ExpectFailingCompilation(
+      R"(
+    extern macro RetVal(): Object;
+    builtin A<T: type extends HeapObject>(implicit context: Context)(): Object {
+      return RetVal();
+    }
+    builtin B<T: type>(implicit context: Context)(): Object {
+      return A<T>();
+    }
+    builtin C<T: type>(implicit context: Context)(): Object {
+      return B<T>();
+    }
+    builtin D(implicit context: Context)(): Object {
+      return C<Smi>();
+    }
+  )",
+      SubstrVector{
+          SubstrTester("cannot find suitable callable", 7, 14),
+          SubstrTester("Note: in specialization B<Smi> requested here", 10, 14),
+          SubstrTester("Note: in specialization C<Smi> requested here", 13,
+                       14)});
+
+  ExpectFailingCompilation(
+      R"(
+    struct A<T: type extends HeapObject> {}
+    struct B<T: type> {
+      a: A<T>;
+    }
+    struct C<T: type> {
+      b: B<T>;
+    }
+    struct D {
+      c: C<Smi>;
+    }
+  )",
+      SubstrVector{
+          SubstrTester("Could not instantiate generic", 4, 10),
+          SubstrTester("Note: in specialization B<Smi> requested here", 7, 10),
+          SubstrTester("Note: in specialization C<Smi> requested here", 10,
+                       10)});
+
+  ExpectFailingCompilation(
+      R"(
+    macro A<T: type extends HeapObject>() {}
+    macro B<T: type>() {
+      A<T>();
+    }
+    struct C<T: type> {
+      Method() {
+        B<T>();
+      }
+    }
+    macro D(_b: C<Smi>) {}
+  )",
+      SubstrVector{
+          SubstrTester("cannot find suitable callable", 4, 7),
+          SubstrTester("Note: in specialization B<Smi> requested here", 8, 9),
+          SubstrTester("Note: in specialization C<Smi> requested here", 11,
+                       5)});
+}
+
 }  // namespace torque
 }  // namespace internal
 }  // namespace v8