Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disallow creating instances of abstract classes #4381

Merged
merged 21 commits into from
Oct 12, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 35 additions & 2 deletions toolchain/check/context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,17 @@ auto Context::DiagnoseNameNotFound(SemIRLoc loc, SemIR::NameId name_id)
emitter_->Emit(loc, NameNotFound, name_id);
}

auto Context::NoteAbstractClass(SemIR::ClassId class_id,
DiagnosticBuilder& builder) -> void {
const auto& class_info = classes().Get(class_id);
CARBON_CHECK(
class_info.inheritance_kind == SemIR::Class::InheritanceKind::Abstract,
"Class is not abstract");
CARBON_DIAGNOSTIC(ClassAbstractHere, Note,
"class was declared abstract here");
builder.Note(class_info.definition_id, ClassAbstractHere);
}

auto Context::NoteIncompleteClass(SemIR::ClassId class_id,
DiagnosticBuilder& builder) -> void {
const auto& class_info = classes().Get(class_id);
Expand Down Expand Up @@ -1127,8 +1138,30 @@ class TypeCompleter {
} // namespace

auto Context::TryToCompleteType(SemIR::TypeId type_id,
BuildDiagnosticFn diagnoser) -> bool {
return TypeCompleter(*this, diagnoser).Complete(type_id);
BuildDiagnosticFn diagnoser,
BuildDiagnosticFn abstract_diagnoser) -> bool {
if (!TypeCompleter(*this, diagnoser).Complete(type_id)) {
return false;
}

if (!abstract_diagnoser) {
return true;
}

if (auto class_type = types().TryGetAs<SemIR::ClassType>(type_id)) {
auto& class_info = classes().Get(class_type->class_id);
if (class_info.inheritance_kind !=
SemIR::Class::InheritanceKind::Abstract) {
return true;
}

auto builder = abstract_diagnoser();
NoteAbstractClass(class_type->class_id, builder);
builder.Emit();
return false;
}

return true;
}

auto Context::TryToDefineType(SemIR::TypeId type_id,
Expand Down
20 changes: 14 additions & 6 deletions toolchain/check/context.h
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,10 @@ class Context {
auto NoteIncompleteClass(SemIR::ClassId class_id, DiagnosticBuilder& builder)
-> void;

// Adds a note to a diagnostic explaining that a class is abstract.
auto NoteAbstractClass(SemIR::ClassId class_id, DiagnosticBuilder& builder)
-> void;

// Adds a note to a diagnostic explaining that an interface is not defined.
auto NoteUndefinedInterface(SemIR::InterfaceId interface_id,
DiagnosticBuilder& builder) -> void;
Expand Down Expand Up @@ -318,8 +322,11 @@ class Context {
// If the type is not complete, `diagnoser` is invoked to diagnose the issue,
// if a `diagnoser` is provided. The builder it returns will be annotated to
// describe the reason why the type is not complete.
auto TryToCompleteType(SemIR::TypeId type_id,
BuildDiagnosticFn diagnoser = nullptr) -> bool;
auto TryToCompleteType(
SemIR::TypeId type_id,
BuildDiagnosticFn diagnoser = nullptr,
BuildDiagnosticFn abstract_diagnoser = nullptr)
-> bool;

// Attempts to complete and define the type `type_id`. Returns `true` if the
// type is defined, or `false` if no definition is available. A defined type
Expand All @@ -333,11 +340,12 @@ class Context {
// Returns the type `type_id` as a complete type, or produces an incomplete
// type error and returns an error type. This is a convenience wrapper around
// TryToCompleteType. `diagnoser` must not be null.
auto AsCompleteType(SemIR::TypeId type_id, BuildDiagnosticFn diagnoser)
auto AsCompleteType(SemIR::TypeId type_id, BuildDiagnosticFn diagnoser,
BuildDiagnosticFn abstract_diagnoser)
-> SemIR::TypeId {
CARBON_CHECK(diagnoser);
return TryToCompleteType(type_id, diagnoser) ? type_id
: SemIR::TypeId::Error;
return TryToCompleteType(type_id, diagnoser, abstract_diagnoser)
? type_id
: SemIR::TypeId::Error;
}

// Returns whether `type_id` represents a facet type.
Expand Down
58 changes: 40 additions & 18 deletions toolchain/check/convert.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -937,24 +937,46 @@ auto Convert(Context& context, SemIR::LocId loc_id, SemIR::InstId expr_id,
}

// We can only perform initialization for complete types.
if (!context.TryToCompleteType(target.type_id, [&] {
CARBON_DIAGNOSTIC(IncompleteTypeInInit, Error,
"initialization of incomplete type `{0}`",
SemIR::TypeId);
CARBON_DIAGNOSTIC(IncompleteTypeInValueConversion, Error,
"forming value of incomplete type `{0}`",
SemIR::TypeId);
CARBON_DIAGNOSTIC(IncompleteTypeInConversion, Error,
"invalid use of incomplete type `{0}`",
SemIR::TypeId);
return context.emitter().Build(loc_id,
target.is_initializer()
? IncompleteTypeInInit
: target.kind == ConversionTarget::Value
? IncompleteTypeInValueConversion
: IncompleteTypeInConversion,
target.type_id);
})) {
if (!context.TryToCompleteType(
target.type_id,
[&] {
CARBON_DIAGNOSTIC(IncompleteTypeInInit, Error,
"initialization of incomplete type `{0}`",
SemIR::TypeId);
CARBON_DIAGNOSTIC(IncompleteTypeInValueConversion, Error,
"forming value of incomplete type `{0}`",
SemIR::TypeId);
CARBON_DIAGNOSTIC(IncompleteTypeInConversion, Error,
"invalid use of incomplete type `{0}`",
SemIR::TypeId);
assert(!target.is_initializer());
assert(target.kind == ConversionTarget::Value);
return context.emitter().Build(
loc_id,
target.is_initializer() ? IncompleteTypeInInit
: target.kind == ConversionTarget::Value
? IncompleteTypeInValueConversion
: IncompleteTypeInConversion,
target.type_id);
},
[&] {
CARBON_DIAGNOSTIC(AbstractTypeInInit, Error,
"initialization of abstract type `{0}`",
SemIR::TypeId);
CARBON_DIAGNOSTIC(AbstractTypeInValueConversion, Error,
"forming value of abstract type `{0}`",
SemIR::TypeId);
CARBON_DIAGNOSTIC(AbstractTypeInConversion, Error,
"invalid use of abstract type `{0}`",
SemIR::TypeId);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we don't want to diagnose in these two cases, only when initializing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done (this also included adding null DiagnosticBuilder - open to riffing on the API (currently just uses the default ctor to create a null DiagnosticBuilder, but could use a named ctor of some kind - and has explicit bool conversion which could be some more explicitly named function))

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a named constructor would be nice. Maybe even a method on DiagnosticEmitter, say BuildSuppressed or something, so you don't need to repeat the template argument.

return context.emitter().Build(
loc_id,
target.is_initializer() ? AbstractTypeInInit
: target.kind == ConversionTarget::Value
? AbstractTypeInValueConversion
: AbstractTypeInConversion,
target.type_id);
})) {
return SemIR::InstId::BuiltinError;
}

Expand Down
9 changes: 8 additions & 1 deletion toolchain/check/function.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,18 @@ auto CheckFunctionReturnType(Context& context, SemIRLoc loc,
return context.emitter().Build(loc, IncompleteTypeInFunctionReturnType,
return_info.type_id);
};
auto diagnose_abstract_return_type = [&] {
CARBON_DIAGNOSTIC(AbstractTypeInFunctionReturnType, Error,
"function returns abstract type `{0}`", SemIR::TypeId);
return context.emitter().Build(loc, AbstractTypeInFunctionReturnType,
return_info.type_id);
};

// TODO: Consider suppressing the diagnostic if we've already diagnosed a
// definition or call to this function.
if (context.TryToCompleteType(return_info.type_id,
diagnose_incomplete_return_type)) {
diagnose_incomplete_return_type,
diagnose_abstract_return_type)) {
return_info = SemIR::ReturnTypeInfo::ForFunction(context.sem_ir(),
function, specific_id);
}
Expand Down
55 changes: 38 additions & 17 deletions toolchain/check/handle_binding_pattern.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,16 +99,28 @@ static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id,

// A `var` declaration at class scope introduces a field.
auto parent_class_decl = context.GetCurrentScopeAs<SemIR::ClassDecl>();
cast_type_id = context.AsCompleteType(cast_type_id, [&] {
CARBON_DIAGNOSTIC(IncompleteTypeInVarDecl, Error,
"{0} has incomplete type `{1}`", llvm::StringLiteral,
SemIR::TypeId);
return context.emitter().Build(type_node, IncompleteTypeInVarDecl,
parent_class_decl
? llvm::StringLiteral("Field")
: llvm::StringLiteral("Variable"),
cast_type_id);
});
cast_type_id = context.AsCompleteType(
cast_type_id,
[&] {
CARBON_DIAGNOSTIC(IncompleteTypeInVarDecl, Error,
"{0} has incomplete type `{1}`",
llvm::StringLiteral, SemIR::TypeId);
return context.emitter().Build(
type_node, IncompleteTypeInVarDecl,
parent_class_decl ? llvm::StringLiteral("Field")
: llvm::StringLiteral("Variable"),
cast_type_id);
},
[&] {
CARBON_DIAGNOSTIC(AbstractTypeInVarDecl, Error,
"{0} has abstract type `{1}`",
llvm::StringLiteral, SemIR::TypeId);
return context.emitter().Build(
type_node, AbstractTypeInVarDecl,
parent_class_decl ? llvm::StringLiteral("Field")
: llvm::StringLiteral("Variable"),
cast_type_id);
});
if (parent_class_decl) {
CARBON_CHECK(context_node_kind == Parse::NodeKind::VariableIntroducer,
"`returned var` at class scope");
Expand Down Expand Up @@ -185,13 +197,22 @@ static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id,
}

case Parse::NodeKind::LetIntroducer: {
cast_type_id = context.AsCompleteType(cast_type_id, [&] {
CARBON_DIAGNOSTIC(IncompleteTypeInLetDecl, Error,
"`let` binding has incomplete type `{0}`",
SemIR::TypeId);
return context.emitter().Build(type_node, IncompleteTypeInLetDecl,
cast_type_id);
});
cast_type_id = context.AsCompleteType(
cast_type_id,
[&] {
CARBON_DIAGNOSTIC(IncompleteTypeInLetDecl, Error,
"`let` binding has incomplete type `{0}`",
SemIR::TypeId);
return context.emitter().Build(type_node, IncompleteTypeInLetDecl,
cast_type_id);
},
[&] {
CARBON_DIAGNOSTIC(AbstractTypeInLetDecl, Error,
"`let` binding has abstract type `{0}`",
SemIR::TypeId);
return context.emitter().Build(type_node, AbstractTypeInLetDecl,
cast_type_id);
});
// Create the instruction, but don't add it to a block until after we've
// formed its initializer.
// TODO: For general pattern parsing, we'll need to create a block to hold
Expand Down
38 changes: 25 additions & 13 deletions toolchain/check/handle_class.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -393,13 +393,22 @@ auto HandleParseNode(Context& context, Parse::AdaptDeclId node_id) -> bool {

auto adapted_type_id =
ExprAsType(context, node_id, adapted_type_expr_id).type_id;
adapted_type_id = context.AsCompleteType(adapted_type_id, [&] {
CARBON_DIAGNOSTIC(IncompleteTypeInAdaptDecl, Error,
"adapted type `{0}` is an incomplete type",
SemIR::TypeId);
return context.emitter().Build(node_id, IncompleteTypeInAdaptDecl,
adapted_type_id);
});
adapted_type_id = context.AsCompleteType(
adapted_type_id,
[&] {
CARBON_DIAGNOSTIC(IncompleteTypeInAdaptDecl, Error,
"adapted type `{0}` is an incomplete type",
SemIR::TypeId);
return context.emitter().Build(node_id, IncompleteTypeInAdaptDecl,
adapted_type_id);
},
[&] {
CARBON_DIAGNOSTIC(AbstractTypeInAdaptDecl, Error,
"adapted type `{0}` is an abstract type",
SemIR::TypeId);
return context.emitter().Build(node_id, AbstractTypeInAdaptDecl,
adapted_type_id);
});

// Build a SemIR representation for the declaration.
class_info.adapt_id = context.AddInst<SemIR::AdaptDecl>(
Expand Down Expand Up @@ -469,12 +478,15 @@ static auto DiagnoseBaseIsFinal(Context& context, Parse::NodeId node_id,
static auto CheckBaseType(Context& context, Parse::NodeId node_id,
SemIR::InstId base_expr_id) -> BaseInfo {
auto base_type_id = ExprAsType(context, node_id, base_expr_id).type_id;
base_type_id = context.AsCompleteType(base_type_id, [&] {
CARBON_DIAGNOSTIC(IncompleteTypeInBaseDecl, Error,
"base `{0}` is an incomplete type", SemIR::TypeId);
return context.emitter().Build(node_id, IncompleteTypeInBaseDecl,
base_type_id);
});
base_type_id = context.AsCompleteType(
base_type_id,
[&] {
CARBON_DIAGNOSTIC(IncompleteTypeInBaseDecl, Error,
"base `{0}` is an incomplete type", SemIR::TypeId);
return context.emitter().Build(node_id, IncompleteTypeInBaseDecl,
base_type_id);
},
nullptr);

if (base_type_id == SemIR::TypeId::Error) {
return BaseInfo::Error;
Expand Down
29 changes: 20 additions & 9 deletions toolchain/check/handle_function.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -364,15 +364,26 @@ static auto HandleFunctionDefinitionAfterSignature(
SemIR::Function::GetParamFromParamRefId(context.sem_ir(), param_ref_id);

// The parameter types need to be complete.
context.TryToCompleteType(param_info.inst.type_id, [&] {
CARBON_DIAGNOSTIC(
IncompleteTypeInFunctionParam, Error,
"parameter has incomplete type `{0}` in function definition",
SemIR::TypeId);
return context.emitter().Build(param_info.inst_id,
IncompleteTypeInFunctionParam,
param_info.inst.type_id);
});
context.TryToCompleteType(
param_info.inst.type_id,
[&] {
CARBON_DIAGNOSTIC(
IncompleteTypeInFunctionParam, Error,
"parameter has incomplete type `{0}` in function definition",
SemIR::TypeId);
return context.emitter().Build(param_info.inst_id,
IncompleteTypeInFunctionParam,
param_info.inst.type_id);
},
[&] {
CARBON_DIAGNOSTIC(
AbstractTypeInFunctionParam, Error,
"parameter has abstract type `{0}` in function definition",
SemIR::TypeId);
return context.emitter().Build(param_info.inst_id,
AbstractTypeInFunctionParam,
param_info.inst.type_id);
});
}

context.node_stack().Push(node_id, function_id);
Expand Down
Loading