Change name poisoning implementation to allow better diagnostics (#4764)

Change the implementation to use an explicit `is_poisoned` bit instead
of `InstId::PoisonedName` value.
Zero behavior change.
This would allow to more easily change the API to support accessing the
poisoning declaration so we can have better name poisoning diagnosis.
#4622
This commit is contained in:
Boaz Brickner
2025-01-08 08:36:14 +00:00
committed by GitHub
parent 246ec785df
commit 74395ce693
18 changed files with 127 additions and 111 deletions
+17 -21
View File
@@ -34,7 +34,7 @@ auto DeclNameStack::NameContext::prev_inst_id() -> SemIR::InstId {
return SemIR::InstId::Invalid;
case NameContext::State::Poisoned:
return SemIR::InstId::PoisonedName;
CARBON_FATAL("Poisoned state should not call prev_inst_id()");
case NameContext::State::Finished:
CARBON_FATAL("Finished state should only be used internally");
@@ -173,12 +173,10 @@ auto DeclNameStack::AddName(NameContext name_context, SemIR::InstId target_id,
auto DeclNameStack::AddNameOrDiagnose(NameContext name_context,
SemIR::InstId target_id,
SemIR::AccessKind access_kind) -> void {
if (auto id = name_context.prev_inst_id(); id.is_valid()) {
if (id.is_poisoned()) {
context_->DiagnosePoisonedName(target_id);
} else {
context_->DiagnoseDuplicateName(target_id, id);
}
if (name_context.state == DeclNameStack::NameContext::State::Poisoned) {
context_->DiagnosePoisonedName(target_id);
} else if (auto id = name_context.prev_inst_id(); id.is_valid()) {
context_->DiagnoseDuplicateName(target_id, id);
} else {
AddName(name_context, target_id, access_kind);
}
@@ -187,12 +185,15 @@ auto DeclNameStack::AddNameOrDiagnose(NameContext name_context,
auto DeclNameStack::LookupOrAddName(NameContext name_context,
SemIR::InstId target_id,
SemIR::AccessKind access_kind)
-> SemIR::InstId {
-> std::pair<SemIR::InstId, bool> {
if (name_context.state == NameContext::State::Poisoned) {
return {SemIR::InstId::Invalid, true};
}
if (auto id = name_context.prev_inst_id(); id.is_valid()) {
return id;
return {id, false};
}
AddName(name_context, target_id, access_kind);
return SemIR::InstId::Invalid;
return {SemIR::InstId::Invalid, false};
}
// Push a scope corresponding to a name qualifier. For example, for
@@ -260,15 +261,15 @@ auto DeclNameStack::ApplyAndLookupName(NameContext& name_context,
}
// For identifier nodes, we need to perform a lookup on the identifier.
auto resolved_inst_id = context_->LookupNameInDecl(
auto [resolved_inst_id, is_poisoned] = context_->LookupNameInDecl(
name_context.loc_id, name_id, name_context.parent_scope_id);
if (!resolved_inst_id.is_valid()) {
if (is_poisoned) {
name_context.unresolved_name_id = name_id;
name_context.state = NameContext::State::Poisoned;
} else if (!resolved_inst_id.is_valid()) {
// Invalid indicates an unresolved name. Store it and return.
name_context.unresolved_name_id = name_id;
name_context.state = NameContext::State::Unresolved;
} else if (resolved_inst_id.is_poisoned()) {
name_context.unresolved_name_id = name_id;
name_context.state = NameContext::State::Poisoned;
} else {
// Store the resolved instruction and continue for the target scope
// update.
@@ -286,11 +287,6 @@ static auto CheckQualifierIsResolved(
CARBON_FATAL("No qualifier to resolve");
case DeclNameStack::NameContext::State::Resolved:
if (name_context.resolved_inst_id.is_poisoned()) {
context.DiagnoseNameNotFound(name_context.loc_id,
name_context.unresolved_name_id);
return false;
}
return true;
case DeclNameStack::NameContext::State::Poisoned:
@@ -382,7 +378,7 @@ auto DeclNameStack::ResolveAsScope(const NameContext& name_context,
return InvalidResult;
}
if (name_context.resolved_inst_id.is_poisoned()) {
if (name_context.state == NameContext::State::Poisoned) {
return InvalidResult;
}