From f0a34e4bf785562fcb878890967beeb0c0629741 Mon Sep 17 00:00:00 2001 From: pk19604014 <95385881+pk19604014@users.noreply.github.com> Date: Mon, 23 May 2022 19:26:35 -0400 Subject: [PATCH] Added TypeCheckTypeExp() and changed function type literal and similar type-checking logic to use the new function (#1256) --- ...5ff569c91b7cbebbb6160b07de5b2ab1.textproto | 20 ++++++ explorer/interpreter/type_checker.cpp | 72 ++++++++----------- explorer/interpreter/type_checker.h | 6 ++ .../fail_alternative_not_type.carbon | 18 +++++ .../fail_return_type_is_type.carbon | 2 +- .../generic_function/fail_not_type.carbon | 18 +++++ .../impl/fail_impl_as_parameterized.carbon | 7 +- .../interface/fail_impl_not_type.carbon | 23 ++++++ 8 files changed, 117 insertions(+), 49 deletions(-) create mode 100644 explorer/fuzzing/fuzzer_corpus/512e48e65ff569c91b7cbebbb6160b07de5b2ab1.textproto create mode 100644 explorer/testdata/basic_syntax/fail_alternative_not_type.carbon create mode 100644 explorer/testdata/generic_function/fail_not_type.carbon create mode 100644 explorer/testdata/interface/fail_impl_not_type.carbon diff --git a/explorer/fuzzing/fuzzer_corpus/512e48e65ff569c91b7cbebbb6160b07de5b2ab1.textproto b/explorer/fuzzing/fuzzer_corpus/512e48e65ff569c91b7cbebbb6160b07de5b2ab1.textproto new file mode 100644 index 000000000000..3ec1b067821e --- /dev/null +++ b/explorer/fuzzing/fuzzer_corpus/512e48e65ff569c91b7cbebbb6160b07de5b2ab1.textproto @@ -0,0 +1,20 @@ +compilation_unit { + declarations { + function { + body { + statements { + match { + expression { + function_type { + parameter { + primitive_operator { + } + } + } + } + } + } + } + } + } +} diff --git a/explorer/interpreter/type_checker.cpp b/explorer/interpreter/type_checker.cpp index 271e47447a25..b942128db8ef 100644 --- a/explorer/interpreter/type_checker.cpp +++ b/explorer/interpreter/type_checker.cpp @@ -10,6 +10,7 @@ #include #include +#include "common/error.h" #include "common/ostream.h" #include "explorer/ast/declaration.h" #include "explorer/common/arena.h" @@ -1112,11 +1113,7 @@ auto TypeChecker::TypeCheckExp(Nonnull e, case ExpressionKind::StructTypeLiteral: { auto& struct_type = cast(*e); for (auto& arg : struct_type.fields()) { - CARBON_RETURN_IF_ERROR(TypeCheckExp(&arg.expression(), impl_scope)); - CARBON_ASSIGN_OR_RETURN( - auto value, InterpExp(&arg.expression(), arena_, trace_stream_)); - CARBON_RETURN_IF_ERROR( - ExpectIsConcreteType(arg.expression().source_loc(), value)); + CARBON_RETURN_IF_ERROR(TypeCheckTypeExp(&arg.expression(), impl_scope)); } if (struct_type.fields().empty()) { // `{}` is the type of `{}`, just as `()` is the type of `()`. @@ -1663,16 +1660,8 @@ auto TypeChecker::TypeCheckExp(Nonnull e, } case ExpressionKind::FunctionTypeLiteral: { auto& fn = cast(*e); - CARBON_ASSIGN_OR_RETURN( - Nonnull param_type, - InterpExp(&fn.parameter(), arena_, trace_stream_)); - CARBON_RETURN_IF_ERROR( - ExpectIsConcreteType(fn.parameter().source_loc(), param_type)); - CARBON_ASSIGN_OR_RETURN( - Nonnull ret_type, - InterpExp(&fn.return_type(), arena_, trace_stream_)); - CARBON_RETURN_IF_ERROR( - ExpectIsConcreteType(fn.return_type().source_loc(), ret_type)); + CARBON_RETURN_IF_ERROR(TypeCheckTypeExp(&fn.parameter(), impl_scope)); + CARBON_RETURN_IF_ERROR(TypeCheckTypeExp(&fn.return_type(), impl_scope)); fn.set_static_type(arena_->New()); fn.set_value_category(ValueCategory::Let); return Success(); @@ -1733,14 +1722,8 @@ auto TypeChecker::TypeCheckExp(Nonnull e, CARBON_FATAL() << "Unimplemented: " << *e; case ExpressionKind::ArrayTypeLiteral: { auto& array_literal = cast(*e); - CARBON_RETURN_IF_ERROR( - TypeCheckExp(&array_literal.element_type_expression(), impl_scope)); - CARBON_ASSIGN_OR_RETURN( - Nonnull element_type, - InterpExp(&array_literal.element_type_expression(), arena_, - trace_stream_)); - CARBON_RETURN_IF_ERROR(ExpectIsConcreteType( - array_literal.element_type_expression().source_loc(), element_type)); + CARBON_RETURN_IF_ERROR(TypeCheckTypeExp( + &array_literal.element_type_expression(), impl_scope)); CARBON_RETURN_IF_ERROR( TypeCheckExp(&array_literal.size_expression(), impl_scope)); @@ -1817,6 +1800,18 @@ void TypeChecker::BringImplIntoScope(Nonnull impl_binding, CreateImplReference(impl_binding)); } +auto TypeChecker::TypeCheckTypeExp(Nonnull type_expression, + const ImplScope& impl_scope, bool concrete) + -> ErrorOr> { + CARBON_RETURN_IF_ERROR(TypeCheckExp(type_expression, impl_scope)); + CARBON_ASSIGN_OR_RETURN(Nonnull type, + InterpExp(type_expression, arena_, trace_stream_)); + CARBON_RETURN_IF_ERROR( + concrete ? ExpectIsConcreteType(type_expression->source_loc(), type) + : ExpectIsType(type_expression->source_loc(), type)); + return type; +} + auto TypeChecker::TypeCheckPattern( Nonnull p, std::optional> expected, ImplScope& impl_scope, ValueCategory enclosing_value_category) @@ -1877,10 +1872,8 @@ auto TypeChecker::TypeCheckPattern( } case PatternKind::GenericBinding: { auto& binding = cast(*p); - CARBON_RETURN_IF_ERROR(TypeCheckExp(&binding.type(), impl_scope)); - CARBON_ASSIGN_OR_RETURN( - Nonnull type, - InterpExp(&binding.type(), arena_, trace_stream_)); + CARBON_ASSIGN_OR_RETURN(Nonnull type, + TypeCheckTypeExp(&binding.type(), impl_scope)); if (expected) { return CompilationError(binding.type().source_loc()) << "Generic binding may not occur in pattern with expected " @@ -2274,13 +2267,11 @@ auto TypeChecker::DeclareFunctionDeclaration(Nonnull f, return_expression.has_value()) { // We ignore the return value because return type expressions can't bring // new types into scope. - CARBON_RETURN_IF_ERROR(TypeCheckExp(*return_expression, function_scope)); // Should we be doing SetConstantValue instead? -Jeremy // And shouldn't the type of this be Type? - CARBON_ASSIGN_OR_RETURN( - Nonnull ret_type, - InterpExp(*return_expression, arena_, trace_stream_)); - CARBON_RETURN_IF_ERROR(ExpectIsType(f->source_loc(), ret_type)); + CARBON_ASSIGN_OR_RETURN(Nonnull ret_type, + TypeCheckTypeExp(*return_expression, function_scope, + /*concrete=*/false)); f->return_term().set_static_type(ret_type); } else if (f->return_term().is_omitted()) { f->return_term().set_static_type(TupleValue::Empty()); @@ -2524,10 +2515,8 @@ auto TypeChecker::DeclareImplDeclaration(Nonnull impl_decl, impl_decl->set_impl_bindings(impl_bindings); // Check and interpret the impl_type - CARBON_RETURN_IF_ERROR(TypeCheckExp(impl_decl->impl_type(), impl_scope)); - CARBON_ASSIGN_OR_RETURN( - Nonnull impl_type_value, - InterpExp(impl_decl->impl_type(), arena_, trace_stream_)); + CARBON_ASSIGN_OR_RETURN(Nonnull impl_type_value, + TypeCheckTypeExp(impl_decl->impl_type(), impl_scope)); // Set `Self` to `impl_type`. We do this whether `Self` resolves to it or to // the `Self` from an enclosing scope. This needs to be done before @@ -2538,11 +2527,9 @@ auto TypeChecker::DeclareImplDeclaration(Nonnull impl_decl, self->set_static_type(&impl_decl->impl_type()->static_type()); // Check and interpret the interface. - CARBON_RETURN_IF_ERROR(TypeCheckExp(&impl_decl->interface(), impl_scope)); CARBON_ASSIGN_OR_RETURN( Nonnull written_iface_type, - InterpExp(&impl_decl->interface(), arena_, trace_stream_)); - + TypeCheckTypeExp(&impl_decl->interface(), impl_scope)); const auto* iface_type = dyn_cast(written_iface_type); if (!iface_type) { return CompilationError(impl_decl->interface().source_loc()) @@ -2630,10 +2617,9 @@ auto TypeChecker::DeclareChoiceDeclaration(Nonnull choice, -> ErrorOr { std::vector alternatives; for (Nonnull alternative : choice->alternatives()) { - CARBON_RETURN_IF_ERROR( - TypeCheckExp(&alternative->signature(), *scope_info.innermost_scope)); - CARBON_ASSIGN_OR_RETURN(auto signature, InterpExp(&alternative->signature(), - arena_, trace_stream_)); + CARBON_ASSIGN_OR_RETURN(auto signature, + TypeCheckTypeExp(&alternative->signature(), + *scope_info.innermost_scope)); alternatives.push_back({.name = alternative->name(), .value = signature}); } auto ct = arena_->New(choice->name(), std::move(alternatives)); diff --git a/explorer/interpreter/type_checker.h b/explorer/interpreter/type_checker.h index e96493de0883..a016da7dc544 100644 --- a/explorer/interpreter/type_checker.h +++ b/explorer/interpreter/type_checker.h @@ -93,6 +93,12 @@ class TypeChecker { auto TypeCheckExp(Nonnull e, const ImplScope& impl_scope) -> ErrorOr; + // Type checks and interprets `type_expression`, and validates it represents a + // [concrete] type. + auto TypeCheckTypeExp(Nonnull type_expression, + const ImplScope& impl_scope, bool concrete = true) + -> ErrorOr>; + // Equivalent to TypeCheckExp, but operates on the AST rooted at `p`. // // `expected` is the type that this pattern is expected to have, if the diff --git a/explorer/testdata/basic_syntax/fail_alternative_not_type.carbon b/explorer/testdata/basic_syntax/fail_alternative_not_type.carbon new file mode 100644 index 000000000000..c08e81614ed0 --- /dev/null +++ b/explorer/testdata/basic_syntax/fail_alternative_not_type.carbon @@ -0,0 +1,18 @@ +// 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} %{explorer} %s 2>&1 | \ +// RUN: %{FileCheck} --match-full-lines --allow-unused-prefixes=false %s +// RUN: %{not} %{explorer} --parser_debug --trace_file=- %s 2>&1 | \ +// RUN: %{FileCheck} --match-full-lines --allow-unused-prefixes %s +// AUTOUPDATE: %{explorer} %s + +package ExplorerTest api; + +// CHECK: COMPILATION ERROR: {{.*}}/explorer/testdata/basic_syntax/fail_alternative_not_type.carbon:[[@LINE+1]]: Expected a type, but got (42) +choice C { X(42) } + +fn Main() -> i32 { + return 0; +} diff --git a/explorer/testdata/generic_class/fail_return_type_is_type.carbon b/explorer/testdata/generic_class/fail_return_type_is_type.carbon index 7232ddcda347..2f757dd2ea2c 100644 --- a/explorer/testdata/generic_class/fail_return_type_is_type.carbon +++ b/explorer/testdata/generic_class/fail_return_type_is_type.carbon @@ -12,9 +12,9 @@ package ExplorerTest api; class Point(T:! Type) { // The return type should be Point(T). Point by itself is not a type. + // CHECK: COMPILATION ERROR: {{.*}}/explorer/testdata/generic_class/fail_return_type_is_type.carbon:[[@LINE+1]]: Expected a type, but got Point fn Create(x: T, y: T) -> Point { return {.x = x, .y = y}; - // CHECK: COMPILATION ERROR: {{.*}}/explorer/testdata/generic_class/fail_return_type_is_type.carbon:[[@LINE+1]]: Expected a type, but got Point } var x: T; diff --git a/explorer/testdata/generic_function/fail_not_type.carbon b/explorer/testdata/generic_function/fail_not_type.carbon new file mode 100644 index 000000000000..c8058432f737 --- /dev/null +++ b/explorer/testdata/generic_function/fail_not_type.carbon @@ -0,0 +1,18 @@ +// 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} %{explorer} %s 2>&1 | \ +// RUN: %{FileCheck} --match-full-lines --allow-unused-prefixes=false %s +// RUN: %{not} %{explorer} --parser_debug --trace_file=- %s 2>&1 | \ +// RUN: %{FileCheck} --match-full-lines --allow-unused-prefixes %s +// AUTOUPDATE: %{explorer} %s + +package ExplorerTest api; + +// CHECK: COMPILATION ERROR: {{.*}}/explorer/testdata/generic_function/fail_not_type.carbon:[[@LINE+1]]: Expected a type, but got 42 +fn F[a:! 42](); + +fn Main() -> i32 { + return 0; +} diff --git a/explorer/testdata/impl/fail_impl_as_parameterized.carbon b/explorer/testdata/impl/fail_impl_as_parameterized.carbon index b799e8e43c2c..88a439d0ccb8 100644 --- a/explorer/testdata/impl/fail_impl_as_parameterized.carbon +++ b/explorer/testdata/impl/fail_impl_as_parameterized.carbon @@ -10,11 +10,8 @@ package ExplorerTest api; -interface Vector(Scalar:! Type) { -} - -// CHECK: COMPILATION ERROR: {{.*}}/explorer/testdata/impl/fail_impl_as_parameterized.carbon:[[@LINE+1]]: expected constraint after `as`, found value of type Vector -external impl i32 as Vector {} +// CHECK: COMPILATION ERROR: {{.*}}/explorer/testdata/impl/fail_impl_as_parameterized.carbon:[[@LINE+1]]: expected constraint after `as`, found value of type String +external impl i32 as String {} fn Main() -> i32 { } diff --git a/explorer/testdata/interface/fail_impl_not_type.carbon b/explorer/testdata/interface/fail_impl_not_type.carbon new file mode 100644 index 000000000000..8f766098f71b --- /dev/null +++ b/explorer/testdata/interface/fail_impl_not_type.carbon @@ -0,0 +1,23 @@ +// 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} %{explorer} %s 2>&1 | \ +// RUN: %{FileCheck} --match-full-lines --allow-unused-prefixes=false %s +// RUN: %{not} %{explorer} --parser_debug --trace_file=- %s 2>&1 | \ +// RUN: %{FileCheck} --match-full-lines --allow-unused-prefixes %s +// AUTOUPDATE: %{explorer} %s + +package ExplorerTest api; + +interface Vector { + fn Zero() -> i32; +} +// CHECK: COMPILATION ERROR: {{.*}}/explorer/testdata/interface/fail_impl_not_type.carbon:[[@LINE+1]]: Expected a type, but got "hello" +impl "hello" as Vector { + fn Zero() -> i32 { return 0; } +} + +fn Main() -> i32 { + return 0; +}