carbon-language/carbon-lang
1
0
Fork 0
mirror of https://github.com/carbon-language/carbon-lang.git synced 2026-10-05 22:02:55 +01:00
Code Issues Packages Projects Releases 10 Wiki Activity

Drop Singleton from ErrorInst::SingletonInstId and similar (#5304)

Browse Source 
We frequently want to operate on singletons. Per discussion, drop
`Singleton` to make the code shorter.

This started off as wanting to write `inst_id.is_error()`, but the
dependency relationship between ids.h and singleton_insts.h would
require some kind of delayed evaluation to allow the implementation to
remain in headers (which I suspect is helpful to have for inlining). I
could have added something like `IsErrorInst`, forward declared in ids.h
and defined in singleton_insts.h (which would always be included by
typed_insts.h), but the template approach felt like a decent balance
between (a) removing the boilerplate `::SingletonInstId`, (b)
understandability, (c) still visually mirroring if we immediately return
a singleton, and (d) flexibility for more than just `ErrorInst`. But TBH
I'd probably still have written `is_error()` if it didn't require
addressing the cross-header cycle.

Then I tried `SemIR::InstId::Is<SemIR::ErrorInst>`, which generally
worked with types but generated the complaint that it didn't shorten
*all* singleton uses. So pulling back on `::Is`, and instead just
dropping `Singleton`.
This commit is contained in:
Jon Ross-Perkins
2025-04-15 22:40:29 +00:00
committed by GitHub
parent 838417e358
commit 4923445e3a
64 changed files with 489 additions and 539 deletions
Download Patch File Download Diff File Expand all files Collapse all files
toolchain/check/eval.cpp
+28 -32
View File
@@ -247,7 +247,7 @@ static auto GetPhase(const SemIR::ConstantValueStore& constant_values,
SemIR::ConstantId constant_id) -> Phase {
if (!constant_id.is_constant()) {
return Phase::Runtime;
} else if (constant_id == SemIR::ErrorInst::SingletonConstantId) {
} else if (constant_id == SemIR::ErrorInst::ConstantId) {
return Phase::UnknownDueToError;
}
switch (constant_values.GetDependence(constant_id)) {
@@ -285,7 +285,7 @@ static auto MakeConstantResult(Context& context, SemIR::Inst inst, Phase phase)
return context.constants().GetOrAdd(inst,
SemIR::ConstantDependence::Template);
case Phase::UnknownDueToError:
return SemIR::ErrorInst::SingletonConstantId;
return SemIR::ErrorInst::ConstantId;
case Phase::Runtime:
return SemIR::ConstantId::NotConstant;
}
@@ -293,9 +293,8 @@ static auto MakeConstantResult(Context& context, SemIR::Inst inst, Phase phase)
// Forms a `constant_id` describing why an evaluation was not constant.
static auto MakeNonConstantResult(Phase phase) -> SemIR::ConstantId {
return phase == Phase::UnknownDueToError
? SemIR::ErrorInst::SingletonConstantId
: SemIR::ConstantId::NotConstant;
return phase == Phase::UnknownDueToError ? SemIR::ErrorInst::ConstantId
: SemIR::ConstantId::NotConstant;
}
// Converts a bool value into a ConstantId.
@@ -334,11 +333,10 @@ static auto MakeFacetTypeResult(Context& context,
const SemIR::FacetTypeInfo& info, Phase phase)
-> SemIR::ConstantId {
SemIR::FacetTypeId facet_type_id = context.facet_types().Add(info);
return MakeConstantResult(
context,
SemIR::FacetType{.type_id = SemIR::TypeType::SingletonTypeId,
.facet_type_id = facet_type_id},
phase);
return MakeConstantResult(context,
SemIR::FacetType{.type_id = SemIR::TypeType::TypeId,
.facet_type_id = facet_type_id},
phase);
}
// `GetConstantValue` checks to see whether the provided ID describes a value
@@ -797,7 +795,7 @@ static auto PerformArrayIndex(EvalContext& eval_context, SemIR::ArrayIndex inst)
eval_context.emitter().Emit(
eval_context.GetDiagnosticLoc(inst.index_id), ArrayIndexOutOfBounds,
{.type = index->type_id, .value = index_val}, aggregate_type_id);
return SemIR::ErrorInst::SingletonConstantId;
return SemIR::ErrorInst::ConstantId;
}
}
}
@@ -824,11 +822,11 @@ static auto MakeIntTypeResult(Context& context, SemIR::LocId loc_id,
SemIR::IntKind int_kind, SemIR::InstId width_id,
Phase phase) -> SemIR::ConstantId {
auto result = SemIR::IntType{
.type_id = GetSingletonType(context, SemIR::TypeType::SingletonInstId),
.type_id = GetSingletonType(context, SemIR::TypeType::InstId),
.int_kind = int_kind,
.bit_width_id = width_id};
if (!ValidateIntType(context, loc_id, result)) {
return SemIR::ErrorInst::SingletonConstantId;
return SemIR::ErrorInst::ConstantId;
}
return MakeConstantResult(context, result, phase);
}
@@ -1101,7 +1099,7 @@ static auto PerformBuiltinIntShiftOp(Context& context, SemIR::LocId loc_id,
builtin_kind == SemIR::BuiltinFunctionKind::IntLeftShift,
{.type = rhs.type_id, .value = rhs_orig_val});
// TODO: Is it useful to recover by returning 0 or -1?
return SemIR::ErrorInst::SingletonConstantId;
return SemIR::ErrorInst::ConstantId;
}
if (rhs_orig_val.isNegative() &&
@@ -1115,7 +1113,7 @@ static auto PerformBuiltinIntShiftOp(Context& context, SemIR::LocId loc_id,
builtin_kind == SemIR::BuiltinFunctionKind::IntLeftShift,
{.type = rhs.type_id, .value = rhs_orig_val});
// TODO: Is it useful to recover by returning 0 or -1?
return SemIR::ErrorInst::SingletonConstantId;
return SemIR::ErrorInst::ConstantId;
}
llvm::APInt result_val;
@@ -1134,7 +1132,7 @@ static auto PerformBuiltinIntShiftOp(Context& context, SemIR::LocId loc_id,
context.emitter().Emit(loc_id, CompileTimeUnsizedShiftOutOfRange,
{.type = rhs.type_id, .value = rhs_orig_val},
IntStore::MaxIntWidth);
return SemIR::ErrorInst::SingletonConstantId;
return SemIR::ErrorInst::ConstantId;
}
lhs_val = lhs_val.sext(
IntStore::CanonicalBitWidth(lhs_val.getSignificantBits() + *width));
@@ -1178,7 +1176,7 @@ static auto PerformBuiltinBinaryIntOp(Context& context, SemIR::LocId loc_id,
case SemIR::BuiltinFunctionKind::IntUMod:
if (rhs_val.isZero()) {
DiagnoseDivisionByZero(context, loc_id);
return SemIR::ErrorInst::SingletonConstantId;
return SemIR::ErrorInst::ConstantId;
}
break;
default:
@@ -1423,7 +1421,7 @@ static auto MakeConstantForBuiltinCall(EvalContext& eval_context,
// Allow errors to be diagnosed for both sides of the operator before
// returning here if any error occurred on either side.
if (!lhs_facet_type_id.has_value() || !rhs_facet_type_id.has_value()) {
return SemIR::ErrorInst::SingletonConstantId;
return SemIR::ErrorInst::ConstantId;
}
// Reuse one of the argument instructions if nothing has changed.
if (lhs_facet_type_id == rhs_facet_type_id) {
@@ -1438,8 +1436,7 @@ static auto MakeConstantForBuiltinCall(EvalContext& eval_context,
}
case SemIR::BuiltinFunctionKind::IntLiteralMakeType: {
return context.constant_values().Get(
SemIR::IntLiteralType::SingletonInstId);
return context.constant_values().Get(SemIR::IntLiteralType::InstId);
}
case SemIR::BuiltinFunctionKind::IntMakeTypeSigned: {
@@ -1458,14 +1455,13 @@ static auto MakeConstantForBuiltinCall(EvalContext& eval_context,
break;
}
if (!ValidateFloatBitWidth(context, loc_id, arg_ids[0])) {
return SemIR::ErrorInst::SingletonConstantId;
return SemIR::ErrorInst::ConstantId;
}
return context.constant_values().Get(
SemIR::LegacyFloatType::SingletonInstId);
return context.constant_values().Get(SemIR::LegacyFloatType::InstId);
}
case SemIR::BuiltinFunctionKind::BoolMakeType: {
return context.constant_values().Get(SemIR::BoolType::SingletonInstId);
return context.constant_values().Get(SemIR::BoolType::InstId);
}
// Integer conversions.
@@ -1598,7 +1594,7 @@ static auto MakeConstantForCall(EvalContext& eval_context, SemIR::LocId loc_id,
//
// TODO: Use a better representation for this.
if (call.args_id == SemIR::InstBlockId::None) {
return SemIR::ErrorInst::SingletonConstantId;
return SemIR::ErrorInst::ConstantId;
}
// Find the constant value of the callee.
@@ -1631,7 +1627,7 @@ static auto MakeConstantForCall(EvalContext& eval_context, SemIR::LocId loc_id,
ReplaceFieldWithConstantValue(eval_context, &call, &SemIR::Call::args_id,
&phase);
if (phase == Phase::UnknownDueToError) {
return SemIR::ErrorInst::SingletonConstantId;
return SemIR::ErrorInst::ConstantId;
}
// If any operand of the call is non-constant, the call is non-constant.
@@ -1743,7 +1739,7 @@ static auto TryEvalTypedInst(EvalContext& eval_context, SemIR::InstId inst_id,
// The result is an instruction.
return MakeConstantResult(
eval_context.context(),
SemIR::InstValue{.type_id = SemIR::InstType::SingletonTypeId,
SemIR::InstValue{.type_id = SemIR::InstType::TypeId,
.inst_id = result_inst_id},
Phase::Concrete);
}
@@ -1880,10 +1876,10 @@ auto TryEvalTypedInst<SemIR::WhereExpr>(EvalContext& eval_context,
if (auto facet_type = base_facet_inst.TryAs<SemIR::FacetType>()) {
info = GetConstantFacetTypeInfo(eval_context, facet_type->facet_type_id,
&phase);
} else if (base_facet_type_id == SemIR::ErrorInst::SingletonTypeId) {
return SemIR::ErrorInst::SingletonConstantId;
} else if (base_facet_type_id == SemIR::ErrorInst::TypeId) {
return SemIR::ErrorInst::ConstantId;
} else {
CARBON_CHECK(base_facet_type_id == SemIR::TypeType::SingletonTypeId,
CARBON_CHECK(base_facet_type_id == SemIR::TypeType::TypeId,
"Unexpected type_id: {0}, inst: {1}", base_facet_type_id,
base_facet_inst);
}
@@ -1906,10 +1902,10 @@ auto TryEvalTypedInst<SemIR::WhereExpr>(EvalContext& eval_context,
inst_id)) {
SemIR::ConstantId lhs = eval_context.GetConstantValue(impls->lhs_id);
SemIR::ConstantId rhs = eval_context.GetConstantValue(impls->rhs_id);
if (rhs != SemIR::ErrorInst::SingletonConstantId &&
if (rhs != SemIR::ErrorInst::ConstantId &&
IsPeriodSelf(eval_context, lhs)) {
auto rhs_inst_id = eval_context.constant_values().GetInstId(rhs);
if (rhs_inst_id == SemIR::TypeType::SingletonInstId) {
if (rhs_inst_id == SemIR::TypeType::InstId) {
// `.Self impls type` -> nothing to do.
} else {
auto facet_type =