Browse Source

Add a diagnostic note for errors during identifying facet types (#6445)

Errors that occur while constructing a specific should be tied back to
the facet type being identified. We don't have an InstId for the facet
type during identify, so provide the means to Stringify a FacetTypeId.

Depends on https://github.com/carbon-language/carbon-lang/pull/6435
Dana Jansens 5 months ago
parent
commit
73e6994d44

+ 3 - 0
toolchain/check/diagnostic_emitter.cpp

@@ -115,6 +115,9 @@ auto DiagnosticEmitter::ConvertArg(llvm::Any arg) const -> llvm::Any {
                                  sem_ir_->types().GetInstId(*type_id)) +
            "`";
   }
+  if (auto* facet_type_id = llvm::any_cast<SemIR::FacetTypeId>(&arg)) {
+    return "`" + StringifyFacetType(*sem_ir_, *facet_type_id) + "`";
+  }
   if (auto* specific_id = llvm::any_cast<SemIR::SpecificId>(&arg)) {
     return "`" + StringifySpecific(*sem_ir_, *specific_id) + "`";
   }

+ 10 - 4
toolchain/check/testdata/generic/identify_specific_facet_type.carbon

@@ -18,13 +18,16 @@ library "[[@TEST_NAME]]";
 
 interface K(T:! type) {}
 constraint J(N:! i32) {
-  // CHECK:STDERR: fail_error_in_identifying_require_facet_type.carbon:[[@LINE+3]]:30: error: array bound of -1 is negative [ArrayBoundNegative]
+  // CHECK:STDERR: fail_error_in_identifying_require_facet_type.carbon:[[@LINE+6]]:30: error: array bound of -1 is negative [ArrayBoundNegative]
   // CHECK:STDERR:   require impls K(array(i32, N));
   // CHECK:STDERR:                              ^
+  // CHECK:STDERR: fail_error_in_identifying_require_facet_type.carbon:[[@LINE+3]]:3: note: in `require` used here [ResolvingSpecificHere]
+  // CHECK:STDERR:   require impls K(array(i32, N));
+  // CHECK:STDERR:   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   require impls K(array(i32, N));
 }
 
-// CHECK:STDERR: fail_error_in_identifying_require_facet_type.carbon:[[@LINE+8]]:12: note: in `require` used here [ResolvingSpecificHere]
+// CHECK:STDERR: fail_error_in_identifying_require_facet_type.carbon:[[@LINE+8]]:12: note: identifying facet type `J(-1)` here [IdentifyingFacetTypeHere]
 // CHECK:STDERR: impl {} as J(-1) {}
 // CHECK:STDERR:            ^~~~~
 // CHECK:STDERR:
@@ -44,13 +47,16 @@ library "[[@TEST_NAME]]";
 
 interface K(T:! type) {}
 constraint J(N:! i32) {
-  // CHECK:STDERR: fail_error_in_identifying_extend_require_facet_type.carbon:[[@LINE+3]]:37: error: array bound of -1 is negative [ArrayBoundNegative]
+  // CHECK:STDERR: fail_error_in_identifying_extend_require_facet_type.carbon:[[@LINE+6]]:37: error: array bound of -1 is negative [ArrayBoundNegative]
   // CHECK:STDERR:   extend require impls K(array(i32, N));
   // CHECK:STDERR:                                     ^
+  // CHECK:STDERR: fail_error_in_identifying_extend_require_facet_type.carbon:[[@LINE+3]]:3: note: in `require` used here [ResolvingSpecificHere]
+  // CHECK:STDERR:   extend require impls K(array(i32, N));
+  // CHECK:STDERR:   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   extend require impls K(array(i32, N));
 }
 
-// CHECK:STDERR: fail_error_in_identifying_extend_require_facet_type.carbon:[[@LINE+8]]:12: note: in `require` used here [ResolvingSpecificHere]
+// CHECK:STDERR: fail_error_in_identifying_extend_require_facet_type.carbon:[[@LINE+8]]:12: note: identifying facet type `J(-1)` here [IdentifyingFacetTypeHere]
 // CHECK:STDERR: impl {} as J(-1) {}
 // CHECK:STDERR:            ^~~~~
 // CHECK:STDERR:

+ 14 - 4
toolchain/check/type_completion.cpp

@@ -811,7 +811,7 @@ static auto RequireIdentifiedNamedConstraints(
 // introduce new generic bindings, the specific for the RequireImpls can be
 // constructed from the enclosing one.
 static auto GetRequireImplsSpecificFromEnclosingSpecific(
-    Context& context, SemIR::LocId loc_id, const SemIR::RequireImpls& require,
+    Context& context, const SemIR::RequireImpls& require,
     SemIR::SpecificId enclosing_specific_id) -> SemIR::SpecificId {
   auto enclosing_specific_args_id =
       context.specifics().GetArgsOrEmpty(enclosing_specific_id);
@@ -836,7 +836,8 @@ static auto GetRequireImplsSpecificFromEnclosingSpecific(
   CARBON_CHECK(context.insts().Is<SemIR::SymbolicBinding>(self_inst_id));
   arg_ids.push_back(self_inst_id);
 
-  return MakeSpecific(context, loc_id, require.generic_id, arg_ids);
+  return MakeSpecific(context, SemIR::LocId(require.decl_id),
+                      require.generic_id, arg_ids);
 }
 
 // Returns the `facet_type` mapped into `specific_id`. If an error results, it
@@ -903,6 +904,15 @@ auto RequireIdentifiedFacetType(Context& context, SemIR::LocId loc_id,
     }
     llvm::append_range(self_impls, facet_type_info.self_impls_constraints);
 
+    Diagnostics::AnnotationScope annotate_diagnostics(
+        &context.emitter(), [&](auto& builder) {
+          CARBON_DIAGNOSTIC(IdentifyingFacetTypeHere, Note,
+                            "identifying facet type {0} here",
+                            SemIR::FacetTypeId);
+          builder.Note(loc_id, IdentifyingFacetTypeHere,
+                       facet_type.facet_type_id);
+        });
+
     for (auto extends : facet_type_info.extend_named_constraints) {
       const auto& constraint =
           context.named_constraints().Get(extends.named_constraint_id);
@@ -910,7 +920,7 @@ auto RequireIdentifiedFacetType(Context& context, SemIR::LocId loc_id,
                constraint.require_impls_block_id)) {
         const auto& require = context.require_impls().Get(require_impls_id);
         auto require_specific_id = GetRequireImplsSpecificFromEnclosingSpecific(
-            context, loc_id, require, extends.specific_id);
+            context, require, extends.specific_id);
         auto facet_type_id = TryGetFacetTypeInSpecific(
             context, require.facet_type_inst_id, require_specific_id);
         if (facet_type_id.has_value()) {
@@ -930,7 +940,7 @@ auto RequireIdentifiedFacetType(Context& context, SemIR::LocId loc_id,
                constraint.require_impls_block_id)) {
         const auto& require = context.require_impls().Get(require_impls_id);
         auto require_specific_id = GetRequireImplsSpecificFromEnclosingSpecific(
-            context, loc_id, require, impls.specific_id);
+            context, require, impls.specific_id);
         auto facet_type_id = TryGetFacetTypeInSpecific(
             context, require.facet_type_inst_id, require_specific_id);
         if (facet_type_id.has_value()) {

+ 3 - 0
toolchain/diagnostics/diagnostic_kind.def

@@ -530,6 +530,9 @@ CARBON_DIAGNOSTIC_KIND(FacetTypeConstraintCycle)
 // Facet type combination.
 CARBON_DIAGNOSTIC_KIND(FacetTypeRequiredForTypeAndOperator)
 
+// Facet type completeness.
+CARBON_DIAGNOSTIC_KIND(IdentifyingFacetTypeHere)
+
 // Generics.
 CARBON_DIAGNOSTIC_KIND(GenericMissingExplicitParameters)
 

+ 1 - 0
toolchain/sem_ir/ids.h

@@ -354,6 +354,7 @@ struct AssociatedConstantId : public IdBase<AssociatedConstantId> {
 // The ID of a `FacetTypeInfo`.
 struct FacetTypeId : public IdBase<FacetTypeId> {
   static constexpr llvm::StringLiteral Label = "facet_type";
+  using DiagnosticType = Diagnostics::TypeInfo<std::string>;
 
   using IdBase::IdBase;
 };

+ 77 - 59
toolchain/sem_ir/stringify.cpp

@@ -48,7 +48,8 @@ class StepStack {
  public:
   // An individual step in the stack, which stringifies some component of a type
   // name.
-  using Step = std::variant<InstId, llvm::StringRef, NameId, ElementIndex>;
+  using Step =
+      std::variant<InstId, llvm::StringRef, NameId, ElementIndex, FacetTypeId>;
 
   // Support `Push` for a qualified name. e.g., `A.B.C`.
   using QualifiedNameItem = std::pair<NameScopeId, NameId>;
@@ -74,6 +75,9 @@ class StepStack {
   auto PushElementIndex(ElementIndex element_index) -> void {
     steps_.push_back(element_index);
   }
+  auto PushFacetType(FacetTypeId facet_type_id) -> void {
+    steps_.push_back(facet_type_id);
+  }
 
   // Pushes all components of a qualified name (`A.B.C`) onto the stack.
   auto PushQualifiedName(NameScopeId name_scope_id, NameId name_id) -> void {
@@ -335,64 +339,7 @@ class Stringifier {
   }
 
   auto StringifyInst(InstId /*inst_id*/, FacetType inst) -> void {
-    const FacetTypeInfo& facet_type_info =
-        sem_ir_->facet_types().Get(inst.facet_type_id);
-    // Output `where` restrictions.
-    bool some_where = false;
-    if (facet_type_info.other_requirements) {
-      step_stack_->PushString("...");
-      some_where = true;
-    }
-    if (facet_type_info.builtin_constraint_mask.HasAnyOf(
-            SemIR::BuiltinConstraintMask::TypeCanDestroy)) {
-      if (some_where) {
-        step_stack_->PushString(" and");
-      }
-      step_stack_->PushString(" .Self impls Core.CanDestroy");
-      some_where = true;
-    }
-    for (auto rewrite : llvm::reverse(facet_type_info.rewrite_constraints)) {
-      if (some_where) {
-        step_stack_->PushString(" and");
-      }
-      step_stack_->Push(" ", rewrite.lhs_id, " = ", rewrite.rhs_id);
-      some_where = true;
-    }
-    if (!facet_type_info.self_impls_constraints.empty() ||
-        !facet_type_info.self_impls_named_constraints.empty()) {
-      if (some_where) {
-        step_stack_->PushString(" and");
-      }
-      llvm::ListSeparator sep(" & ");
-      for (auto impls :
-           llvm::reverse(facet_type_info.self_impls_named_constraints)) {
-        step_stack_->Push(impls, &sep);
-      }
-      for (auto impls : llvm::reverse(facet_type_info.self_impls_constraints)) {
-        step_stack_->Push(impls, &sep);
-      }
-      step_stack_->PushString(" .Self impls ");
-      some_where = true;
-    }
-    // TODO: Other restrictions from facet_type_info.
-    if (some_where) {
-      step_stack_->PushString(" where");
-    }
-
-    // Output extend interface and named constraint requirements.
-    if (facet_type_info.extend_constraints.empty() &&
-        facet_type_info.extend_named_constraints.empty()) {
-      step_stack_->PushString("type");
-      return;
-    }
-    llvm::ListSeparator sep(" & ");
-    for (auto extend :
-         llvm::reverse(facet_type_info.extend_named_constraints)) {
-      step_stack_->Push(extend, &sep);
-    }
-    for (auto extend : llvm::reverse(facet_type_info.extend_constraints)) {
-      step_stack_->Push(extend, &sep);
-    }
+    step_stack_->PushFacetType(inst.facet_type_id);
   }
 
   auto StringifyInst(InstId /*inst_id*/, FacetValue inst) -> void {
@@ -721,6 +668,67 @@ class Stringifier {
     *out_ << "<vtable ptr>";
   }
 
+  auto StringifyFacetType(FacetTypeId facet_type_id) -> void {
+    const FacetTypeInfo& facet_type_info =
+        sem_ir_->facet_types().Get(facet_type_id);
+    // Output `where` restrictions.
+    bool some_where = false;
+    if (facet_type_info.other_requirements) {
+      step_stack_->PushString("...");
+      some_where = true;
+    }
+    if (facet_type_info.builtin_constraint_mask.HasAnyOf(
+            SemIR::BuiltinConstraintMask::TypeCanDestroy)) {
+      if (some_where) {
+        step_stack_->PushString(" and");
+      }
+      step_stack_->PushString(" .Self impls Core.CanDestroy");
+      some_where = true;
+    }
+    for (auto rewrite : llvm::reverse(facet_type_info.rewrite_constraints)) {
+      if (some_where) {
+        step_stack_->PushString(" and");
+      }
+      step_stack_->Push(" ", rewrite.lhs_id, " = ", rewrite.rhs_id);
+      some_where = true;
+    }
+    if (!facet_type_info.self_impls_constraints.empty() ||
+        !facet_type_info.self_impls_named_constraints.empty()) {
+      if (some_where) {
+        step_stack_->PushString(" and");
+      }
+      llvm::ListSeparator sep(" & ");
+      for (auto impls :
+           llvm::reverse(facet_type_info.self_impls_named_constraints)) {
+        step_stack_->Push(impls, &sep);
+      }
+      for (auto impls : llvm::reverse(facet_type_info.self_impls_constraints)) {
+        step_stack_->Push(impls, &sep);
+      }
+      step_stack_->PushString(" .Self impls ");
+      some_where = true;
+    }
+    // TODO: Other restrictions from facet_type_info.
+    if (some_where) {
+      step_stack_->PushString(" where");
+    }
+
+    // Output extend interface and named constraint requirements.
+    if (facet_type_info.extend_constraints.empty() &&
+        facet_type_info.extend_named_constraints.empty()) {
+      step_stack_->PushString("type");
+      return;
+    }
+    llvm::ListSeparator sep(" & ");
+    for (auto extend :
+         llvm::reverse(facet_type_info.extend_named_constraints)) {
+      step_stack_->Push(extend, &sep);
+    }
+    for (auto extend : llvm::reverse(facet_type_info.extend_constraints)) {
+      step_stack_->Push(extend, &sep);
+    }
+  }
+
  private:
   const File* sem_ir_;
   StepStack* step_stack_;
@@ -763,6 +771,9 @@ static auto Stringify(const File& sem_ir, StepStack& step_stack)
       case CARBON_KIND(ElementIndex element_index):
         out << element_index.index;
         break;
+      case CARBON_KIND(FacetTypeId facet_type_id):
+        stringifier.StringifyFacetType(facet_type_id);
+        break;
     }
   }
 
@@ -841,4 +852,11 @@ auto StringifySpecificInterface(const File& sem_ir,
   }
 }
 
+auto StringifyFacetType(const File& sem_ir, FacetTypeId facet_type_id)
+    -> std::string {
+  StepStack step_stack(&sem_ir);
+  step_stack.PushFacetType(facet_type_id);
+  return Stringify(sem_ir, step_stack);
+}
+
 }  // namespace Carbon::SemIR

+ 7 - 1
toolchain/sem_ir/stringify.h

@@ -26,7 +26,7 @@ auto StringifySpecific(const File& sem_ir, SpecificId specific_id)
     -> std::string;
 
 // Produces a string version of the name of a specific interface. If the
-// interface is not generic, this is just the name of the interface. Otheewise,
+// interface is not generic, this is just the name of the interface. Otherwise,
 // it is the interface name and its generic arguments.  Generally, this should
 // not be called directly. To format a string into a diagnostic, use a
 // diagnostic parameter of type `SpecificInterface`.
@@ -34,6 +34,12 @@ auto StringifySpecificInterface(const File& sem_ir,
                                 SpecificInterface specific_interface)
     -> std::string;
 
+// Produces a string version of the facet type. This contains the name of the
+// interfaces or named constraints that the facet type names, and any
+// requirements such as rewrites.
+auto StringifyFacetType(const File& sem_ir, FacetTypeId facet_type_id)
+    -> std::string;
+
 }  // namespace Carbon::SemIR
 
 #endif  // CARBON_TOOLCHAIN_SEM_IR_STRINGIFY_H_