Invalid comment cleanup (#4836)

This is a followup from #4834, I searched for "invalid" uses in our
codebase. This is mostly changing comments, and a couple debug
functions, but shouldn't affect testable behavior.

Note a couple things I'll highlight as not changing (but could) are:
- `ReturnTypeInfo::is_valid`
- `"invalid"` uses in the formatter
- `AddInvalid` for `!has_value` in `inst_fingerprinter` (because the
cases it's called sound invalid-ish)

toolchain/base/int.h

@@ -55,8 +55,8 @@ class IntId : public Printable<IntId> {
   static constexpr llvm::StringLiteral Label = "int";
   using ValueType = llvm::APInt;
 
-  // The encoding of integer IDs ensures that valid IDs associated with tokens
-  // during lexing can fit into a compressed storage space. We arrange for
+  // The encoding of integer IDs ensures that IDs associated with tokens during
+  // lexing can fit into a compressed storage space. We arrange for
   // `TokenIdBits` to be the minimum number of bits of storage for token
   // associated IDs. The constant is public so the lexer can ensure it reserves
   // adequate space.

toolchain/check/check.cpp

@@ -25,7 +25,7 @@ using ImportKey = std::pair<llvm::StringRef, llvm::StringRef>;
 
 // Returns a key form of the package object. file_package_id is only used for
 // imports, not the main package declaration; as a consequence, it will be
-// invalid for the main package declaration.
+// `None` for the main package declaration.
 static auto GetImportKey(UnitAndImports& unit_info,
                          IdentifierId file_package_id,
                          Parse::Tree::PackagingNames names) -> ImportKey {
@@ -277,7 +277,7 @@ static auto BuildApiMapAndDiagnosePackaging(
           CARBON_DIAGNOSTIC(DuplicateMainApi, Error,
                             "`Main//default` previously provided by `{0}`",
                             std::string);
-          // Use the invalid node because there's no node to associate with.
+          // Use `NodeId::None` because there's no node to associate with.
           unit_info.emitter.Emit(Parse::NodeId::None, DuplicateMainApi,
                                  prev_filename.str());
         }

toolchain/check/check_unit.cpp

@@ -252,7 +252,7 @@ auto CheckUnit::ImportOtherPackages(SemIR::TypeId namespace_type_id) -> void {
   //
   // For packages imported by the API file, the IdentifierId is the package name
   // and the index is into the API's import list. Otherwise, the initial
-  // {Invalid, -1} state remains.
+  // {None, -1} state remains.
   llvm::SmallVector<std::pair<IdentifierId, int32_t>> api_imports_list;
   api_imports_list.resize(unit_and_imports_->package_imports.size(),
                           {IdentifierId::None, -1});

toolchain/check/check_unit.h

@@ -36,7 +36,7 @@ struct PackageImports {
   // identifier (even if the import failed). Used for associating diagnostics
   // not specific to a single import.
   Parse::ImportDeclId node_id;
-  // The associated `import` instruction. Only valid once a file is checked.
+  // The associated `import` instruction. Has a value after a file is checked.
   SemIR::InstId import_decl_id = SemIR::InstId::None;
   // Whether there's an import that failed to load.
   bool has_load_error = false;

toolchain/check/context.cpp

@@ -685,7 +685,7 @@ auto Context::LookupQualifiedName(SemIR::LocId loc_id, SemIR::NameId name_id,
   return result;
 }
 
-// Returns the scope of the Core package, or Invalid if it's not found.
+// Returns the scope of the Core package, or `None` if it's not found.
 //
 // TODO: Consider tracking the Core package in SemIR so we don't need to use
 // name lookup to find it.

toolchain/check/context.h

@@ -45,8 +45,7 @@ struct LookupResult {
   // The specific in which the lookup result was found. `None` if the result
   // was not found in a specific.
   SemIR::SpecificId specific_id;
-  // The declaration that was found by name lookup.
-  // Invalid for poisoned items.
+  // The declaration that was found by name lookup. `None` for poisoned items.
   // TODO: Make this point to the poisoning declaration.
   SemIR::InstId inst_id;
   // Whether the lookup found a poisoned name.
@@ -78,7 +77,7 @@ class Context {
     // The matching entity if found, or `None` if poisoned or not found.
     SemIR::InstId inst_id;
 
-    // The access level required to use inst_id, if it's valid.
+    // The access level required to use inst_id when it's not `None`.
     SemIR::AccessKind access_kind;
 
     // Whether a poisoned entry was found.
@@ -190,15 +189,15 @@ class Context {
     node_stack_.Push(node_id, AddInst(node_id, inst));
   }
 
-  // Replaces the instruction `inst_id` with `loc_id_and_inst`. The instruction
-  // is required to not have been used in any constant evaluation, either
-  // because it's newly created and entirely unused, or because it's only used
-  // in a position that constant evaluation ignores, such as a return slot.
+  // Replaces the instruction at `inst_id` with `loc_id_and_inst`. The
+  // instruction is required to not have been used in any constant evaluation,
+  // either because it's newly created and entirely unused, or because it's only
+  // used in a position that constant evaluation ignores, such as a return slot.
   auto ReplaceLocIdAndInstBeforeConstantUse(SemIR::InstId inst_id,
                                             SemIR::LocIdAndInst loc_id_and_inst)
       -> void;
 
-  // Replaces the instruction `inst_id` with `inst`, not affecting location.
+  // Replaces the instruction at `inst_id` with `inst`, not affecting location.
   // The instruction is required to not have been used in any constant
   // evaluation, either because it's newly created and entirely unused, or
   // because it's only used in a position that constant evaluation ignores, such
@@ -206,7 +205,7 @@ class Context {
   auto ReplaceInstBeforeConstantUse(SemIR::InstId inst_id, SemIR::Inst inst)
       -> void;
 
-  // Replaces the instruction `inst_id` with `inst`, not affecting location.
+  // Replaces the instruction at `inst_id` with `inst`, not affecting location.
   // The instruction is required to not change its constant value.
   auto ReplaceInstPreservingConstantValue(SemIR::InstId inst_id,
                                           SemIR::Inst inst) -> void;
@@ -230,20 +229,19 @@ class Context {
   // Performs name lookup in a specified scope for a name appearing in a
   // declaration. If scope_id is `None`, performs lookup into the lexical scope
   // specified by scope_index instead. If found, returns the referenced
-  // instruction and false. If poisoned, returns an `None` instruction and
-  // true.
-  // TODO: For poisoned names, return the poisoning instruction.
+  // `InstId` and false. If poisoned, returns `InstId::None` and true.
+  // TODO: For poisoned names, return the poisoning `InstId`.
   auto LookupNameInDecl(SemIR::LocId loc_id, SemIR::NameId name_id,
                         SemIR::NameScopeId scope_id, ScopeIndex scope_index)
       -> std::pair<SemIR::InstId, bool>;
 
-  // Performs an unqualified name lookup, returning the referenced instruction.
+  // Performs an unqualified name lookup, returning the referenced `InstId`.
   auto LookupUnqualifiedName(Parse::NodeId node_id, SemIR::NameId name_id,
                              bool required = true) -> LookupResult;
 
   // Performs a name lookup in a specified scope, returning the referenced
-  // instruction. Does not look into extended scopes. Returns a `None`
-  // instruction if the name is not found.
+  // `InstId`. Does not look into extended scopes. Returns `InstId::None` if the
+  // name is not found.
   //
   // If `is_being_declared` is false, then this is a regular name lookup, and
   // the name will be poisoned if not found so that later lookups will fail; a
@@ -265,14 +263,14 @@ class Context {
       -> bool;
 
   // Performs a qualified name lookup in a specified scopes and in scopes that
-  // they extend, returning the referenced instruction.
+  // they extend, returning the referenced `InstId`.
   auto LookupQualifiedName(SemIR::LocId loc_id, SemIR::NameId name_id,
                            llvm::ArrayRef<LookupScope> lookup_scopes,
                            bool required = true,
                            std::optional<AccessInfo> access_info = std::nullopt)
       -> LookupResult;
 
-  // Returns the instruction corresponding to a name in the core package, or
+  // Returns the `InstId` corresponding to a name in the core package, or
   // BuiltinErrorInst if not found.
   auto LookupNameInCore(SemIRLoc loc, llvm::StringRef name) -> SemIR::InstId;
 

toolchain/check/convert.cpp

@@ -124,9 +124,8 @@ static auto FinalizeTemporary(Context& context, SemIR::InstId init_id,
 
   // The initializer has no return slot, but we want to produce a temporary
   // object. Materialize one now.
-  // TODO: Consider using an invalid ID to mean that we immediately
-  // materialize and initialize a temporary, rather than two separate
-  // instructions.
+  // TODO: Consider using `None` to mean that we immediately materialize and
+  // initialize a temporary, rather than two separate instructions.
   auto init = sem_ir.insts().Get(init_id);
   auto loc_id = sem_ir.insts().GetLocId(init_id);
   auto temporary_id = context.AddInst<SemIR::TemporaryStorage>(
@@ -1022,8 +1021,8 @@ auto Convert(Context& context, SemIR::LocId loc_id, SemIR::InstId expr_id,
   auto& sem_ir = context.sem_ir();
   auto orig_expr_id = expr_id;
 
-  // Start by making sure both sides are valid. If any part is invalid, the
-  // result is invalid and we shouldn't error.
+  // Start by making sure both sides are non-errors. If any part is an error,
+  // the result is an error and we shouldn't diagnose.
   if (sem_ir.insts().Get(expr_id).type_id() ==
           SemIR::ErrorInst::SingletonTypeId ||
       target.type_id == SemIR::ErrorInst::SingletonTypeId) {

toolchain/check/decl_name_stack.cpp

@@ -19,7 +19,7 @@ namespace Carbon::Check {
 auto DeclNameStack::NameContext::prev_inst_id() -> SemIR::InstId {
   switch (state) {
     case NameContext::State::Error:
-      // The name is invalid and a diagnostic has already been emitted.
+      // The name is malformed and a diagnostic has already been emitted.
       return SemIR::InstId::None;
 
     case NameContext::State::Empty:

toolchain/check/decl_name_stack.h

@@ -116,10 +116,10 @@ class DeclNameStack {
       };
     }
 
-    // Returns any name collision found, or Invalid.
+    // Returns any name collision found, or `None`.
     auto prev_inst_id() -> SemIR::InstId;
 
-    // Returns the name_id for a new instruction. This is invalid when the name
+    // Returns the name_id for a new instruction. This is `None` when the name
     // resolved.
     auto name_id_for_new_inst() -> SemIR::NameId {
       return state == State::Unresolved ? unresolved_name_id
@@ -136,7 +136,7 @@ class DeclNameStack {
     bool has_qualifiers = false;
 
     // The scope which qualified names are added to. For unqualified names in
-    // an unnamed scope, this will be Invalid to indicate the current scope
+    // an unnamed scope, this will be `None` to indicate the current scope
     // should be used.
     SemIR::NameScopeId parent_scope_id;
 
@@ -145,7 +145,7 @@ class DeclNameStack {
 
     union {
       // The ID of a resolved qualifier, including both identifiers and
-      // expressions. Invalid indicates resolution failed.
+      // expressions. `None` indicates resolution failed.
       SemIR::InstId resolved_inst_id;
 
       // The ID of an unresolved identifier.
@@ -238,8 +238,8 @@ class DeclNameStack {
                          SemIR::AccessKind access_kind) -> void;
 
   // Adds a name to name lookup if neither already declared nor poisoned in this
-  // scope. If declared, returns the existing instruction and false. If
-  // poisoned, returns an invalid instruction and true.
+  // scope. If declared, returns the existing `InstId` and false. If poisoned,
+  // returns `None` and true.
   // TODO: Return the poisoning instruction if poisoned.
   auto LookupOrAddName(NameContext name_context, SemIR::InstId target_id,
                        SemIR::AccessKind access_kind)
@@ -255,7 +255,7 @@ class DeclNameStack {
                           SemIR::NameId name_id) -> void;
 
   // Attempts to resolve the given name context as a scope, and returns the
-  // corresponding scope. Issues a suitable diagnostic and returns Invalid if
+  // corresponding scope. Issues a suitable diagnostic and returns `None` if
   // the name doesn't resolve to a scope.
   auto ResolveAsScope(const NameContext& name_context,
                       const NameComponent& name) const

toolchain/check/deduce.cpp

@@ -260,7 +260,7 @@ DeductionContext::DeductionContext(Context& context, SemIR::LocId loc_id,
   CARBON_CHECK(generic_id.has_value(),
                "Performing deduction for non-generic entity");
 
-  // Initialize the deduced arguments to Invalid.
+  // Initialize the deduced arguments to `None`.
   result_arg_ids_.resize(
       context.inst_blocks()
           .Get(context.generics().Get(generic_id_).bindings_id)

toolchain/check/dump.cpp

@@ -31,7 +31,7 @@ namespace Carbon::Check {
 
 static auto DumpNoNewline(const Context& context, SemIR::LocId loc_id) -> void {
   if (!loc_id.has_value()) {
-    llvm::errs() << "LocId(invalid)";
+    llvm::errs() << "LocId(<none>)";
     return;
   }
 

toolchain/check/eval.cpp

@@ -788,8 +788,8 @@ static auto DiagnoseDivisionByZero(Context& context, SemIRLoc loc) -> void {
   context.emitter().Emit(loc, CompileTimeDivisionByZero);
 }
 
-// Get an integer at a suitable bit-width: either `bit_width_id` if it is valid,
-// or the canonical width from the value store if not.
+// Get an integer at a suitable bit-width: either `bit_width_id` if it has a
+// value, or the canonical width from the value store if not.
 static auto GetIntAtSuitableWidth(Context& context, IntId int_id,
                                   IntId bit_width_id) -> llvm::APInt {
   return bit_width_id.has_value()

toolchain/check/generic.h

@@ -60,8 +60,8 @@ auto RebuildGenericEvalBlock(Context& context, SemIR::GenericId generic_id,
 auto MakeSpecific(Context& context, SemIRLoc loc, SemIR::GenericId generic_id,
                   SemIR::InstBlockId args_id) -> SemIR::SpecificId;
 
-// Builds a new specific if the given generic is valid. Otherwise returns an
-// invalid specific.
+// Builds a new specific if the given generic is it has a value. Otherwise
+// returns `None`.
 inline auto MakeSpecificIfGeneric(Context& context, SemIRLoc loc,
                                   SemIR::GenericId generic_id,
                                   SemIR::InstBlockId args_id)
@@ -72,8 +72,8 @@ inline auto MakeSpecificIfGeneric(Context& context, SemIRLoc loc,
 }
 
 // Builds the specific that describes how the generic should refer to itself.
-// For example, for a generic `G(T:! type)`, this is the specific `G(T)`. For an
-// invalid `generic_id`, returns an invalid specific ID.
+// For example, for a generic `G(T:! type)`, this is the specific `G(T)`. If
+// `generic_id` is `None`, returns `None`.
 auto MakeSelfSpecific(Context& context, SemIRLoc loc,
                       SemIR::GenericId generic_id) -> SemIR::SpecificId;
 

toolchain/check/handle_binding_pattern.cpp

@@ -262,8 +262,8 @@ static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id,
       }
       if (had_error) {
         context.AddNameToLookup(name_id, SemIR::ErrorInst::SingletonInstId);
-        // Replace the parameter with an invalid instruction so that we don't
-        // try constructing a generic based on it.
+        // Replace the parameter with `ErrorInst` so that we don't try
+        // constructing a generic based on it.
         param_pattern_id = SemIR::ErrorInst::SingletonInstId;
       } else {
         auto pattern_inst_id = make_binding_pattern();
@@ -321,7 +321,7 @@ auto HandleParseNode(Context& context,
   if (context.decl_introducer_state_stack().innermost().kind ==
       Lex::TokenKind::Let) {
     // Disallow `let` outside of function and interface definitions.
-    // TODO: Find a less brittle way of doing this. An invalid scope_inst_id
+    // TODO: Find a less brittle way of doing this. A `scope_inst_id` of `None`
     // can represent a block scope, but is also used for other kinds of scopes
     // that aren't necessarily part of an interface or function decl.
     auto scope_inst_id = context.scope_stack().PeekInstId();

toolchain/check/handle_where.cpp

@@ -32,13 +32,13 @@ auto HandleParseNode(Context& context, Parse::WhereOperandId node_id) -> bool {
   auto entity_name_id = context.entity_names().Add(
       {.name_id = SemIR::NameId::PeriodSelf,
        .parent_scope_id = context.scope_stack().PeekNameScopeId(),
-       // Invalid because this is not the parameter of a generic.
+       // `None` because this is not the parameter of a generic.
        .bind_index = SemIR::CompileTimeBindIndex::None});
   auto inst_id =
       context.AddInst(SemIR::LocIdAndInst::NoLoc<SemIR::BindSymbolicName>(
           {.type_id = self_type_id,
            .entity_name_id = entity_name_id,
-           // Invalid because there is no equivalent non-symbolic value.
+           // `None` because there is no equivalent non-symbolic value.
            .value_id = SemIR::InstId::None}));
   auto existing =
       context.scope_stack().LookupOrAddName(SemIR::NameId::PeriodSelf, inst_id);

toolchain/check/import.h

@@ -39,13 +39,13 @@ auto ImportLibrariesFromOtherPackage(Context& context,
                                      bool has_load_error) -> void;
 
 // Given a name scope that corresponds to another package (having one or more
-// import_irs), looks for the name in imports. Name resolution results are added
-// to the scope, and the InstId (possibly invalid) is returned.
+// `import_irs`), looks for the name in imports. Name resolution results are
+// added to the scope, and the `InstId` (possibly `None`) is returned.
 //
-// In general, this will add an ImportRef and load it; it's never left unloaded
-// because the result is expected to be immediately used. Namespaces will be
-// directly produced, similar to how they function for imports from the current
-// package. Conflicts will be resolved and diagnosed.
+// In general, this will add an `ImportRef` and load it; it's never left
+// unloaded because the result is expected to be immediately used. Namespaces
+// will be directly produced, similar to how they function for imports from the
+// current package. Conflicts will be resolved and diagnosed.
 //
 // Arguments are all in the context of the current IR. Scope lookup is expected
 // to be resolved first.

toolchain/check/import_ref.cpp

@@ -547,12 +547,12 @@ class ImportRefResolver : public ImportContext {
 
   // The constant found by FindResolvedConstId.
   struct ResolvedConstId {
-    // The constant for the instruction. Invalid if not yet resolved.
+    // The constant for the instruction. `None` if not yet resolved.
     SemIR::ConstantId const_id = SemIR::ConstantId::None;
 
     // Instructions which are indirect but equivalent to the current instruction
     // being resolved, and should have their constant set to the same. Empty
-    // when const_id is valid.
+    // when `const_id` has a value.
     llvm::SmallVector<SemIR::ImportIRInst> indirect_insts = {};
   };
 
@@ -1087,9 +1087,8 @@ static auto GetLocalNameScopeId(ImportRefResolver& resolver,
   auto [inst_id, inst] =
       resolver.import_name_scopes().GetInstIfValid(name_scope_id);
   if (!inst) {
-    // Map scopes that aren't associated with an instruction to invalid
-    // scopes. For now, such scopes aren't used, and we don't have a good way
-    // to remap them.
+    // Map scopes that aren't associated with an instruction to `None`. For now,
+    // such scopes aren't used, and we don't have a good way to remap them.
     return SemIR::NameScopeId::None;
   }
 
@@ -2593,7 +2592,7 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver,
 }
 
 // Tries to resolve the InstId, returning a canonical constant when ready, or
-// Invalid if more has been added to the stack. This is the same as
+// `None` if more has been added to the stack. This is the same as
 // TryResolveInst, except that it may resolve symbolic constants as canonical
 // constants instead of as constants associated with a particular generic.
 //
@@ -2763,11 +2762,11 @@ static auto TryResolveInstCanonical(ImportRefResolver& resolver,
   }
 }
 
-// Tries to resolve the InstId, returning a constant when ready, or Invalid if
+// Tries to resolve the InstId, returning a constant when ready, or `None` if
 // more has been added to the stack. A similar API is followed for all
 // following TryResolveTypedInst helper functions.
 //
-// `const_id` is Invalid unless we've tried to resolve this instruction
+// `const_id` is `None` unless we've tried to resolve this instruction
 // before, in which case it's the previous result.
 //
 // TODO: Error is returned when support is missing, but that should go away.

toolchain/check/import_ref.h

@@ -25,7 +25,7 @@ auto AddImportRef(Context& context, SemIR::ImportIRInst import_ir_inst,
                   SemIR::EntityNameId entity_name_id) -> SemIR::InstId;
 
 // Returns the canonical IR inst for an entity. Returns an `ImportIRInst` with
-// an invalid ir_id for an entity that was not imported.
+// a `None` ir_id for an entity that was not imported.
 auto GetCanonicalImportIRInst(Context& context, SemIR::InstId inst_id)
     -> SemIR::ImportIRInst;
 

toolchain/check/inst_block_stack.h

@@ -31,7 +31,7 @@ class InstBlockStack {
   auto Push(SemIR::InstBlockId id, llvm::ArrayRef<SemIR::InstId> inst_ids)
       -> void;
 
-  // Pushes a new instruction block. It will be invalid unless PeekOrAdd is
+  // Pushes a new instruction block. It will be `None` unless PeekOrAdd is
   // called in order to support lazy allocation.
   auto Push() -> void { Push(SemIR::InstBlockId::None); }
 
@@ -44,8 +44,8 @@ class InstBlockStack {
   // 0.
   auto PeekOrAdd(int depth = 0) -> SemIR::InstBlockId;
 
-  // Pops the top instruction block. This will always return a valid instruction
-  // block; SemIR::InstBlockId::Empty is returned if one wasn't allocated.
+  // Pops the top instruction block. This will never return `None`; `Empty` is
+  // returned if one wasn't allocated.
   auto Pop() -> SemIR::InstBlockId;
 
   // Pops the top instruction block, and discards it if it hasn't had an ID
@@ -89,8 +89,8 @@ class InstBlockStack {
   // Whether to print verbose output.
   llvm::raw_ostream* vlog_stream_;
 
-  // The stack of block IDs. Valid if allocated, Invalid if no block has been
-  // allocated, or Unreachable if this block is known to be unreachable.
+  // The stack of block IDs. A value if allocated, `None` if no block has been
+  // allocated, or `Unreachable` if this block is known to be unreachable.
   llvm::SmallVector<SemIR::InstBlockId> id_stack_;
 
   // The stack of insts in each block.

toolchain/check/member_access.cpp

@@ -219,8 +219,8 @@ static auto PerformImplLookup(
 }
 
 // Performs a member name lookup into the specified scope, including performing
-// impl lookup if necessary. If the scope is invalid, assume an error has
-// already been diagnosed, and return BuiltinErrorInst.
+// impl lookup if necessary. If the scope result is `None`, assume an error has
+// already been diagnosed, and return `ErrorInst`.
 static auto LookupMemberNameInScope(Context& context, SemIR::LocId loc_id,
                                     SemIR::InstId base_id,
                                     SemIR::NameId name_id,

toolchain/check/merge.cpp

@@ -379,9 +379,9 @@ static auto CheckRedeclParamSyntax(Context& context,
                                    Parse::NodeId prev_last_param_node_id,
                                    bool diagnose) -> bool {
   // Parse nodes may not always be available to compare.
-  // TODO: Support cross-file syntax checks. Right now imports provide invalid
-  // nodes, and we'll need to follow the declaration to its original file to
-  // get the parse tree.
+  // TODO: Support cross-file syntax checks. Right now imports provide
+  // `NodeId::None`, and we'll need to follow the declaration to its original
+  // file to get the parse tree.
   if (!new_first_param_node_id.has_value() ||
       !prev_first_param_node_id.has_value()) {
     return true;

toolchain/check/merge.h

@@ -31,7 +31,7 @@ struct RedeclInfo {
   bool is_definition;
   // True if an `extern` declaration.
   bool is_extern;
-  // The library name in `extern library`, or invalid if not present.
+  // The library name in `extern library`, or `None` if not present.
   SemIR::LibraryNameId extern_library_id;
 };
 
@@ -85,10 +85,10 @@ struct DeclParams {
   Parse::NodeId first_param_node_id;
   Parse::NodeId last_param_node_id;
 
-  // The implicit parameters of the entity. Can be Invalid if there is no
+  // The implicit parameters of the entity. Can be `None` if there is no
   // implicit parameter list.
   SemIR::InstBlockId implicit_param_patterns_id;
-  // The explicit parameters of the entity. Can be Invalid if there is no
+  // The explicit parameters of the entity. Can be `None` if there is no
   // explicit parameter list.
   SemIR::InstBlockId param_patterns_id;
 };

toolchain/check/node_stack.h

@@ -19,7 +19,7 @@ namespace Carbon::Check {
 // A non-discriminated union of ID types.
 class IdUnion {
  public:
-  // The default constructor forms an invalid ID.
+  // The default constructor forms a `None` ID.
   explicit constexpr IdUnion() : index(AnyIdBase::NoneIndex) {}
 
   template <typename IdT>

toolchain/check/pattern_match.cpp

@@ -56,12 +56,12 @@ class MatchContext {
  public:
   struct WorkItem {
     SemIR::InstId pattern_id;
-    // Invalid when processing the callee side.
+    // `None` when processing the callee side.
     SemIR::InstId scrutinee_id;
   };
 
-  // Constructs a MatchContext. If `callee_specific_id` is valid, this pattern
-  // match operation is part of implementing the signature of the given
+  // Constructs a MatchContext. If `callee_specific_id` is not `None`, this
+  // pattern match operation is part of implementing the signature of the given
   // specific.
   explicit MatchContext(MatchKind kind, SemIR::SpecificId callee_specific_id =
                                             SemIR::SpecificId::None)

toolchain/check/pattern_match.h

@@ -20,7 +20,7 @@ namespace Carbon::Check {
 
 // Emits the pattern-match IR for the declaration of a parameterized entity with
 // the given implicit and explicit parameter patterns, and the given return slot
-// pattern (any of which may be invalid if not applicable). This IR performs the
+// pattern (any of which may be `None` if not applicable). This IR performs the
 // callee side of pattern matching, starting at the `ParamPattern` insts, and
 // matching them against the corresponding `Call` parameters (see
 // entity_with_params_base.h for the definition of that term).

toolchain/check/scope_stack.h

@@ -84,7 +84,7 @@ class ScopeStack {
   // Returns the name scope associated with the current lexical scope, if any.
   auto PeekNameScopeId() const -> SemIR::NameScopeId { return Peek().scope_id; }
 
-  // Returns the instruction associated with the current scope, or Invalid if
+  // Returns the instruction associated with the current scope, or `None` if
   // there is no such instruction, such as for a block scope.
   auto PeekInstId() const -> SemIR::InstId { return Peek().scope_inst_id; }
 
@@ -108,7 +108,7 @@ class ScopeStack {
   }
 
   // If there is no `returned var` in scope, sets the given instruction to be
-  // the current `returned var` and returns an invalid instruction ID. If there
+  // the current `returned var` and returns an `None`. If there
   // is already a `returned var`, returns it instead.
   auto SetReturnedVarOrGetExisting(SemIR::InstId inst_id) -> SemIR::InstId;
 
@@ -127,7 +127,7 @@ class ScopeStack {
   // Looks up the name `name_id` in the current scope, or in `scope_index` if
   // specified. Returns the existing instruction if the name is already declared
   // in that scope or any unfinished scope within it, and otherwise adds the
-  // name with the value `target_id` and returns Invalid.
+  // name with the value `target_id` and returns `None`.
   auto LookupOrAddName(SemIR::NameId name_id, SemIR::InstId target_id,
                        ScopeIndex scope_index = ScopeIndex::None)
       -> SemIR::InstId;

toolchain/check/sem_ir_diagnostic_converter.cpp

@@ -45,7 +45,7 @@ auto SemIRDiagnosticConverter::ConvertLocImpl(SemIRLoc loc,
     auto import_ir_inst = cursor_ir->import_ir_insts().Get(import_ir_inst_id);
     const auto& import_ir = cursor_ir->import_irs().Get(import_ir_inst.ir_id);
     CARBON_CHECK(import_ir.decl_id.has_value(),
-                 "If we get invalid locations here, we may need to more "
+                 "If we get `None` locations here, we may need to more "
                  "thoroughly track ImportDecls.");
 
     ConvertedDiagnosticLoc in_import_loc;
@@ -115,7 +115,7 @@ auto SemIRDiagnosticConverter::ConvertLocImpl(SemIRLoc loc,
         continue;
       }
 
-      // If the parse node is valid, use it for the location.
+      // If the parse node has a value, use it for the location.
       if (auto loc_id = cursor_ir->insts().GetLocId(cursor_inst_id);
           loc_id.has_value()) {
         if (auto diag_loc = handle_loc(loc_id)) {
@@ -134,7 +134,7 @@ auto SemIRDiagnosticConverter::ConvertLocImpl(SemIRLoc loc,
       }
     }
 
-    // Invalid parse node but not an import; just nothing to point at.
+    // `None` parse node but not an import; just nothing to point at.
     return ConvertLocInFile(cursor_ir, Parse::NodeId::None, loc.token_only,
                             context_fn);
   }

toolchain/lex/dump.cpp

@@ -12,7 +12,7 @@ namespace Carbon::Lex {
 
 auto DumpNoNewline(const TokenizedBuffer& tokens, TokenIndex token) -> void {
   if (!token.has_value()) {
-    llvm::errs() << "TokenIndex(invalid)";
+    llvm::errs() << "TokenIndex(<none>)";
     return;
   }
 

toolchain/lower/file_context.h

@@ -53,8 +53,7 @@ class FileContext {
 
   // Returns a lowered type for the given type_id.
   auto GetType(SemIR::TypeId type_id) -> llvm::Type* {
-    // InvalidType should not be passed in.
-    CARBON_CHECK(type_id.index >= 0, "{0}", type_id);
+    CARBON_CHECK(type_id.has_value(), "Should not be called with `None`");
     CARBON_CHECK(types_[type_id.index], "Missing type {0}: {1}", type_id,
                  sem_ir().types().GetAsInst(type_id));
     return types_[type_id.index];

toolchain/parse/node_ids.h

@@ -22,7 +22,7 @@ struct NoneNodeId {};
 struct NodeId : public IdBase<NodeId> {
   static constexpr llvm::StringLiteral Label = "node";
 
-  // An explicitly invalid node ID.
+  // A node ID with no value.
   static constexpr NoneNodeId None;
 
   using IdBase::IdBase;

toolchain/sem_ir/formatter.cpp

@@ -689,7 +689,7 @@ class FormatterImpl {
   auto FormatInst(InstId inst_id) -> void {
     if (!inst_id.has_value()) {
       Indent();
-      out_ << "invalid\n";
+      out_ << "none\n";
       return;
     }
 
@@ -843,7 +843,7 @@ class FormatterImpl {
 
   auto FormatInstRHS(BindSymbolicName inst) -> void {
     // A BindSymbolicName with no value is a purely symbolic binding, such as
-    // the `Self` in an interface. Don't print out `invalid` for the value.
+    // the `Self` in an interface. Don't print out `none` for the value.
     if (inst.value_id.has_value()) {
       FormatArgs(inst.entity_name_id, inst.value_id);
     } else {
@@ -1313,7 +1313,7 @@ class FormatterImpl {
   // Returns the label for the indicated IR.
   auto GetImportIRLabel(ImportIRId id) -> std::string {
     CARBON_CHECK(id.has_value(),
-                 "GetImportIRLabel should only be called where we a valid ID.");
+                 "Callers are responsible for checking `id.has_value`");
     const auto& import_ir = *sem_ir_->import_irs().Get(id).sem_ir;
     CARBON_CHECK(import_ir.library_id().has_value());
 

toolchain/sem_ir/function.h

@@ -81,7 +81,7 @@ struct Function : public EntityWithParamsBase,
                                                InstId param_pattern_id)
       -> ParamPatternInfo;
 
-  // Gets the name from the name binding instruction, or invalid if this pattern
+  // Gets the name from the name binding instruction, or `None` if this pattern
   // has been replaced with BuiltinErrorInst.
   static auto GetNameFromPatternId(const File& sem_ir, InstId param_pattern_id)
       -> SemIR::NameId;
@@ -98,13 +98,13 @@ struct Function : public EntityWithParamsBase,
 class File;
 
 struct CalleeFunction {
-  // The function. Invalid if not a function.
+  // The function. `None` if not a function.
   SemIR::FunctionId function_id;
   // The specific that contains the function.
   SemIR::SpecificId enclosing_specific_id;
   // The specific for the callee itself, in a resolved call.
   SemIR::SpecificId resolved_specific_id;
-  // The bound `self` parameter. Invalid if not a method.
+  // The bound `self` parameter. `None` if not a method.
   SemIR::InstId self_id;
   // True if an error instruction was found.
   bool is_error;

toolchain/sem_ir/generic.h

@@ -55,8 +55,7 @@ struct Generic : public Printable<Generic> {
 // Provides storage for generics.
 class GenericStore : public ValueStore<GenericId> {
  public:
-  // Get the self specific for a generic, or an invalid specific for an invalid
-  // generic ID.
+  // Get the self specific for a generic, or `None` if the `id` is `None`.
   auto GetSelfSpecific(GenericId id) -> SpecificId {
     return id.has_value() ? Get(id).self_specific_id : SpecificId::None;
   }

toolchain/sem_ir/impl.h

@@ -65,7 +65,7 @@ class ImplStore {
    public:
     static constexpr llvm::StringLiteral Label = "impl_or_lookup_bucket";
 
-    // An explicitly invalid ID, corresponding to to ImplId::None.
+    // An ID with no value, corresponding to to ImplId::None.
     static const ImplOrLookupBucketId None;
 
     static auto ForImplId(ImplId impl_id) -> ImplOrLookupBucketId {
@@ -77,7 +77,7 @@ class ImplStore {
     }
 
     // Returns whether this ID represents a bucket index, rather than an ImplId.
-    // An invalid ID is not a bucket index.
+    // `None` is not a bucket index.
     auto is_bucket() const { return index < ImplId::NoneIndex; }
 
     // Returns the bucket index represented by this ID. Requires is_bucket().
@@ -103,8 +103,8 @@ class ImplStore {
   // The bucket is held indirectly as an `ImplOrLookupBucketId`, in one of three
   // states:
   //
-  //   - An invalid `ImplId` represents an empty bucket.
-  //   - A valid `ImplId` represents a bucket with exactly one impl. This is
+  //   - `ImplId::None` represents an empty bucket.
+  //   - An `ImplId` value represents a bucket with exactly one impl. This is
   //     expected to be by far the most common case.
   //   - A lookup bucket index represents an index within the `ImplStore`'s
   //     array of variable-sized lookup buckets.

toolchain/sem_ir/inst_fingerprinter.cpp

@@ -35,7 +35,7 @@ struct Worklist {
   auto Finish() -> uint64_t { return llvm::stable_hash_combine(contents); }
 
   // Add an invalid marker to the contents. This is used when the entity
-  // contains an invalid ID. This uses an arbitrary fixed value that is assumed
+  // contains a `None` ID. This uses an arbitrary fixed value that is assumed
   // to be unlikely to collide with a valid value.
   auto AddInvalid() -> void { contents.push_back(-1); }
 

toolchain/sem_ir/inst_namer.cpp

@@ -111,7 +111,7 @@ InstNamer::InstNamer(const File* sem_ir) : sem_ir_(sem_ir) {
 auto InstNamer::GetScopeName(ScopeId scope) const -> std::string {
   switch (scope) {
     case ScopeId::None:
-      return "<invalid scope>";
+      return "<no scope>";
 
     // These are treated as SemIR keywords.
     case ScopeId::File:

toolchain/sem_ir/name.h

@@ -46,8 +46,8 @@ class NameStoreWrapper {
 
   // Returns a best-effort name to use as the basis for SemIR and LLVM IR names.
   // This is always identifier-shaped, but may be ambiguous, for example if
-  // there is both a `self` and an `r#self` in the same scope. Returns "" for an
-  // invalid name.
+  // there is both a `self` and an `r#self` in the same scope. Returns "" if
+  // `name_id` is `None`.
   auto GetIRBaseName(NameId name_id) const -> llvm::StringRef;
 
  private:

toolchain/sem_ir/name_scope.h

@@ -149,7 +149,7 @@ class NameScope : public Printable<NameScope> {
   // The instruction which owns the scope.
   InstId inst_id_;
 
-  // When the scope is a namespace, the name. Otherwise, invalid.
+  // When the scope is a namespace, the name. Otherwise, `None`.
   NameId name_id_;
 
   // The parent scope.
@@ -198,8 +198,8 @@ class NameScopeStore {
     return values_.Get(scope_id);
   }
 
-  // Returns the instruction owning the requested name scope, or Invalid with
-  // nullopt if the scope is either invalid or has no associated instruction.
+  // Returns the instruction owning the requested name scope, or `None` with
+  // nullopt if the scope is either `None` or has no associated instruction.
   auto GetInstIfValid(NameScopeId scope_id) const
       -> std::pair<InstId, std::optional<Inst>>;
 

toolchain/sem_ir/type.h

@@ -27,7 +27,7 @@ class TypeStore : public Yaml::Printable<TypeStore> {
   // Returns the ID of the constant used to define the specified type.
   auto GetConstantId(TypeId type_id) const -> ConstantId {
     if (!type_id.has_value()) {
-      // TODO: Investigate replacing this with a CHECK or returning Invalid.
+      // TODO: Investigate replacing this with a CHECK or returning `None`.
       return ConstantId::NotConstant;
     }
     return type_id.AsConstantId();
@@ -88,8 +88,8 @@ class TypeStore : public Yaml::Printable<TypeStore> {
   }
 
   // Get the object representation associated with a type. For a non-class type,
-  // this is the type itself. An invalid TypeId is returned if the object
-  // representation cannot be determined because the type is not complete.
+  // this is the type itself. `None` is returned if the object representation
+  // cannot be determined because the type is not complete.
   auto GetObjectRepr(TypeId type_id) const -> TypeId;
 
   // Determines whether the given type is known to be complete. This does not

toolchain/sem_ir/type_info.h

@@ -142,7 +142,7 @@ struct ReturnTypeInfo {
     return init_repr.kind == InitRepr::InPlace;
   }
 
-  // The declared return type. Invalid if no return type was specified.
+  // The declared return type. `None` if no return type was specified.
   TypeId type_id;
   // The initializing representation for the return type.
   InitRepr init_repr;

toolchain/sem_ir/typed_insts.h

@@ -999,7 +999,8 @@ struct AnyParam {
   RuntimeParamIndex runtime_index;
 
   // A name to associate with this Param in pretty-printed IR. This is not
-  // necessarily unique, or even valid, and has no semantic significance.
+  // necessarily unique, and can even be `None`; it has no semantic
+  // significance.
   NameId pretty_name_id;
 };
 
@@ -1111,7 +1112,7 @@ struct ReturnExpr {
 
   // This is a statement, so has no type.
   InstId expr_id;
-  // The return slot, if any. Invalid if we're not returning through memory.
+  // The return slot, if any. `None` if we're not returning through memory.
   InstId dest_id;
 };
 
@@ -1492,7 +1493,8 @@ struct VarStorage {
   TypeId type_id;
 
   // A name to associate with this var in pretty-printed IR. This is not
-  // necessarily unique, or even valid, and has no semantic significance.
+  // necessarily unique, and can even be `None`; it has no semantic
+  // significance.
   NameId pretty_name_id;
 };