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 14 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
40 changes: 38 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,33 @@ 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();
if (!builder) {
return false;
}
NoteAbstractClass(class_type->class_id, builder);
builder.Emit();
return false;
}

return true;
}

auto Context::TryToDefineType(SemIR::TypeId type_id,
Expand Down
17 changes: 12 additions & 5 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 @@ -319,7 +323,9 @@ class Context {
// 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;
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 +339,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 = nullptr)
-> 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
53 changes: 35 additions & 18 deletions toolchain/check/convert.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -937,24 +937,41 @@ 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);
if (!target.is_initializer()) {
return DiagnosticEmitter<SemIRLoc>::DiagnosticBuilder();
}
// return // add support for creating a null DiagnosticBuilder and
// return one here. Include null-testability and use that
// to skip adding notes in the caller if it's null.
dwblaikie marked this conversation as resolved.
Show resolved Hide resolved
return context.emitter().Build(loc_id, AbstractTypeInInit,
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
32 changes: 22 additions & 10 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
23 changes: 16 additions & 7 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);
});
zygoloid marked this conversation as resolved.
Show resolved Hide resolved

// Build a SemIR representation for the declaration.
class_info.adapt_id = context.AddInst<SemIR::AdaptDecl>(
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);
});
zygoloid marked this conversation as resolved.
Show resolved Hide resolved
}

context.node_stack().Push(node_id, function_id);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

class Incomplete;

// CHECK:STDERR: fail_incomplete_element.carbon:[[@LINE+6]]:8: error: Variable has incomplete type `[Incomplete; 1]`
// CHECK:STDERR: fail_incomplete_element.carbon:[[@LINE+6]]:8: error: variable has incomplete type `[Incomplete; 1]`
// CHECK:STDERR: var a: [Incomplete; 1];
// CHECK:STDERR: ^~~~~~~~~~~~~~~
// CHECK:STDERR: fail_incomplete_element.carbon:[[@LINE-5]]:1: note: class was forward declared here
Expand Down
2 changes: 1 addition & 1 deletion toolchain/check/testdata/class/cross_package_import.carbon
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ library "[[@TEST_NAME]]";

import Other library "other_extern";

// CHECK:STDERR: fail_extern.carbon:[[@LINE+8]]:8: error: Variable has incomplete type `C`
// CHECK:STDERR: fail_extern.carbon:[[@LINE+8]]:8: error: variable has incomplete type `C`
// CHECK:STDERR: var c: Other.C = {};
// CHECK:STDERR: ^~~~~~~
// CHECK:STDERR: fail_extern.carbon:[[@LINE-5]]:1: in import
Expand Down
Loading