From adf1a85ee905f2f068e6ca2f0b7fe2b1602d8dcd Mon Sep 17 00:00:00 2001 From: "Jeremy G. Siek" Date: Mon, 25 Apr 2022 16:25:22 -0400 Subject: [PATCH] Fix IsConcreteType for generic classes and add IsType predicate. (#1204) * check that the return type is a type, add more implicit conversions * update IsConcreteType for generic classes, add IsType predicate * delete a comment * trivial change * move Witness wrt IsConcreteType and IsType * added comments to tests * update error line numbers --- .../interpreter/type_checker.cpp | 114 ++++++++++++++++-- .../interpreter/type_checker.h | 5 + executable_semantics/interpreter/value.cpp | 4 +- .../basic_syntax/fail_var_type.carbon | 19 +++ .../function/fail_parameter_type.carbon | 21 ++++ .../fail_return_type_is_type.carbon | 27 +++++ 6 files changed, 174 insertions(+), 16 deletions(-) create mode 100644 executable_semantics/testdata/basic_syntax/fail_var_type.carbon create mode 100644 executable_semantics/testdata/function/fail_parameter_type.carbon create mode 100644 executable_semantics/testdata/generic_class/fail_return_type_is_type.carbon diff --git a/executable_semantics/interpreter/type_checker.cpp b/executable_semantics/interpreter/type_checker.cpp index 4f033bcb8da1..2b3cb31b9a79 100644 --- a/executable_semantics/interpreter/type_checker.cpp +++ b/executable_semantics/interpreter/type_checker.cpp @@ -61,8 +61,74 @@ static auto ExpectPointerType(SourceLocation source_loc, return Success(); } -// Returns whether *value represents a concrete type, as opposed to a -// type pattern or a non-type value. +// Returns whether the value is a valid result from a type expression, +// as opposed to a non-type value. +static auto IsType(Nonnull value) -> bool { + switch (value->kind()) { + case Value::Kind::IntValue: + case Value::Kind::FunctionValue: + case Value::Kind::BoundMethodValue: + case Value::Kind::PointerValue: + case Value::Kind::LValue: + case Value::Kind::BoolValue: + case Value::Kind::StructValue: + case Value::Kind::NominalClassValue: + case Value::Kind::AlternativeValue: + case Value::Kind::BindingPlaceholderValue: + case Value::Kind::AlternativeConstructorValue: + case Value::Kind::ContinuationValue: + case Value::Kind::StringValue: + case Value::Kind::Witness: + return false; + case Value::Kind::IntType: + case Value::Kind::BoolType: + case Value::Kind::TypeType: + case Value::Kind::FunctionType: + case Value::Kind::PointerType: + case Value::Kind::StructType: + case Value::Kind::InterfaceType: + case Value::Kind::ChoiceType: + case Value::Kind::ContinuationType: + case Value::Kind::VariableType: + case Value::Kind::StringType: + case Value::Kind::TypeOfClassType: + case Value::Kind::TypeOfInterfaceType: + case Value::Kind::TypeOfChoiceType: + case Value::Kind::StaticArrayType: + case Value::Kind::AutoType: + return true; + case Value::Kind::TupleValue: { + for (Nonnull field : cast(*value).elements()) { + if (!IsType(field)) { + return false; + } + } + return true; + } + case Value::Kind::NominalClassType: { + const NominalClassType& class_type = cast(*value); + // A NominalClassType is concrete if + // 1) it is not a generic class (has no type parameters), or + // 2) it is a generic class applied to some type arguments. + return !class_type.declaration().type_params().has_value() || + !class_type.type_args().empty(); + } + } +} + +auto TypeChecker::ExpectIsType(SourceLocation source_loc, + Nonnull value) + -> ErrorOr { + if (!IsType(value)) { + return CompilationError(source_loc) + << "Expected a type, but got " << *value; + } else { + return Success(); + } +} + +// Returns whether *value represents the type of a Carbon value, as +// opposed to a type pattern or a non-type value. static auto IsConcreteType(Nonnull value) -> bool { switch (value->kind()) { case Value::Kind::IntValue: @@ -78,6 +144,7 @@ static auto IsConcreteType(Nonnull value) -> bool { case Value::Kind::AlternativeConstructorValue: case Value::Kind::ContinuationValue: case Value::Kind::StringValue: + case Value::Kind::Witness: return false; case Value::Kind::IntType: case Value::Kind::BoolType: @@ -85,9 +152,7 @@ static auto IsConcreteType(Nonnull value) -> bool { case Value::Kind::FunctionType: case Value::Kind::PointerType: case Value::Kind::StructType: - case Value::Kind::NominalClassType: case Value::Kind::InterfaceType: - case Value::Kind::Witness: case Value::Kind::ChoiceType: case Value::Kind::ContinuationType: case Value::Kind::VariableType: @@ -100,13 +165,22 @@ static auto IsConcreteType(Nonnull value) -> bool { case Value::Kind::AutoType: // `auto` isn't a concrete type, it's a pattern that matches types. return false; - case Value::Kind::TupleValue: + case Value::Kind::TupleValue: { for (Nonnull field : cast(*value).elements()) { if (!IsConcreteType(field)) { return false; } } return true; + } + case Value::Kind::NominalClassType: { + const NominalClassType& class_type = cast(*value); + // A NominalClassType is concrete if + // 1) it is not a generic class (has no type parameters), or + // 2) it is a generic class applied to some type arguments. + return !class_type.declaration().type_params().has_value() || + !class_type.type_args().empty(); + } } } @@ -212,12 +286,28 @@ auto TypeChecker::IsImplicitlyConvertible( } return true; } + case Value::Kind::TypeType: { + for (Nonnull source_element : source_tuple.elements()) { + if (!IsImplicitlyConvertible(source_element, destination)) { + return false; + } + } + return true; + } default: return false; } } case Value::Kind::TypeType: return destination->kind() == Value::Kind::InterfaceType; + case Value::Kind::InterfaceType: + return destination->kind() == Value::Kind::TypeType; + case Value::Kind::TypeOfClassType: { + const auto& class_type = cast(*source).class_type(); + return ((!class_type.declaration().type_params().has_value()) || + (!class_type.type_args().empty())) && + destination->kind() == Value::Kind::TypeType; + } default: return false; } @@ -1017,7 +1107,7 @@ auto TypeChecker::TypeCheckExp(Nonnull e, Nonnull class_type = arena_->New(&class_decl, generic_args, impls); call.set_impls(impls); - call.set_static_type(class_type); + call.set_static_type(arena_->New(class_type)); call.set_value_category(ValueCategory::Let); return Success(); } @@ -1188,6 +1278,7 @@ auto TypeChecker::TypeCheckPattern( impl_scope, enclosing_value_category)); ASSIGN_OR_RETURN(Nonnull type, InterpPattern(&binding.type(), arena_, trace_stream_)); + RETURN_IF_ERROR(ExpectIsType(binding.source_loc(), type)); if (expected) { if (IsConcreteType(type)) { RETURN_IF_ERROR( @@ -1452,8 +1543,7 @@ auto TypeChecker::ExpectReturnOnAllPaths( if (!opt_stmt) { return CompilationError(source_loc) << "control-flow reaches end of function that provides a `->` " - "return " - "type without reaching a return statement"; + "return type without reaching a return statement"; } Nonnull stmt = *opt_stmt; switch (stmt->kind()) { @@ -1593,6 +1683,7 @@ auto TypeChecker::DeclareFunctionDeclaration(Nonnull f, // And shouldn't the type of this be Type? ASSIGN_OR_RETURN(Nonnull ret_type, InterpExp(*return_expression, arena_, trace_stream_)); + RETURN_IF_ERROR(ExpectIsType(f->source_loc(), ret_type)); f->return_term().set_static_type(ret_type); } else if (f->return_term().is_omitted()) { f->return_term().set_static_type(TupleValue::Empty()); @@ -1738,11 +1829,8 @@ auto TypeChecker::DeclareInterfaceDeclaration( iface_decl->set_static_type(arena_->New(iface_type)); // Process the Self parameter. - RETURN_IF_ERROR(TypeCheckExp(&iface_decl->self()->type(), enclosing_scope)); - iface_decl->self()->set_static_type( - arena_->New(iface_decl->self())); - iface_decl->self()->set_symbolic_identity(&iface_decl->self()->static_type()); - + RETURN_IF_ERROR(TypeCheckPattern(iface_decl->self(), std::nullopt, + enclosing_scope, ValueCategory::Let)); for (Nonnull m : iface_decl->members()) { RETURN_IF_ERROR(DeclareDeclaration(m, enclosing_scope)); } diff --git a/executable_semantics/interpreter/type_checker.h b/executable_semantics/interpreter/type_checker.h index 0442c4aaca87..115d0778d5cb 100644 --- a/executable_semantics/interpreter/type_checker.h +++ b/executable_semantics/interpreter/type_checker.h @@ -144,6 +144,11 @@ class TypeChecker { auto ExpectReturnOnAllPaths(std::optional> opt_stmt, SourceLocation source_loc) -> ErrorOr; + // Verifies that *value represents the result of a type expression, + // as opposed to a non-type value. + auto ExpectIsType(SourceLocation source_loc, Nonnull value) + -> ErrorOr; + // Verifies that *value represents a concrete type, as opposed to a // type pattern or a non-type value. auto ExpectIsConcreteType(SourceLocation source_loc, diff --git a/executable_semantics/interpreter/value.cpp b/executable_semantics/interpreter/value.cpp index 20297357196e..f1fb99e5ff3c 100644 --- a/executable_semantics/interpreter/value.cpp +++ b/executable_semantics/interpreter/value.cpp @@ -361,9 +361,7 @@ void Value::Print(llvm::raw_ostream& out) const { out << "\""; break; case Value::Kind::TypeOfClassType: - out << "typeof(" - << cast(*this).class_type().declaration().name() - << ")"; + out << "typeof(" << cast(*this).class_type() << ")"; break; case Value::Kind::TypeOfInterfaceType: out << "typeof(" diff --git a/executable_semantics/testdata/basic_syntax/fail_var_type.carbon b/executable_semantics/testdata/basic_syntax/fail_var_type.carbon new file mode 100644 index 000000000000..4ed796330de6 --- /dev/null +++ b/executable_semantics/testdata/basic_syntax/fail_var_type.carbon @@ -0,0 +1,19 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// RUN: %{not} %{executable_semantics} %s 2>&1 | \ +// RUN: %{FileCheck} --match-full-lines --allow-unused-prefixes=false %s +// RUN: %{not} %{executable_semantics} --parser_debug --trace_file=- %s 2>&1 | \ +// RUN: %{FileCheck} --match-full-lines --allow-unused-prefixes %s +// AUTOUPDATE: %{executable_semantics} %s +// CHECK: COMPILATION ERROR: {{.*}}/executable_semantics/testdata/basic_syntax/fail_var_type.carbon:17: Expected a type, but got 42 + +package ExecutableSemanticsTest api; + +fn Main () -> i32 +{ + // 42 cannot be used as the type of a variable. + var x: 42 = 0; + return x; +} diff --git a/executable_semantics/testdata/function/fail_parameter_type.carbon b/executable_semantics/testdata/function/fail_parameter_type.carbon new file mode 100644 index 000000000000..8dbb785c8877 --- /dev/null +++ b/executable_semantics/testdata/function/fail_parameter_type.carbon @@ -0,0 +1,21 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// RUN: %{not} %{executable_semantics} %s 2>&1 | \ +// RUN: %{FileCheck} --match-full-lines --allow-unused-prefixes=false %s +// RUN: %{not} %{executable_semantics} --parser_debug --trace_file=- %s 2>&1 | \ +// RUN: %{FileCheck} --match-full-lines --allow-unused-prefixes %s +// AUTOUPDATE: %{executable_semantics} %s +// CHECK: COMPILATION ERROR: {{.*}}/executable_semantics/testdata/function/fail_parameter_type.carbon:15: Expected a type, but got 42 + +package ExecutableSemanticsTest api; + +// 42 cannot be used as the type of a parameter. +fn f(x: 42) -> i32 { + return x - 1; +} + +fn Main() -> i32 { + return f(1); +} diff --git a/executable_semantics/testdata/generic_class/fail_return_type_is_type.carbon b/executable_semantics/testdata/generic_class/fail_return_type_is_type.carbon new file mode 100644 index 000000000000..8563195cdff8 --- /dev/null +++ b/executable_semantics/testdata/generic_class/fail_return_type_is_type.carbon @@ -0,0 +1,27 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// RUN: %{not} %{executable_semantics} %s 2>&1 | \ +// RUN: %{FileCheck} --match-full-lines --allow-unused-prefixes=false %s +// RUN: %{not} %{executable_semantics} --parser_debug --trace_file=- %s 2>&1 | \ +// RUN: %{FileCheck} --match-full-lines --allow-unused-prefixes %s +// AUTOUPDATE: %{executable_semantics} %s +// CHECK: COMPILATION ERROR: {{.*}}/executable_semantics/testdata/generic_class/fail_return_type_is_type.carbon:18: Expected a type, but got class Point + +package ExecutableSemanticsTest api; + +class Point(T:! Type) { + // The return type should be Point(T). Point by itself is not a type. + fn Create(x: T, y: T) -> Point { + return {.x = x, .y = y}; + } + + var x: T; + var y: T; +} + +fn Main() -> i32 { + var p: Point(i32) = Point(i32).Create(0, 1); + return p.x; +}