Skip to content

Commit c33adfa

Browse files
zygoloidzygoloid
authored
Replace GetTypeInSpecific with GetTypeOfInstInSpecific. (carbon-language#5232)
Reduce usage of `GetConstantInSpecific` to a single caller in constant evaluation, with a TODO to remove that. This gets us closer to being able to fully perform type-checking against abstract types instead of types anchored within a particular generic.
1 parent 077cf56 commit c33adfa

12 files changed

+132
-105
lines changed

‎toolchain/check/eval.cpp

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -96,26 +96,36 @@ class EvalContext {
9696

9797
// While resolving a specific, map from previous instructions in the eval
9898
// block into their evaluated values. These values won't be present on the
99-
// specific itself yet, so `GetConstantInSpecific` won't be able to find
100-
// them.
101-
if (specific_eval_info_) {
102-
const auto& symbolic_info =
103-
constant_values().GetSymbolicConstant(const_id);
104-
if (symbolic_info.index.has_value() &&
105-
symbolic_info.generic_id ==
106-
specifics().Get(specific_id_).generic_id &&
107-
symbolic_info.index.region() == specific_eval_info_->region) {
99+
// specific itself yet, so `GetConstantValueInSpecific` won't be able to
100+
// find them.
101+
const auto& symbolic_info = constant_values().GetSymbolicConstant(const_id);
102+
if (specific_eval_info_ && symbolic_info.index.has_value()) {
103+
CARBON_CHECK(
104+
symbolic_info.generic_id == specifics().Get(specific_id_).generic_id,
105+
"Instruction has constant operand in wrong generic");
106+
if (symbolic_info.index.region() == specific_eval_info_->region) {
108107
auto inst_id = specific_eval_info_->values[symbolic_info.index.index()];
109108
CARBON_CHECK(inst_id.has_value(),
110109
"Forward reference in eval block: index {0} referenced "
111110
"before evaluation",
112111
symbolic_info.index.index());
113112
return constant_values().Get(inst_id);
113+
} else {
114+
// TODO: Eliminate this call. This is the only place where we get a
115+
// value from a specific without using an InstId. There are three ways
116+
// we can get here:
117+
// 1) From GetConstantValue(InstId): these can use
118+
// GetConstantValueInSpecific.
119+
// 2) From GetConstantValue(TypeId): for these, we could change
120+
// instructions so they store InstIds instead of TypeIds.
121+
// 3) From GetConstantFacetTypeInfo: for these, we could store an
122+
// InstId instead of a ConstantId in rewrite_constraints.
123+
return GetConstantInSpecific(sem_ir(), specific_id_, const_id);
114124
}
115125
}
116126

117127
// Map from a specific constant value to the canonical value.
118-
return GetConstantInSpecific(sem_ir(), specific_id_, const_id);
128+
return constant_values().Get(symbolic_info.inst_id);
119129
}
120130

121131
// Gets the constant value of the specified instruction in this context.

‎toolchain/check/eval_inst.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ auto EvalConstantInst(Context& context, SemIRLoc loc,
243243

244244
auto EvalConstantInst(Context& context, SemIRLoc loc,
245245
SemIR::ImplWitnessAccess inst) -> ConstantEvalResult {
246-
// This is PerformAggregateAccess followed by GetConstantInSpecific.
246+
// This is PerformAggregateAccess followed by GetConstantValueInSpecific.
247247
if (auto witness =
248248
context.insts().TryGetAs<SemIR::ImplWitness>(inst.witness_id)) {
249249
auto elements = context.inst_blocks().Get(witness->elements_id);

‎toolchain/check/handle_name.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,10 +109,10 @@ static auto HandleNameAsExpr(Context& context, Parse::NodeId node_id,
109109
SemIR::NameId name_id) -> SemIR::InstId {
110110
auto result = LookupUnqualifiedName(context, node_id, name_id);
111111
SemIR::InstId inst_id = result.scope_result.target_inst_id();
112-
auto value = context.insts().Get(inst_id);
113-
auto type_id = SemIR::GetTypeInSpecific(context.sem_ir(), result.specific_id,
114-
value.type_id());
115-
CARBON_CHECK(type_id.has_value(), "Missing type for {0}", value);
112+
auto type_id = SemIR::GetTypeOfInstInSpecific(context.sem_ir(),
113+
result.specific_id, inst_id);
114+
CARBON_CHECK(type_id.has_value(), "Missing type for {0}",
115+
context.insts().Get(inst_id));
116116

117117
// If the named entity has a constant value that depends on its specific,
118118
// store the specific too.

‎toolchain/check/interface.cpp

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,8 @@ static auto GetSelfFacet(Context& context,
8181
SemIR::InstId self_witness_id) -> SemIR::InstId {
8282
auto self_binding_id =
8383
GetSelfBinding(context, interface_specific_id, generic_id);
84-
auto self_binding = context.insts().Get(self_binding_id);
85-
auto self_facet_type_id = SemIR::GetTypeInSpecific(
86-
context.sem_ir(), interface_specific_id, self_binding.type_id());
84+
auto self_facet_type_id = SemIR::GetTypeOfInstInSpecific(
85+
context.sem_ir(), interface_specific_id, self_binding_id);
8786
// Create a facet value to be the value of `Self` in the interface.
8887
// TODO: Pass this in instead of creating it here. The caller sometimes
8988
// already has a facet value.
@@ -190,15 +189,14 @@ auto GetTypeForSpecificAssociatedEntity(Context& context, SemIRLoc loc,
190189
GetGenericArgsWithSelfType(context, interface_specific_id, generic_id,
191190
self_type_id, self_witness_id);
192191
auto const_specific_id = MakeSpecific(context, loc, generic_id, arg_ids);
193-
return SemIR::GetTypeInSpecific(context.sem_ir(), const_specific_id,
194-
context.insts().Get(decl_id).type_id());
192+
return SemIR::GetTypeOfInstInSpecific(context.sem_ir(), const_specific_id,
193+
decl_id);
195194
} else if (auto fn = context.types().TryGetAs<SemIR::FunctionType>(
196195
decl.type_id())) {
197196
// Form the type of the function within the interface, and attach the `Self`
198197
// type.
199-
auto interface_fn_type_id =
200-
SemIR::GetTypeInSpecific(context.sem_ir(), interface_specific_id,
201-
context.insts().Get(decl_id).type_id());
198+
auto interface_fn_type_id = SemIR::GetTypeOfInstInSpecific(
199+
context.sem_ir(), interface_specific_id, decl_id);
202200
auto self_facet_id =
203201
GetSelfFacet(context, interface_specific_id,
204202
context.functions().Get(fn->function_id).generic_id,

‎toolchain/check/member_access.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -261,10 +261,11 @@ static auto LookupMemberNameInScope(Context& context, SemIR::LocId loc_id,
261261
}
262262

263263
// TODO: This duplicates the work that HandleNameAsExpr does. Factor this out.
264-
auto inst = context.insts().Get(result.scope_result.target_inst_id());
265-
auto type_id = SemIR::GetTypeInSpecific(context.sem_ir(), result.specific_id,
266-
inst.type_id());
267-
CARBON_CHECK(type_id.has_value(), "Missing type for member {0}", inst);
264+
auto type_id =
265+
SemIR::GetTypeOfInstInSpecific(context.sem_ir(), result.specific_id,
266+
result.scope_result.target_inst_id());
267+
CARBON_CHECK(type_id.has_value(), "Missing type for member {0}",
268+
context.insts().Get(result.scope_result.target_inst_id()));
268269

269270
// If the named entity has a constant value that depends on its specific,
270271
// store the specific too.

‎toolchain/check/merge.cpp

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,9 @@ static auto CheckRedeclParam(Context& context, bool is_implicit_param,
203203
SemIR::InstId prev_param_pattern_id,
204204
SemIR::SpecificId prev_specific_id, bool diagnose,
205205
bool check_self) -> bool {
206+
auto orig_new_param_pattern_id = new_param_pattern_id;
207+
auto orig_prev_param_pattern_id = prev_param_pattern_id;
208+
206209
// TODO: Consider differentiating between type and name mistakes. For now,
207210
// taking the simpler approach because I also think we may want to refactor
208211
// params.
@@ -218,9 +221,10 @@ static auto CheckRedeclParam(Context& context, bool is_implicit_param,
218221
"redeclaration differs at {0:implicit |}parameter {1}",
219222
Diagnostics::BoolAsSelect, int32_t);
220223
context.emitter()
221-
.Build(new_param_pattern_id, RedeclParamDiffers, is_implicit_param,
224+
.Build(orig_new_param_pattern_id, RedeclParamDiffers, is_implicit_param,
222225
param_index + 1)
223-
.Note(prev_param_pattern_id, RedeclParamPrevious, is_implicit_param)
226+
.Note(orig_prev_param_pattern_id, RedeclParamPrevious,
227+
is_implicit_param)
224228
.Emit();
225229
};
226230

@@ -232,21 +236,24 @@ static auto CheckRedeclParam(Context& context, bool is_implicit_param,
232236
}
233237

234238
if (new_param_pattern.Is<SemIR::AddrPattern>()) {
235-
new_param_pattern = context.insts().Get(
236-
new_param_pattern.As<SemIR::AddrPattern>().inner_id);
237-
prev_param_pattern = context.insts().Get(
238-
prev_param_pattern.As<SemIR::AddrPattern>().inner_id);
239+
new_param_pattern_id = new_param_pattern.As<SemIR::AddrPattern>().inner_id;
240+
new_param_pattern = context.insts().Get(new_param_pattern_id);
241+
prev_param_pattern_id =
242+
prev_param_pattern.As<SemIR::AddrPattern>().inner_id;
243+
prev_param_pattern = context.insts().Get(prev_param_pattern_id);
239244
if (new_param_pattern.kind() != prev_param_pattern.kind()) {
240245
emit_diagnostic();
241246
return false;
242247
}
243248
}
244249

245250
if (new_param_pattern.Is<SemIR::AnyParamPattern>()) {
246-
new_param_pattern = context.insts().Get(
247-
new_param_pattern.As<SemIR::ValueParamPattern>().subpattern_id);
248-
prev_param_pattern = context.insts().Get(
249-
prev_param_pattern.As<SemIR::ValueParamPattern>().subpattern_id);
251+
new_param_pattern_id =
252+
new_param_pattern.As<SemIR::ValueParamPattern>().subpattern_id;
253+
new_param_pattern = context.insts().Get(new_param_pattern_id);
254+
prev_param_pattern_id =
255+
prev_param_pattern.As<SemIR::ValueParamPattern>().subpattern_id;
256+
prev_param_pattern = context.insts().Get(prev_param_pattern_id);
250257
if (new_param_pattern.kind() != prev_param_pattern.kind()) {
251258
emit_diagnostic();
252259
return false;
@@ -267,8 +274,8 @@ static auto CheckRedeclParam(Context& context, bool is_implicit_param,
267274
return true;
268275
}
269276

270-
auto prev_param_type_id = SemIR::GetTypeInSpecific(
271-
context.sem_ir(), prev_specific_id, prev_param_pattern.type_id());
277+
auto prev_param_type_id = SemIR::GetTypeOfInstInSpecific(
278+
context.sem_ir(), prev_specific_id, prev_param_pattern_id);
272279
if (!context.types().AreEqualAcrossDeclarations(new_param_pattern.type_id(),
273280
prev_param_type_id)) {
274281
if (!diagnose) {
@@ -280,9 +287,11 @@ static auto CheckRedeclParam(Context& context, bool is_implicit_param,
280287
Diagnostics::BoolAsSelect, int32_t, SemIR::TypeId,
281288
SemIR::TypeId);
282289
context.emitter()
283-
.Build(new_param_pattern_id, RedeclParamDiffersType, is_implicit_param,
284-
param_index + 1, prev_param_type_id, new_param_pattern.type_id())
285-
.Note(prev_param_pattern_id, RedeclParamPrevious, is_implicit_param)
290+
.Build(orig_new_param_pattern_id, RedeclParamDiffersType,
291+
is_implicit_param, param_index + 1, prev_param_type_id,
292+
new_param_pattern.type_id())
293+
.Note(orig_prev_param_pattern_id, RedeclParamPrevious,
294+
is_implicit_param)
286295
.Emit();
287296
return false;
288297
}

0 commit comments

Comments
 (0)