Skip to content

Commit

Permalink
Produce a note indicating where the specific was used from if monomor…
Browse files Browse the repository at this point in the history 
…phization fails. (#4662)

Also fix a bug in `Context::GetClassType` that previously tried to
complete the class type before returning it. That's not correct --
`GetCompleteTypeImpl` is only appropriate for cases where the type can
trivially be completed and completing it can't fail -- and led to
infinite recursion with this change because we would call `GetClassType`
when producing a diagnostic if completing that class type failed.
  • Loading branch information
@zygoloid
zygoloid authored Dec 11, 2024
1 parent 042ac39 commit 758b6c4
Show file tree
Hide file tree
Showing 16 changed files with 179 additions and 102 deletions.
5 changes: 4 additions & 1 deletion toolchain/check/check_unit.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,10 @@ auto CheckUnit::CheckRequiredDefinitions() -> void {
CARBON_FATAL("TODO: Support interfaces in DiagnoseMissingDefinitions");
}
case CARBON_KIND(SemIR::SpecificFunction specific_function): {
if (!ResolveSpecificDefinition(context_,
// TODO: Track a location for the use. In general we may want to track a
// list of enclosing locations if this was used from a generic.
SemIRLoc use_loc = decl_inst_id;
if (!ResolveSpecificDefinition(context_, use_loc,
specific_function.specific_id)) {
CARBON_DIAGNOSTIC(MissingGenericFunctionDefinition, Error,
"use of undefined generic function");
Expand Down
63 changes: 20 additions & 43 deletions toolchain/check/context.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,47 +228,21 @@ auto Context::DiagnoseNameNotFound(SemIRLoc loc, SemIR::NameId name_id)
emitter_->Emit(loc, NameNotFound, name_id);
}

// Given an instruction associated with a scope and a `SpecificId` for that
// scope, returns an instruction that describes the specific scope.
static auto GetInstForSpecificScope(Context& context, SemIR::InstId inst_id,
SemIR::SpecificId specific_id)
-> SemIR::InstId {
if (!specific_id.is_valid()) {
return inst_id;
}
auto inst = context.insts().Get(inst_id);
CARBON_KIND_SWITCH(inst) {
case CARBON_KIND(SemIR::ClassDecl class_decl): {
return context.types().GetInstId(
context.GetClassType(class_decl.class_id, specific_id));
}
case CARBON_KIND(SemIR::InterfaceDecl interface_decl): {
return context.types().GetInstId(
context.GetInterfaceType(interface_decl.interface_id, specific_id));
}
default: {
// Don't know how to form a specific for this generic scope.
// TODO: Handle more cases.
return SemIR::InstId::Invalid;
}
}
}

auto Context::DiagnoseMemberNameNotFound(
SemIRLoc loc, SemIR::NameId name_id,
llvm::ArrayRef<LookupScope> lookup_scopes) -> void {
if (lookup_scopes.size() == 1 &&
lookup_scopes.front().name_scope_id.is_valid()) {
const auto& scope = name_scopes().Get(lookup_scopes.front().name_scope_id);
if (auto specific_inst_id = GetInstForSpecificScope(
*this, scope.inst_id(), lookup_scopes.front().specific_id);
specific_inst_id.is_valid()) {
CARBON_DIAGNOSTIC(MemberNameNotFoundInScope, Error,
"member name `{0}` not found in {1}", SemIR::NameId,
InstIdAsType);
emitter_->Emit(loc, MemberNameNotFoundInScope, name_id, specific_inst_id);
return;
}
auto specific_id = lookup_scopes.front().specific_id;
auto scope_inst_id =
specific_id.is_valid()
? GetInstForSpecific(*this, specific_id)
: name_scopes().Get(lookup_scopes.front().name_scope_id).inst_id();
CARBON_DIAGNOSTIC(MemberNameNotFoundInScope, Error,
"member name `{0}` not found in {1}", SemIR::NameId,
InstIdAsType);
emitter_->Emit(loc, MemberNameNotFoundInScope, name_id, scope_inst_id);
return;
}

CARBON_DIAGNOSTIC(MemberNameNotFound, Error, "member name `{0}` not found",
Expand Down Expand Up @@ -895,8 +869,9 @@ namespace {
// complete.
class TypeCompleter {
public:
TypeCompleter(Context& context, Context::BuildDiagnosticFn diagnoser)
: context_(context), diagnoser_(diagnoser) {}
TypeCompleter(Context& context, SemIRLoc loc,
Context::BuildDiagnosticFn diagnoser)
: context_(context), loc_(loc), diagnoser_(diagnoser) {}

// Attempts to complete the given type. Returns true if it is now complete,
// false if it could not be completed.
Expand Down Expand Up @@ -1009,7 +984,7 @@ class TypeCompleter {
return false;
}
if (inst.specific_id.is_valid()) {
ResolveSpecificDefinition(context_, inst.specific_id);
ResolveSpecificDefinition(context_, loc_, inst.specific_id);
}
if (auto adapted_type_id =
class_info.GetAdaptedType(context_.sem_ir(), inst.specific_id);
Expand Down Expand Up @@ -1281,19 +1256,21 @@ class TypeCompleter {

Context& context_;
llvm::SmallVector<WorkItem> work_list_;
SemIRLoc loc_;
Context::BuildDiagnosticFn diagnoser_;
};
} // namespace

auto Context::TryToCompleteType(SemIR::TypeId type_id) -> bool {
return TypeCompleter(*this, nullptr).Complete(type_id);
// TODO: We need a location here in case we need to instantiate a class type.
return TypeCompleter(*this, SemIR::LocId::Invalid, nullptr).Complete(type_id);
}

auto Context::RequireCompleteType(SemIR::TypeId type_id, SemIR::LocId loc_id,
BuildDiagnosticFn diagnoser) -> bool {
CARBON_CHECK(diagnoser);

if (!TypeCompleter(*this, diagnoser).Complete(type_id)) {
if (!TypeCompleter(*this, loc_id, diagnoser).Complete(type_id)) {
return false;
}

Expand Down Expand Up @@ -1358,7 +1335,7 @@ auto Context::RequireDefinedType(SemIR::TypeId type_id, SemIR::LocId loc_id,
}

if (interface.specific_id.is_valid()) {
ResolveSpecificDefinition(*this, interface.specific_id);
ResolveSpecificDefinition(*this, loc_id, interface.specific_id);
}
}
// TODO: Finish facet type resolution.
Expand Down Expand Up @@ -1443,7 +1420,7 @@ auto Context::GetSingletonType(SemIR::InstId singleton_id) -> SemIR::TypeId {

auto Context::GetClassType(SemIR::ClassId class_id,
SemIR::SpecificId specific_id) -> SemIR::TypeId {
return GetCompleteTypeImpl<SemIR::ClassType>(*this, class_id, specific_id);
return GetTypeImpl<SemIR::ClassType>(*this, class_id, specific_id);
}

auto Context::GetFunctionType(SemIR::FunctionId fn_id,
Expand Down
11 changes: 6 additions & 5 deletions toolchain/check/decl_name_stack.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,8 @@ auto DeclNameStack::Restore(SuspendedName sus) -> void {
// Reattempt to resolve the definition of the specific. The generic might
// have been defined after we suspended this scope.
if (suspended_scope.entry.specific_id.is_valid()) {
ResolveSpecificDefinition(*context_, suspended_scope.entry.specific_id);
ResolveSpecificDefinition(*context_, sus.name_context.loc_id,
suspended_scope.entry.specific_id);
}

context_->scope_stack().Restore(std::move(suspended_scope));
Expand Down Expand Up @@ -192,7 +193,7 @@ auto DeclNameStack::LookupOrAddName(NameContext name_context,
// `fn Class(T:! type).F(n: i32)` we will push the scope for `Class(T:! type)`
// between the scope containing the declaration of `T` and the scope
// containing the declaration of `n`.
static auto PushNameQualifierScope(Context& context,
static auto PushNameQualifierScope(Context& context, SemIRLoc loc,
SemIR::InstId scope_inst_id,
SemIR::NameScopeId scope_id,
SemIR::SpecificId specific_id,
Expand All @@ -203,7 +204,7 @@ static auto PushNameQualifierScope(Context& context,

// When declaring a member of a generic, resolve the self specific.
if (specific_id.is_valid()) {
ResolveSpecificDefinition(context, specific_id);
ResolveSpecificDefinition(context, loc, specific_id);
}

context.scope_stack().Push(scope_inst_id, scope_id, specific_id, has_error);
Expand All @@ -229,8 +230,8 @@ auto DeclNameStack::ApplyNameQualifier(const NameComponent& name) -> void {
// Resolve the qualifier as a scope and enter the new scope.
auto [scope_id, specific_id] = ResolveAsScope(name_context, name);
if (scope_id.is_valid()) {
PushNameQualifierScope(*context_, name_context.resolved_inst_id, scope_id,
specific_id,
PushNameQualifierScope(*context_, name_context.loc_id,
name_context.resolved_inst_id, scope_id, specific_id,
context_->name_scopes().Get(scope_id).has_error());
name_context.parent_scope_id = scope_id;
} else {
Expand Down
2 changes: 1 addition & 1 deletion toolchain/check/deduce.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,7 @@ auto DeductionContext::MakeSpecific() -> SemIR::SpecificId {
// TODO: Convert the deduced values to the types of the bindings.

return Check::MakeSpecific(
context(), generic_id_,
context(), loc_id_, generic_id_,
context().inst_blocks().AddCanonical(result_arg_ids_));
}

Expand Down
67 changes: 51 additions & 16 deletions toolchain/check/eval.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,32 @@ struct SpecificEvalInfo {
class EvalContext {
public:
explicit EvalContext(
Context& context,
Context& context, SemIRLoc fallback_loc,
SemIR::SpecificId specific_id = SemIR::SpecificId::Invalid,
std::optional<SpecificEvalInfo> specific_eval_info = std::nullopt)
: context_(context),
fallback_loc_(fallback_loc),
specific_id_(specific_id),
specific_eval_info_(specific_eval_info) {}

// Gets the location to use for diagnostics if a better location is
// unavailable.
// TODO: This is also sometimes unavailable.
auto fallback_loc() const -> SemIRLoc { return fallback_loc_; }

// Returns a location to use to point at an instruction in a diagnostic, given
// a list of instructions that might have an attached location. This is the
// location of the first instruction in the list that has a location if there
// is one, and otherwise the fallback location.
auto GetDiagnosticLoc(llvm::ArrayRef<SemIR::InstId> inst_ids) -> SemIRLoc {
for (auto inst_id : inst_ids) {
if (inst_id.is_valid() && context_.insts().GetLocId(inst_id).is_valid()) {
return inst_id;
}
}
return fallback_loc_;
}

// Gets the value of the specified compile-time binding in this context.
// Returns `Invalid` if the value is not fixed in this context.
auto GetCompileTimeBindValue(SemIR::CompileTimeBindIndex bind_index)
Expand Down Expand Up @@ -161,6 +180,8 @@ class EvalContext {
private:
// The type-checking context in which we're performing evaluation.
Context& context_;
// The location to use for diagnostics when a better location isn't available.
SemIRLoc fallback_loc_;
// The specific that we are evaluating within.
SemIR::SpecificId specific_id_;
// If we are currently evaluating an eval block for `specific_id_`,
Expand Down Expand Up @@ -419,7 +440,8 @@ static auto GetConstantValue(EvalContext& eval_context,
if (args_id == specific.args_id) {
return specific_id;
}
return MakeSpecific(eval_context.context(), specific.generic_id, args_id);
return MakeSpecific(eval_context.context(), eval_context.fallback_loc(),
specific.generic_id, args_id);
}

// Like `GetConstantValue` but does a `FacetTypeId` -> `FacetTypeInfo`
Expand Down Expand Up @@ -602,7 +624,7 @@ static auto PerformArrayIndex(EvalContext& eval_context, SemIR::ArrayIndex inst)
"array index `{0}` is past the end of type {1}",
TypedInt, SemIR::TypeId);
eval_context.emitter().Emit(
inst.index_id, ArrayIndexOutOfBounds,
eval_context.GetDiagnosticLoc(inst.index_id), ArrayIndexOutOfBounds,
{.type = index->type_id, .value = index_val}, aggregate_type_id);
return SemIR::ErrorInst::SingletonConstantId;
}
Expand Down Expand Up @@ -1336,15 +1358,15 @@ static auto TryEvalInstInContext(EvalContext& eval_context,
CARBON_DIAGNOSTIC(ArrayBoundNegative, Error,
"array bound of {0} is negative", TypedInt);
eval_context.emitter().Emit(
bound_id, ArrayBoundNegative,
eval_context.GetDiagnosticLoc(bound_id), ArrayBoundNegative,
{.type = int_bound->type_id, .value = bound_val});
return false;
}
if (bound_val.getActiveBits() > 64) {
CARBON_DIAGNOSTIC(ArrayBoundTooLarge, Error,
"array bound of {0} is too large", TypedInt);
eval_context.emitter().Emit(
bound_id, ArrayBoundTooLarge,
eval_context.GetDiagnosticLoc(bound_id), ArrayBoundTooLarge,
{.type = int_bound->type_id, .value = bound_val});
return false;
}
Expand Down Expand Up @@ -1393,7 +1415,8 @@ static auto TryEvalInstInContext(EvalContext& eval_context,
[&](SemIR::IntType result) {
return ValidateIntType(
eval_context.context(),
inst_id.is_valid() ? inst_id : int_type.bit_width_id, result);
eval_context.GetDiagnosticLoc({inst_id, int_type.bit_width_id}),
result);
},
&SemIR::IntType::bit_width_id);
}
Expand All @@ -1405,7 +1428,9 @@ static auto TryEvalInstInContext(EvalContext& eval_context,
eval_context, inst,
[&](SemIR::FloatType result) {
return ValidateFloatType(eval_context.context(),
float_type.bit_width_id, result);
eval_context.GetDiagnosticLoc(
{inst_id, float_type.bit_width_id}),
result);
},
&SemIR::FloatType::bit_width_id);
}
Expand Down Expand Up @@ -1568,7 +1593,8 @@ static auto TryEvalInstInContext(EvalContext& eval_context,
}

case CARBON_KIND(SemIR::Call call): {
return MakeConstantForCall(eval_context, inst_id, call);
return MakeConstantForCall(eval_context,
eval_context.GetDiagnosticLoc(inst_id), call);
}

// TODO: These need special handling.
Expand Down Expand Up @@ -1785,7 +1811,8 @@ static auto TryEvalInstInContext(EvalContext& eval_context,
"{0} evaluates to incomplete type {1}",
SemIR::TypeId, SemIR::TypeId);
return eval_context.emitter().Build(
inst_id, IncompleteTypeInMonomorphization,
eval_context.GetDiagnosticLoc(inst_id),
IncompleteTypeInMonomorphization,
require_complete.complete_type_id, complete_type_id);
});
if (complete_type_id == SemIR::ErrorInst::SingletonTypeId) {
Expand Down Expand Up @@ -1842,11 +1869,12 @@ static auto TryEvalInstInContext(EvalContext& eval_context,

auto TryEvalInst(Context& context, SemIR::InstId inst_id, SemIR::Inst inst)
-> SemIR::ConstantId {
EvalContext eval_context(context);
EvalContext eval_context(context, inst_id);
return TryEvalInstInContext(eval_context, inst_id, inst);
}

auto TryEvalBlockForSpecific(Context& context, SemIR::SpecificId specific_id,
auto TryEvalBlockForSpecific(Context& context, SemIRLoc loc,
SemIR::SpecificId specific_id,
SemIR::GenericInstIndex::Region region)
-> SemIR::InstBlockId {
auto generic_id = context.specifics().Get(specific_id).generic_id;
Expand All @@ -1856,20 +1884,27 @@ auto TryEvalBlockForSpecific(Context& context, SemIR::SpecificId specific_id,
llvm::SmallVector<SemIR::InstId> result;
result.resize(eval_block.size(), SemIR::InstId::Invalid);

EvalContext eval_context(context, specific_id,
EvalContext eval_context(context, loc, specific_id,
SpecificEvalInfo{
.region = region,
.values = result,
});

DiagnosticAnnotationScope annotate_diagnostics(
&context.emitter(), [&](auto& builder) {
CARBON_DIAGNOSTIC(ResolvingSpecificHere, Note, "in {0} used here",
InstIdAsType);
if (loc.is_inst_id && !loc.inst_id.is_valid()) {
return;
}
builder.Note(loc, ResolvingSpecificHere,
GetInstForSpecific(context, specific_id));
});

for (auto [i, inst_id] : llvm::enumerate(eval_block)) {
auto const_id = TryEvalInstInContext(eval_context, inst_id,
context.insts().Get(inst_id));
result[i] = context.constant_values().GetInstId(const_id);

// TODO: If this becomes possible through monomorphization failure, produce
// a diagnostic and put `SemIR::ErrorInst::SingletonInstId` in the table
// entry.
CARBON_CHECK(result[i].is_valid());
}

Expand Down
3 changes: 2 additions & 1 deletion toolchain/check/eval.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ auto TryEvalInst(Context& context, SemIR::InstId inst_id, SemIR::Inst inst)
// Evaluates the eval block for a region of a specific. Produces a block
// containing the evaluated constant values of the instructions in the eval
// block.
auto TryEvalBlockForSpecific(Context& context, SemIR::SpecificId specific_id,
auto TryEvalBlockForSpecific(Context& context, SemIRLoc loc,
SemIR::SpecificId specific_id,
SemIR::GenericInstIndex::Region region)
-> SemIR::InstBlockId;

Expand Down
Loading

0 comments on commit 758b6c4

Please sign in to comment.