Repository: carbon-language/carbon-lang
File: toolchain/sem_ir/typed_insts.h (C/C++)

Code before:
       .deduce_through = true});

  TypeId type_id;
  StructTypeFieldsId fields_id;
  CustomLayoutId layout_id;
};

// The `*` dereference operator, as in `*pointer`.
struct Deref {
  static constexpr auto Kind =
      InstKind::Deref.Define<Parse::NodeId>({.ir_name = "deref"});

  TypeId type_id;
  InstId pointer_id;
};

// Used when a semantic error has been detected, and a SemIR InstId is still
// required. For example, when there is a type checking issue, this will be used
// in the type_id. It's typically used as a cue that semantic checking doesn't
// need to issue further diagnostics.
struct ErrorInst : public SingletonTypeInst<InstKind::ErrorInst, "<error>"> {
  // Convenience for returning error constants directly.
  static constexpr auto ConstantId =
      ConstantId::ForConcreteConstant(TypeInstId);

  // `ErrorInst` is always set complete in file.cpp.
  static constexpr auto TypeId = TypeId::ForTypeConstant(ConstantId);
};

// An `export bind_name` declaration.
struct ExportDecl {
  static constexpr auto Kind =
      InstKind::ExportDecl.Define<Parse::ExportDeclId>({.ir_name = "export"});

  TypeId type_id;
  EntityNameId entity_name_id;
  // The exported entity.
  InstId value_id;
};

// Represents accessing the `type` field in a facet value, which is notionally a
// pair of a type and a witness.
struct FacetAccessType {
  static constexpr auto Kind = InstKind::FacetAccessType.Define<Parse::NodeId>(
      {.ir_name = "facet_access_type",
       .is_type = InstIsType::Always,
       .constant_kind = InstConstantKind::SymbolicOnly});

  // Always the builtin type TypeType.
  TypeId type_id;
  // An instruction that evaluates to a `FacetValue`.

Review comment (suggestion):
For the `*Type` singletons, the change to use `::TypeInstId` seems like an improvement. But for `ErrorInst` the change seems less straightforwardly like an improvement. Could we keep `InstId` for `ErrorInst` and leave the users of it alone?

While it's a singleton, ErrorInst is another oddball that isn't _really_ a type, but _can be_ a type.

Code after:
       .deduce_through = true});

  TypeId type_id;
  StructTypeFieldsId fields_id;
  CustomLayoutId layout_id;
};

// The `*` dereference operator, as in `*pointer`.
struct Deref {
  static constexpr auto Kind =
      InstKind::Deref.Define<Parse::NodeId>({.ir_name = "deref"});

  TypeId type_id;
  InstId pointer_id;
};

// Used when a semantic error has been detected, and a SemIR InstId is still
// required. For example, when there is a type checking issue, this will be used
// in the type_id. It's typically used as a cue that semantic checking doesn't
// need to issue further diagnostics.
struct ErrorInst : public SingletonTypeInst<InstKind::ErrorInst, "<error>"> {
  // Convenience for returning error InstIds and ConstantIds directly.
  static constexpr InstId InstId = TypeInstId;
  static constexpr auto ConstantId =
      ConstantId::ForConcreteConstant(TypeInstId);

  // `ErrorInst` is always set complete in file.cpp.
  static constexpr auto TypeId = TypeId::ForTypeConstant(ConstantId);
};

// An `export bind_name` declaration.
struct ExportDecl {
  static constexpr auto Kind =
      InstKind::ExportDecl.Define<Parse::ExportDeclId>({.ir_name = "export"});

  TypeId type_id;
  EntityNameId entity_name_id;
  // The exported entity.
  InstId value_id;
};

// Represents accessing the `type` field in a facet value, which is notionally a
// pair of a type and a witness.
struct FacetAccessType {
  static constexpr auto Kind = InstKind::FacetAccessType.Define<Parse::NodeId>(
      {.ir_name = "facet_access_type",
       .is_type = InstIsType::Always,
       .constant_kind = InstConstantKind::SymbolicOnly});

  // Always the builtin type TypeType.
  TypeId type_id;