From bc5a42211bae93638042b074283cddafdaa0e3b5 Mon Sep 17 00:00:00 2001 From: Geoff Romer Date: Tue, 12 Oct 2021 10:27:20 -0700 Subject: [PATCH] Support struct implicit conversions in type-checking (#870) I should emphasize that I am **completely cheating** here. This PR does not add support for actually _performing_ implicit conversions at run time, because the AST doesn't yet contain the necessary type information. At run time, code like `var p: Point = {.x = 1, .y = 2};` directly initializes the name `p` with the _struct_ value `{.x = 1, .y = 2}`; no object of type `Point` is actually created. I'm only getting away with this because we don't yet have any tests that can tell the difference. Co-authored-by: Jon Meow <46229924+jonmeow@users.noreply.github.com> --- .../interpreter/type_checker.cpp | 167 ++++++++++++++---- .../testdata/class/assign.carbon | 24 +++ .../testdata/class/assign_member.carbon | 2 +- .../class/fail_field_access_mismatch.carbon | 5 +- .../testdata/class/fail_field_mismatch.carbon | 7 +- .../testdata/class/fail_field_missing.carbon | 7 +- .../testdata/class/function_param.carbon | 25 +++ .../testdata/class/global_var.carbon | 23 +++ .../testdata/class/temp.carbon | 6 +- .../testdata/class/var.carbon | 2 +- .../function/fail_call_with_tuple.carbon | 4 +- .../fail_init_type_mismatch.carbon | 4 +- ...il_name_order.carbon => name_order.carbon} | 8 +- .../testdata/tuple/fail_name_order.carbon | 2 +- 14 files changed, 226 insertions(+), 60 deletions(-) create mode 100644 executable_semantics/testdata/class/assign.carbon create mode 100644 executable_semantics/testdata/class/function_param.carbon create mode 100644 executable_semantics/testdata/class/global_var.carbon rename executable_semantics/testdata/struct/{fail_name_order.carbon => name_order.carbon} (59%) diff --git a/executable_semantics/interpreter/type_checker.cpp b/executable_semantics/interpreter/type_checker.cpp index 736ace435df4..8ce8e90aa8ff 100644 --- a/executable_semantics/interpreter/type_checker.cpp +++ b/executable_semantics/interpreter/type_checker.cpp @@ -40,9 +40,10 @@ void PrintTypeEnv(TypeEnv types, llvm::raw_ostream& out) { } } -static void ExpectType(SourceLocation source_loc, const std::string& context, - Nonnull expected, - Nonnull actual) { +static void ExpectExactType(SourceLocation source_loc, + const std::string& context, + Nonnull expected, + Nonnull actual) { if (!TypeEqual(expected, actual)) { FATAL_COMPILATION_ERROR(source_loc) << "type error in " << context << "\n" << "expected: " << *expected << "\n" @@ -109,6 +110,90 @@ void TypeChecker::ExpectIsConcreteType(SourceLocation source_loc, } } +// Returns true if *source is implicitly convertible to *destination. *source +// and *destination must be concrete types. +static auto IsImplicitlyConvertible(Nonnull source, + Nonnull destination) -> bool; + +// Returns true if source_fields and destination_fields contain the same set +// of names, and each value in source_fields is implicitly convertible to +// the corresponding value in destination_fields. All values in both arguments +// must be types. +static auto FieldTypesImplicitlyConvertible( + const VarValues& source_fields, const VarValues& destination_fields) { + if (source_fields.size() != destination_fields.size()) { + return false; + } + for (const auto& [field_name, source_field_type] : source_fields) { + std::optional> destination_field_type = + FindInVarValues(field_name, destination_fields); + if (!destination_field_type.has_value() || + !IsImplicitlyConvertible(source_field_type, *destination_field_type)) { + return false; + } + } + return true; +} + +static auto IsImplicitlyConvertible(Nonnull source, + Nonnull destination) -> bool { + CHECK(IsConcreteType(source)); + CHECK(IsConcreteType(destination)); + if (TypeEqual(source, destination)) { + return true; + } + switch (source->kind()) { + case Value::Kind::StructType: + switch (destination->kind()) { + case Value::Kind::StructType: + return FieldTypesImplicitlyConvertible( + cast(*source).fields(), + cast(*destination).fields()); + case Value::Kind::NominalClassType: + return FieldTypesImplicitlyConvertible( + cast(*source).fields(), + cast(*destination).Fields()); + default: + return false; + } + case Value::Kind::TupleValue: + switch (destination->kind()) { + case Value::Kind::TupleValue: { + const std::vector& source_elements = + cast(*source).Elements(); + const std::vector& destination_elements = + cast(*destination).Elements(); + if (source_elements.size() != destination_elements.size()) { + return false; + } + for (size_t i = 0; i < source_elements.size(); ++i) { + if (source_elements[i].name != destination_elements[i].name || + !IsImplicitlyConvertible(source_elements[i].value, + destination_elements[i].value)) { + return false; + } + } + return true; + } + default: + return false; + } + default: + return false; + } +} + +static void ExpectType(SourceLocation source_loc, const std::string& context, + Nonnull expected, + Nonnull actual) { + if (!IsImplicitlyConvertible(actual, expected)) { + FATAL_COMPILATION_ERROR(source_loc) + << "type error in " << context << ": " + << "'" << *actual << "' is not implicitly convertible to '" << *expected + << "'"; + } +} + // Perform type argument deduction, matching the parameter type `param` // against the argument type `arg`. Whenever there is an VariableType // in the parameter type, it is deduced to be the corresponding type @@ -125,7 +210,8 @@ static auto ArgumentDeduction(SourceLocation source_loc, TypeEnv deduced, if (!d) { deduced.Set(var_type.Name(), arg); } else { - ExpectType(source_loc, "argument deduction", *d, arg); + // TODO: can we allow implicit conversions here? + ExpectExactType(source_loc, "argument deduction", *d, arg); } return deduced; } @@ -214,7 +300,7 @@ static auto ArgumentDeduction(SourceLocation source_loc, TypeEnv deduced, case Value::Kind::AutoType: { return deduced; } - // For the following cases, we check for type equality. + // For the following cases, we check for type convertability. case Value::Kind::ContinuationType: case Value::Kind::NominalClassType: case Value::Kind::ChoiceType: @@ -476,45 +562,50 @@ auto TypeChecker::TypeCheckExp(Nonnull e, TypeEnv types, } switch (op.Op()) { case Operator::Neg: - ExpectType(e->source_loc(), "negation", arena->New(), ts[0]); + ExpectExactType(e->source_loc(), "negation", arena->New(), + ts[0]); return TCResult(arena->New(), new_types); case Operator::Add: - ExpectType(e->source_loc(), "addition(1)", arena->New(), - ts[0]); - ExpectType(e->source_loc(), "addition(2)", arena->New(), - ts[1]); + ExpectExactType(e->source_loc(), "addition(1)", arena->New(), + ts[0]); + ExpectExactType(e->source_loc(), "addition(2)", arena->New(), + ts[1]); return TCResult(arena->New(), new_types); case Operator::Sub: - ExpectType(e->source_loc(), "subtraction(1)", arena->New(), - ts[0]); - ExpectType(e->source_loc(), "subtraction(2)", arena->New(), - ts[1]); + ExpectExactType(e->source_loc(), "subtraction(1)", + arena->New(), ts[0]); + ExpectExactType(e->source_loc(), "subtraction(2)", + arena->New(), ts[1]); return TCResult(arena->New(), new_types); case Operator::Mul: - ExpectType(e->source_loc(), "multiplication(1)", - arena->New(), ts[0]); - ExpectType(e->source_loc(), "multiplication(2)", - arena->New(), ts[1]); + ExpectExactType(e->source_loc(), "multiplication(1)", + arena->New(), ts[0]); + ExpectExactType(e->source_loc(), "multiplication(2)", + arena->New(), ts[1]); return TCResult(arena->New(), new_types); case Operator::And: - ExpectType(e->source_loc(), "&&(1)", arena->New(), ts[0]); - ExpectType(e->source_loc(), "&&(2)", arena->New(), ts[1]); + ExpectExactType(e->source_loc(), "&&(1)", arena->New(), + ts[0]); + ExpectExactType(e->source_loc(), "&&(2)", arena->New(), + ts[1]); return TCResult(arena->New(), new_types); case Operator::Or: - ExpectType(e->source_loc(), "||(1)", arena->New(), ts[0]); - ExpectType(e->source_loc(), "||(2)", arena->New(), ts[1]); + ExpectExactType(e->source_loc(), "||(1)", arena->New(), + ts[0]); + ExpectExactType(e->source_loc(), "||(2)", arena->New(), + ts[1]); return TCResult(arena->New(), new_types); case Operator::Not: - ExpectType(e->source_loc(), "!", arena->New(), ts[0]); + ExpectExactType(e->source_loc(), "!", arena->New(), ts[0]); return TCResult(arena->New(), new_types); case Operator::Eq: - ExpectType(e->source_loc(), "==", ts[0], ts[1]); + ExpectExactType(e->source_loc(), "==", ts[0], ts[1]); return TCResult(arena->New(), new_types); case Operator::Deref: ExpectPointerType(e->source_loc(), "*", ts[0]); return TCResult(cast(*ts[0]).Type(), new_types); case Operator::Ptr: - ExpectType(e->source_loc(), "*", arena->New(), ts[0]); + ExpectExactType(e->source_loc(), "*", arena->New(), ts[0]); return TCResult(arena->New(), new_types); } break; @@ -603,16 +694,20 @@ auto TypeChecker::TypeCheckPattern( Nonnull type = interpreter.InterpPattern(values, binding.Type()); if (expected) { - std::optional values = interpreter.PatternMatch( - type, *expected, binding.Type()->source_loc()); - if (values == std::nullopt) { - FATAL_COMPILATION_ERROR(binding.Type()->source_loc()) - << "Type pattern '" << *type << "' does not match actual type '" - << **expected << "'"; + if (IsConcreteType(type)) { + ExpectType(p->source_loc(), "name binding", type, *expected); + } else { + std::optional values = interpreter.PatternMatch( + type, *expected, binding.Type()->source_loc()); + if (values == std::nullopt) { + FATAL_COMPILATION_ERROR(binding.Type()->source_loc()) + << "Type pattern '" << *type << "' does not match actual type '" + << **expected << "'"; + } + CHECK(values->begin() == values->end()) + << "Name bindings within type patterns are unsupported"; + type = *expected; } - CHECK(values->begin() == values->end()) - << "Name bindings within type patterns are unsupported"; - type = *expected; } ExpectIsConcreteType(binding.source_loc(), type); if (binding.Name().has_value()) { @@ -664,8 +759,8 @@ auto TypeChecker::TypeCheckPattern( << "alternative pattern does not name a choice type."; } if (expected) { - ExpectType(alternative.source_loc(), "alternative pattern", *expected, - choice_type); + ExpectExactType(alternative.source_loc(), "alternative pattern", + *expected, choice_type); } std::optional> parameter_types = FindInVarValues(alternative.AlternativeName(), diff --git a/executable_semantics/testdata/class/assign.carbon b/executable_semantics/testdata/class/assign.carbon new file mode 100644 index 000000000000..102275682881 --- /dev/null +++ b/executable_semantics/testdata/class/assign.carbon @@ -0,0 +1,24 @@ +// 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: executable_semantics %s 2>&1 | \ +// RUN: FileCheck --match-full-lines --allow-unused-prefixes=false %s +// RUN: executable_semantics --trace %s 2>&1 | \ +// RUN: FileCheck --match-full-lines --allow-unused-prefixes %s +// AUTOUPDATE: executable_semantics %s +// CHECK: result: 0 + +package ExecutableSemanticsTest api; + +class Point { + var x: i32; + var y: i32; +} + +fn main() -> i32 { + var p1: Point = {.x = 1, .y = 2}; + var p2: auto = p1; + p2 = {.x = 3, .y = 2}; + return p1.x - 1; +} diff --git a/executable_semantics/testdata/class/assign_member.carbon b/executable_semantics/testdata/class/assign_member.carbon index 0b0f0abc3aec..c334a8f76166 100644 --- a/executable_semantics/testdata/class/assign_member.carbon +++ b/executable_semantics/testdata/class/assign_member.carbon @@ -17,7 +17,7 @@ class Point { } fn main() -> i32 { - var p1: auto = Point(.x = 1, .y = 2); + var p1: Point = {.x = 1, .y = 2}; var p2: auto = p1; p2.x = 3; return p1.x - 1; diff --git a/executable_semantics/testdata/class/fail_field_access_mismatch.carbon b/executable_semantics/testdata/class/fail_field_access_mismatch.carbon index 28eb051ad680..3ee338dd2439 100644 --- a/executable_semantics/testdata/class/fail_field_access_mismatch.carbon +++ b/executable_semantics/testdata/class/fail_field_access_mismatch.carbon @@ -7,7 +7,7 @@ // RUN: not executable_semantics --trace %s 2>&1 | \ // RUN: FileCheck --match-full-lines --allow-unused-prefixes %s // AUTOUPDATE: executable_semantics %s -// CHECK: COMPILATION ERROR: {{.*}}/executable_semantics/testdata/class/fail_field_access_mismatch.carbon:20: class Point does not have a field named z +// CHECK: COMPILATION ERROR: {{.*}}/executable_semantics/testdata/class/fail_field_access_mismatch.carbon:21: class Point does not have a field named z package ExecutableSemanticsTest api; @@ -17,5 +17,6 @@ class Point { } fn main() -> i32 { - return Point(.x = 1, .y = 2).z - 1; + var p: Point = {.x = 1, .y = 2}; + return p.z - 1; } diff --git a/executable_semantics/testdata/class/fail_field_mismatch.carbon b/executable_semantics/testdata/class/fail_field_mismatch.carbon index 2b1a6f0b2557..ea8f6882e71f 100644 --- a/executable_semantics/testdata/class/fail_field_mismatch.carbon +++ b/executable_semantics/testdata/class/fail_field_mismatch.carbon @@ -7,9 +7,7 @@ // RUN: not executable_semantics --trace %s 2>&1 | \ // RUN: FileCheck --match-full-lines --allow-unused-prefixes %s // AUTOUPDATE: executable_semantics %s -// CHECK: COMPILATION ERROR: {{.*}}/executable_semantics/testdata/class/fail_field_mismatch.carbon:22: type error in call -// CHECK: expected: (x = i32, y = i32) -// CHECK: actual: (x = i32, z = i32) +// CHECK: COMPILATION ERROR: {{.*}}/executable_semantics/testdata/class/fail_field_mismatch.carbon:20: type error in name binding: '{.x: i32, .z: i32}' is not implicitly convertible to 'class Point' package ExecutableSemanticsTest api; @@ -19,5 +17,6 @@ class Point { } fn main() -> i32 { - return Point(.x = 1, .z = 2).x - 1; + var p: Point = {.x = 1, .z = 2}; + return p.x - 1; } diff --git a/executable_semantics/testdata/class/fail_field_missing.carbon b/executable_semantics/testdata/class/fail_field_missing.carbon index 78e7fbd89e79..75ecf70c1876 100644 --- a/executable_semantics/testdata/class/fail_field_missing.carbon +++ b/executable_semantics/testdata/class/fail_field_missing.carbon @@ -7,9 +7,7 @@ // RUN: not executable_semantics --trace %s 2>&1 | \ // RUN: FileCheck --match-full-lines --allow-unused-prefixes %s // AUTOUPDATE: executable_semantics %s -// CHECK: COMPILATION ERROR: {{.*}}/executable_semantics/testdata/class/fail_field_missing.carbon:22: type error in call -// CHECK: expected: (x = i32, y = i32) -// CHECK: actual: (x = i32) +// CHECK: COMPILATION ERROR: {{.*}}/executable_semantics/testdata/class/fail_field_missing.carbon:20: type error in name binding: '{.x: i32}' is not implicitly convertible to 'class Point' package ExecutableSemanticsTest api; @@ -19,5 +17,6 @@ class Point { } fn main() -> i32 { - return Point(.x = 1).x - 1; + var p: Point = {.x = 1}; + return p.x - 1; } diff --git a/executable_semantics/testdata/class/function_param.carbon b/executable_semantics/testdata/class/function_param.carbon new file mode 100644 index 000000000000..1c062e45ab9e --- /dev/null +++ b/executable_semantics/testdata/class/function_param.carbon @@ -0,0 +1,25 @@ +// 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: executable_semantics %s 2>&1 | \ +// RUN: FileCheck --match-full-lines --allow-unused-prefixes=false %s +// RUN: executable_semantics --trace %s 2>&1 | \ +// RUN: FileCheck --match-full-lines --allow-unused-prefixes %s +// AUTOUPDATE: executable_semantics %s +// CHECK: result: 0 + +package ExecutableSemanticsTest api; + +class Point { + var x: i32; + var y: i32; +} + +fn GetX(p: Point) -> i32 { + return p.x; +} + +fn main() -> i32 { + return GetX({.x = 1, .y = 2}) - 1; +} diff --git a/executable_semantics/testdata/class/global_var.carbon b/executable_semantics/testdata/class/global_var.carbon new file mode 100644 index 000000000000..fb971e7009a8 --- /dev/null +++ b/executable_semantics/testdata/class/global_var.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: executable_semantics %s 2>&1 | \ +// RUN: FileCheck --match-full-lines --allow-unused-prefixes=false %s +// RUN: executable_semantics --trace %s 2>&1 | \ +// RUN: FileCheck --match-full-lines --allow-unused-prefixes %s +// AUTOUPDATE: executable_semantics %s +// CHECK: result: 0 + +package ExecutableSemanticsTest api; + +class Point { + var x: i32; + var y: i32; +} + +var p: Point = {.x = 1, .y = 2}; + +fn main() -> i32 { + return p.y - p.x - 1; +} diff --git a/executable_semantics/testdata/class/temp.carbon b/executable_semantics/testdata/class/temp.carbon index 7ceb047e3ee4..b0ab1a050933 100644 --- a/executable_semantics/testdata/class/temp.carbon +++ b/executable_semantics/testdata/class/temp.carbon @@ -16,6 +16,10 @@ class Point { var y: i32; } +fn MakePoint() -> Point { + return {.x = 1, .y = 2}; +} + fn main() -> i32 { - return Point(.x = 1, .y = 2).x - 1; + return MakePoint().x - 1; } diff --git a/executable_semantics/testdata/class/var.carbon b/executable_semantics/testdata/class/var.carbon index d3574e2ec11b..cf50b726d98a 100644 --- a/executable_semantics/testdata/class/var.carbon +++ b/executable_semantics/testdata/class/var.carbon @@ -17,6 +17,6 @@ class Point { } fn main() -> i32 { - var p: auto = Point(.x = 1, .y = 2); + var p: Point = {.x = 1, .y = 2}; return p.y - p.x - 1; } diff --git a/executable_semantics/testdata/function/fail_call_with_tuple.carbon b/executable_semantics/testdata/function/fail_call_with_tuple.carbon index ccc0fe516175..298cb4614d57 100644 --- a/executable_semantics/testdata/function/fail_call_with_tuple.carbon +++ b/executable_semantics/testdata/function/fail_call_with_tuple.carbon @@ -7,9 +7,7 @@ // RUN: not executable_semantics --trace %s 2>&1 | \ // RUN: FileCheck --match-full-lines --allow-unused-prefixes %s // AUTOUPDATE: executable_semantics %s -// CHECK: COMPILATION ERROR: {{.*}}/executable_semantics/testdata/function/fail_call_with_tuple.carbon:21: type error in call -// CHECK: expected: (0 = i32, 1 = i32) -// CHECK: actual: (0 = (0 = i32, 1 = i32)) +// CHECK: COMPILATION ERROR: {{.*}}/executable_semantics/testdata/function/fail_call_with_tuple.carbon:19: type error in call: '(0 = (0 = i32, 1 = i32))' is not implicitly convertible to '(0 = i32, 1 = i32)' package ExecutableSemanticsTest api; diff --git a/executable_semantics/testdata/global_variable/fail_init_type_mismatch.carbon b/executable_semantics/testdata/global_variable/fail_init_type_mismatch.carbon index 1bc3dba302a2..7c5b662654ab 100644 --- a/executable_semantics/testdata/global_variable/fail_init_type_mismatch.carbon +++ b/executable_semantics/testdata/global_variable/fail_init_type_mismatch.carbon @@ -7,9 +7,7 @@ // RUN: not executable_semantics --trace %s 2>&1 | \ // RUN: FileCheck --match-full-lines --allow-unused-prefixes %s // AUTOUPDATE: executable_semantics %s -// CHECK: COMPILATION ERROR: {{.*}}/executable_semantics/testdata/global_variable/fail_init_type_mismatch.carbon:18: type error in initializer of variable -// CHECK: expected: i32 -// CHECK: actual: Bool +// CHECK: COMPILATION ERROR: {{.*}}/executable_semantics/testdata/global_variable/fail_init_type_mismatch.carbon:16: type error in initializer of variable: 'Bool' is not implicitly convertible to 'i32' package ExecutableSemanticsTest api; diff --git a/executable_semantics/testdata/struct/fail_name_order.carbon b/executable_semantics/testdata/struct/name_order.carbon similarity index 59% rename from executable_semantics/testdata/struct/fail_name_order.carbon rename to executable_semantics/testdata/struct/name_order.carbon index 9d69962eff75..15cc0514585b 100644 --- a/executable_semantics/testdata/struct/fail_name_order.carbon +++ b/executable_semantics/testdata/struct/name_order.carbon @@ -2,16 +2,16 @@ // Exceptions. See /LICENSE for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // -// RUN: not executable_semantics %s 2>&1 | \ +// RUN: executable_semantics %s 2>&1 | \ // RUN: FileCheck --match-full-lines --allow-unused-prefixes=false %s -// RUN: not executable_semantics --trace %s 2>&1 | \ +// RUN: executable_semantics --trace %s 2>&1 | \ // RUN: FileCheck --match-full-lines --allow-unused-prefixes %s // AUTOUPDATE: executable_semantics %s -// CHECK: COMPILATION ERROR: {{.*}}/executable_semantics/testdata/struct/fail_name_order.carbon:17: Type pattern '{.x: i32, .y: i32}' does not match actual type '{.y: i32, .x: i32}' +// CHECK: result: 0 package ExecutableSemanticsTest api; -// Test the that field order matters for structs. +// Test the that field order doesn't matter for structs. fn main() -> i32 { var t: {.x: i32, .y: i32} = {.y = 2, .x = 3}; diff --git a/executable_semantics/testdata/tuple/fail_name_order.carbon b/executable_semantics/testdata/tuple/fail_name_order.carbon index 8f65f52f819c..ae991d850543 100644 --- a/executable_semantics/testdata/tuple/fail_name_order.carbon +++ b/executable_semantics/testdata/tuple/fail_name_order.carbon @@ -7,7 +7,7 @@ // RUN: not executable_semantics --trace %s 2>&1 | \ // RUN: FileCheck --match-full-lines --allow-unused-prefixes %s // AUTOUPDATE: executable_semantics %s -// CHECK: PROGRAM ERROR: {{.*}}/executable_semantics/testdata/tuple/fail_name_order.carbon:17: Tuple field name 'y' does not match pattern field name 'x' +// CHECK: COMPILATION ERROR: {{.*}}/executable_semantics/testdata/tuple/fail_name_order.carbon:17: type error in name binding: '(y = i32, x = i32)' is not implicitly convertible to '(x = i32, y = i32)' package ExecutableSemanticsTest api;