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

Reject abstract types in var function parameters (#6499)

Browse Source 
### Description
Fixes an issue where the toolchain accepted abstract types in function
parameters declared with `var`.

### Changes
- Implemented a check for abstract types for function parameters with
`var` binding pattern in `HandleAnyBindingPattern`.
- Added a test case to
`toolchain/check/testdata/class/fail_abstract.carbon`.

**Note:** I did not use `AsConcreteType` like used in `case
FullPatternStack::Kind::NameBindingDecl`. Using it enforces type
completion, thus causing valid signatures such as `fn F[var self: Self]`
to fail.

Also the pre-commit checks fail due to a diagnostic name collision with
`toolchain/check/type_completion.cpp`. Should I add a function that just
checks if the type is abstract to share the diagnostic?

Fixes #6402
This commit is contained in:
Özgür
2025-12-15 20:17:34 +00:00
committed by GitHub
parent 25f63140e6
commit 2a3d0b71bb
5 changed files with 157 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files
toolchain/check/handle_binding_pattern.cpp
+34 -10
View File
@@ -158,6 +158,15 @@ static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id,
return binding.pattern_id;
};
auto abstract_diagnoser = [&] {
CARBON_DIAGNOSTIC(AbstractTypeInVarPattern, Error,
"binding pattern has abstract type {0} in `var` "
"pattern",
SemIR::TypeId);
return context.emitter().Build(type_node, AbstractTypeInVarPattern,
cast_type_id);
};
// A `self` binding can only appear in an implicit parameter list.
if (name_id == SemIR::NameId::SelfValue &&
!context.node_stack().PeekIs(Parse::NodeKind::ImplicitParamListStart)) {
@@ -192,6 +201,29 @@ static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id,
break;
}
// Using `AsConcreteType` here causes `fn F[var self: Self]();`
// to fail since `Self` is an incomplete type.
if (node_kind == Parse::NodeKind::VarBindingPattern) {
auto [unqualified_type_id, qualifiers] =
context.types().GetUnqualifiedTypeAndQualifiers(cast_type_id);
if ((qualifiers & SemIR::TypeQualifiers::Partial) !=
SemIR::TypeQualifiers::Partial &&
context.types().Is<SemIR::ClassType>(unqualified_type_id)) {
auto class_type =
context.types().GetAs<SemIR::ClassType>(unqualified_type_id);
auto& class_info = context.classes().Get(class_type.class_id);
if (class_info.inheritance_kind ==
SemIR::Class::InheritanceKind::Abstract) {
auto builder = abstract_diagnoser();
auto direct_use = true;
NoteAbstractClass(context, class_type.class_id, direct_use,
builder);
builder.Emit();
cast_type_id = SemIR::ErrorInst::TypeId;
}
}
}
auto result_inst_id = make_binding_pattern();
// A binding pattern in a function signature is a `Call` parameter
@@ -228,16 +260,8 @@ static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id,
cast_type_inst_id);
};
if (node_kind == Parse::NodeKind::VarBindingPattern) {
cast_type_id = AsConcreteType(
context, cast_type_id, type_node, incomplete_diagnoser, [&] {
CARBON_DIAGNOSTIC(
AbstractTypeInVarPattern, Error,
"binding pattern has abstract type {0} in `var` "
"pattern",
SemIR::TypeId);
return context.emitter().Build(
type_node, AbstractTypeInVarPattern, cast_type_id);
});
cast_type_id = AsConcreteType(context, cast_type_id, type_node,
incomplete_diagnoser, abstract_diagnoser);
} else {
cast_type_id = AsCompleteType(context, cast_type_id, type_node,
incomplete_diagnoser);
toolchain/check/testdata/class/fail_abstract.carbon
Vendored
+66
View File
@@ -48,6 +48,23 @@ fn Var() {
var v: Abstract;
}
// --- fail_abstract_var_function_param.carbon
library "[[@TEST_NAME]]";
abstract class Abstract {
}
// CHECK:STDERR: fail_abstract_var_function_param.carbon:[[@LINE+7]]:13: error: binding pattern has abstract type `Abstract` in `var` pattern [AbstractTypeInVarPattern]
// CHECK:STDERR: fn F(var p: Abstract) {
// CHECK:STDERR: ^~~~~~~~
// CHECK:STDERR: fail_abstract_var_function_param.carbon:[[@LINE-6]]:1: note: class was declared abstract here [ClassAbstractHere]
// CHECK:STDERR: abstract class Abstract {
// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~
// CHECK:STDERR:
fn F(var p: Abstract) {
}
// --- abstract_let.carbon
library "[[@TEST_NAME]]";
@@ -291,6 +308,55 @@ fn CallReturnAbstract() {
// CHECK:STDOUT: return
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: --- fail_abstract_var_function_param.carbon
// CHECK:STDOUT:
// CHECK:STDOUT: constants {
// CHECK:STDOUT: %Abstract: type = class_type @Abstract [concrete]
// CHECK:STDOUT: %empty_struct_type: type = struct_type {} [concrete]
// CHECK:STDOUT: %complete_type: <witness> = complete_type_witness %empty_struct_type [concrete]
// CHECK:STDOUT: %F.type: type = fn_type @F [concrete]
// CHECK:STDOUT: %F: %F.type = struct_value () [concrete]
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: imports {
// CHECK:STDOUT: %Core: <namespace> = namespace file.%Core.import, [concrete] {
// CHECK:STDOUT: import Core//prelude
// CHECK:STDOUT: import Core//prelude/...
// CHECK:STDOUT: }
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: file {
// CHECK:STDOUT: package: <namespace> = namespace [concrete] {
// CHECK:STDOUT: .Core = imports.%Core
// CHECK:STDOUT: .Abstract = %Abstract.decl
// CHECK:STDOUT: .F = %F.decl
// CHECK:STDOUT: }
// CHECK:STDOUT: %Core.import = import Core
// CHECK:STDOUT: %Abstract.decl: type = class_decl @Abstract [concrete = constants.%Abstract] {} {}
// CHECK:STDOUT: %F.decl: %F.type = fn_decl @F [concrete = constants.%F] {
// CHECK:STDOUT: %p.patt: <error> = ref_binding_pattern p [concrete]
// CHECK:STDOUT: %p.param_patt: <error> = var_param_pattern %p.patt, call_param0 [concrete]
// CHECK:STDOUT: %p.var_patt: <error> = var_pattern %p.param_patt [concrete]
// CHECK:STDOUT: } {
// CHECK:STDOUT: %p.param: ref <error> = ref_param call_param0
// CHECK:STDOUT: %Abstract.ref: type = name_ref Abstract, file.%Abstract.decl [concrete = constants.%Abstract]
// CHECK:STDOUT: %p: ref <error> = ref_binding p, %p.param
// CHECK:STDOUT: }
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: class @Abstract {
// CHECK:STDOUT: %complete_type: <witness> = complete_type_witness constants.%empty_struct_type [concrete = constants.%complete_type]
// CHECK:STDOUT: complete_type_witness = %complete_type
// CHECK:STDOUT:
// CHECK:STDOUT: !members:
// CHECK:STDOUT: .Self = constants.%Abstract
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: fn @F(%p.param: <error>) {
// CHECK:STDOUT: !entry:
// CHECK:STDOUT: return
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: --- abstract_let.carbon
// CHECK:STDOUT:
// CHECK:STDOUT: constants {
toolchain/check/testdata/class/partial.carbon
Vendored
+39
View File
@@ -28,6 +28,15 @@ abstract class C { }
fn A(p: partial C);
//@dump-sem-ir-end
// --- abstract_var.carbon
library "[[@TEST_NAME]]";
abstract class C { }
//@dump-sem-ir-begin
fn A(var p: partial C);
//@dump-sem-ir-end
// --- fail_partial_nondynamic.carbon
library "[[@TEST_NAME]]";
@@ -224,6 +233,36 @@ fn F[T:! type](p: partial T*);
// CHECK:STDOUT:
// CHECK:STDOUT: fn @A(%p.param: %.43f);
// CHECK:STDOUT:
// CHECK:STDOUT: --- abstract_var.carbon
// CHECK:STDOUT:
// CHECK:STDOUT: constants {
// CHECK:STDOUT: %C: type = class_type @C [concrete]
// CHECK:STDOUT: %.43f: type = partial_type %C [concrete]
// CHECK:STDOUT: %pattern_type: type = pattern_type %.43f [concrete]
// CHECK:STDOUT: %A.type: type = fn_type @A [concrete]
// CHECK:STDOUT: %A: %A.type = struct_value () [concrete]
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: imports {
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: file {
// CHECK:STDOUT: %A.decl: %A.type = fn_decl @A [concrete = constants.%A] {
// CHECK:STDOUT: %p.patt: %pattern_type = ref_binding_pattern p [concrete]
// CHECK:STDOUT: %p.param_patt: %pattern_type = var_param_pattern %p.patt, call_param0 [concrete]
// CHECK:STDOUT: %p.var_patt: %pattern_type = var_pattern %p.param_patt [concrete]
// CHECK:STDOUT: } {
// CHECK:STDOUT: %p.param: ref %.43f = ref_param call_param0
// CHECK:STDOUT: %.loc6_13.1: type = splice_block %.loc6_13.2 [concrete = constants.%.43f] {
// CHECK:STDOUT: %C.ref: type = name_ref C, file.%C.decl [concrete = constants.%C]
// CHECK:STDOUT: %.loc6_13.2: type = partial_type %C.ref [concrete = constants.%.43f]
// CHECK:STDOUT: }
// CHECK:STDOUT: %p: ref %.43f = ref_binding p, %p.param
// CHECK:STDOUT: }
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: fn @A(%p.param: %.43f);
// CHECK:STDOUT:
// CHECK:STDOUT: --- fail_partial_nondynamic.carbon
// CHECK:STDOUT:
// CHECK:STDOUT: constants {
toolchain/check/type_completion.cpp
+14 -16
View File
@@ -54,6 +54,20 @@ auto NoteIncompleteInterface(Context& context, SemIR::InterfaceId interface_id,
}
}
auto NoteAbstractClass(Context& context, SemIR::ClassId class_id,
bool direct_use, DiagnosticBuilder& builder) -> void {
const auto& class_info = context.classes().Get(class_id);
CARBON_CHECK(
class_info.inheritance_kind == SemIR::Class::InheritanceKind::Abstract,
"Class is not abstract");
CARBON_DIAGNOSTIC(
ClassAbstractHere, Note,
"{0:=0:uses class that|=1:class} was declared abstract here",
Diagnostics::IntAsSelect);
builder.Note(class_info.definition_id, ClassAbstractHere,
static_cast<int>(direct_use));
}
static auto NoteIncompleteNamedConstraint(
Context& context, SemIR::NamedConstraintId named_constraint_id,
DiagnosticBuilder& builder) -> void {
@@ -728,22 +742,6 @@ auto RequireCompleteType(Context& context, SemIR::TypeId type_id,
return true;
}
// Adds a note to a diagnostic explaining that a class is abstract.
static auto NoteAbstractClass(Context& context, SemIR::ClassId class_id,
bool direct_use, DiagnosticBuilder& builder)
-> void {
const auto& class_info = context.classes().Get(class_id);
CARBON_CHECK(
class_info.inheritance_kind == SemIR::Class::InheritanceKind::Abstract,
"Class is not abstract");
CARBON_DIAGNOSTIC(
ClassAbstractHere, Note,
"{0:=0:uses class that|=1:class} was declared abstract here",
Diagnostics::IntAsSelect);
builder.Note(class_info.definition_id, ClassAbstractHere,
static_cast<int>(direct_use));
}
auto RequireConcreteType(Context& context, SemIR::TypeId type_id,
SemIR::LocId loc_id, MakeDiagnosticBuilderFn diagnoser,
MakeDiagnosticBuilderFn abstract_diagnoser) -> bool {
toolchain/check/type_completion.h
+4
View File
@@ -85,6 +85,10 @@ auto NoteIncompleteClass(Context& context, SemIR::ClassId class_id,
auto NoteIncompleteInterface(Context& context, SemIR::InterfaceId interface_id,
DiagnosticBuilder& builder) -> void;
// Adds a note to a diagnostic explaining that a class is abstract.
auto NoteAbstractClass(Context& context, SemIR::ClassId class_id,
bool direct_use, DiagnosticBuilder& builder) -> void;
} // namespace Carbon::Check
#endif // CARBON_TOOLCHAIN_CHECK_TYPE_COMPLETION_H_