Id::Invalid -> Id::None (#4834)

High level, replacing `Id::Invalid` with `Id::None` and `Id::is_valid`
with `Id::has_value` for clarity, as discussed
[here](https://discord.com/channels/655572317891461132/655578254970716160/1331664574545395794).
The `IntId` refactoring is needed together with `AnyIdBase` because it's
also used with `ValueStore`.

Note, trying to be careful not to rewrite `EnumBase::InvalidIndex`, or
`is_valid` in general (e.g., `IdKind::is_valid`).

I've tried to sequence commits here:

1. Automatic replacements:

- `((?:Id|Index)(?: |::|\(|Base(?:\(|::)))Invalid((?:Index)?\W)` ->
`$1None$2`
  - `<invalid>` -> `<none>`
  - `InvalidNodeId` -> `NoneNodeId`
  - `/\*invalid\*/` -> `/*none*/`
  - `id((?:_|\(\))(?:\.|->))is_valid` -> `id$1has_value`

2. Manual edits:

  - In `int.h` and `int_test.cpp`
    - `IntT` has `is_value`, which I'm renaming to `is_embedded_value`.
    - Manual edits to comments in this file.
  - `AnyIdBase` and `IdBase`
- Declaration of `is_valid` -> `has_value`, `InvalidIndex` ->
`NoneIndex`.
  - In `ids.h` and `ids.cpp`
    - `is_valid` -> `has_value`
- `// An explicitly invalid ID.` -> `// An ID with no value.`; similar
for index
    - Various math on `InvalidIndex` -> `NoneIndex`
    - Various mentions of "valid" in comments
  - In `value_store.h`, for `IdT::Invalid`, plus one comment
- In `impl.h` and `tokenized_buffer.h`, we had different initialization
of `::None` values (versus `ids.h` syntax) that I fixed manually.
  - Spot checks to compile
- Particularly where `is_valid` replacements didn't catch spots due to
different naming.

3. Autoupdate tests

4. verbose.carbon (NOAUTOUPDATE)

5. Comment spot checks

Note there are probably other mentions of "Invalid" that should be swept
up, but I'd like to argue for merging and separating out remaining
cleanup since this is so sweeping (and likely to hit merge conflicts
from churn). We'll probably have lingering mentions of "invalid" for a
bit regardless, just because there are uses of "invalid" in non-Id APIs.
This commit is contained in:
Jon Ross-Perkins
2025-01-22 23:15:00 +00:00
committed by GitHub
parent b292943648
commit 6b5eb1a101
361 changed files with 2612 additions and 2624 deletions
+15 -16
View File
@@ -36,8 +36,8 @@ static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id,
bool is_generic = node_kind == Parse::NodeKind::CompileTimeBindingPattern;
if (is_generic) {
auto inst_id = context.scope_stack().PeekInstId();
is_associated_constant =
inst_id.is_valid() && context.insts().Is<SemIR::InterfaceDecl>(inst_id);
is_associated_constant = inst_id.has_value() &&
context.insts().Is<SemIR::InterfaceDecl>(inst_id);
}
bool needs_compile_time_binding = is_generic && !is_associated_constant;
@@ -46,8 +46,8 @@ static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id,
context.decl_introducer_state_stack().innermost();
auto make_binding_pattern = [&]() -> SemIR::InstId {
auto bind_id = SemIR::InstId::Invalid;
auto binding_pattern_id = SemIR::InstId::Invalid;
auto bind_id = SemIR::InstId::None;
auto binding_pattern_id = SemIR::InstId::None;
// TODO: Eventually the name will need to support associations with other
// scopes, but right now we don't support qualified names here.
auto entity_name_id = context.entity_names().Add(
@@ -57,14 +57,13 @@ static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id,
// constant declaration.
.bind_index = needs_compile_time_binding
? context.scope_stack().AddCompileTimeBinding()
: SemIR::CompileTimeBindIndex::Invalid});
: SemIR::CompileTimeBindIndex::None});
if (is_generic) {
// TODO: Create a `BindTemplateName` instead inside a `template` pattern.
bind_id = context.AddInstInNoBlock(SemIR::LocIdAndInst(
name_node,
SemIR::BindSymbolicName{.type_id = cast_type_id,
.entity_name_id = entity_name_id,
.value_id = SemIR::InstId::Invalid}));
name_node, SemIR::BindSymbolicName{.type_id = cast_type_id,
.entity_name_id = entity_name_id,
.value_id = SemIR::InstId::None}));
binding_pattern_id =
context.AddPatternInst<SemIR::SymbolicBindingPattern>(
name_node,
@@ -73,7 +72,7 @@ static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id,
bind_id = context.AddInstInNoBlock(SemIR::LocIdAndInst(
name_node, SemIR::BindName{.type_id = cast_type_id,
.entity_name_id = entity_name_id,
.value_id = SemIR::InstId::Invalid}));
.value_id = SemIR::InstId::None}));
binding_pattern_id = context.AddPatternInst<SemIR::BindingPattern>(
name_node,
{.type_id = cast_type_id, .entity_name_id = entity_name_id});
@@ -133,7 +132,7 @@ static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id,
});
auto binding_id =
is_generic
? Parse::NodeId::Invalid
? Parse::NodeId::None
: context.parse_tree().As<Parse::VarBindingPatternId>(node_id);
auto& class_info = context.classes().Get(parent_class_decl->class_id);
auto field_type_id =
@@ -141,7 +140,7 @@ static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id,
auto field_id = context.AddInst<SemIR::FieldDecl>(
binding_id, {.type_id = field_type_id,
.name_id = name_id,
.index = SemIR::ElementIndex::Invalid});
.index = SemIR::ElementIndex::None});
context.field_decls_stack().AppendToTop(field_id);
context.node_stack().Push(node_id, field_id);
@@ -167,7 +166,7 @@ static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id,
context.entity_names().Add(
{.name_id = name_id,
.parent_scope_id = context.scope_stack().PeekNameScopeId(),
.bind_index = SemIR::CompileTimeBindIndex::Invalid});
.bind_index = SemIR::CompileTimeBindIndex::None});
SemIR::InstId decl_id = context.AddInst<SemIR::AssociatedConstantDecl>(
context.parse_tree().As<Parse::CompileTimeBindingPatternId>(node_id),
@@ -228,7 +227,7 @@ static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id,
// in a function definition. We don't know which kind we have here.
// TODO: A tuple pattern can appear in other places than function
// parameters.
auto param_pattern_id = SemIR::InstId::Invalid;
auto param_pattern_id = SemIR::InstId::None;
bool had_error = false;
switch (introducer.kind) {
case Lex::TokenKind::Fn: {
@@ -273,7 +272,7 @@ static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id,
{
.type_id = context.insts().Get(pattern_inst_id).type_id(),
.subpattern_id = pattern_inst_id,
.runtime_index = is_generic ? SemIR::RuntimeParamIndex::Invalid
.runtime_index = is_generic ? SemIR::RuntimeParamIndex::None
: SemIR::RuntimeParamIndex::Unknown,
});
}
@@ -326,7 +325,7 @@ auto HandleParseNode(Context& context,
// 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();
if (scope_inst_id.is_valid()) {
if (scope_inst_id.has_value()) {
auto scope_inst = context.insts().Get(scope_inst_id);
if (!scope_inst.Is<SemIR::InterfaceDecl>() &&
!scope_inst.Is<SemIR::FunctionDecl>()) {