From 8ab54a37f6db2cb9fe03e90e3a12d7bc3ccd6b00 2021-04-06 19:08:32 From: MH Date: 2021-04-06 19:08:32 Subject: [PATCH] Initial bugfixing for unions --- diff --git a/src/protocol/parser/type_resolver.rs b/src/protocol/parser/type_resolver.rs index 9248f11b8c1a23825225b47e27e4f7d6fe85a59f..e6061c05796a07c9957aeb6484bca766f96a3ede 100644 --- a/src/protocol/parser/type_resolver.rs +++ b/src/protocol/parser/type_resolver.rs @@ -1426,6 +1426,7 @@ impl TypeResolvingVisitor { let definition_id = match &lit_expr.value { Literal::Struct(literal) => literal.definition.as_ref().unwrap(), Literal::Enum(literal) => literal.definition.as_ref().unwrap(), + Literal::Union(literal) => literal.definition.as_ref().unwrap(), _ => unreachable!("post-inference monomorph for non-struct, non-enum literal") }; if !ctx.types.has_monomorph(definition_id, &monomorph_types) { @@ -3038,7 +3039,7 @@ impl TypeResolvingVisitor { let mut embedded = Vec::with_capacity(variant_definition.embedded.len()); for embedded_id in &variant_definition.embedded { let inference_type = self.determine_inference_type_from_parser_type( - ctx, *embedded_id, true + ctx, *embedded_id, false ); embedded.push(inference_type); } @@ -3195,10 +3196,11 @@ impl TypeResolvingVisitor { // TODO: @cleanup if cfg!(debug_assertions) { let definition = &ctx.heap[*definition_id]; - debug_assert!(definition.is_struct() || definition.is_enum()); // TODO: @function_ptrs + debug_assert!(definition.is_struct() || definition.is_enum() || definition.is_union()); // TODO: @function_ptrs let num_poly = match definition { Definition::Struct(v) => v.poly_vars.len(), Definition::Enum(v) => v.poly_vars.len(), + Definition::Union(v) => v.poly_vars.len(), _ => unreachable!(), }; debug_assert_eq!(symbolic.poly_args2.len(), num_poly); diff --git a/src/protocol/parser/type_table.rs b/src/protocol/parser/type_table.rs index e1735e1e230dd82a76e1e9f2d2f660457c4e71ae..4f8b078c61061da64096c3d2445f6647087142a6 100644 --- a/src/protocol/parser/type_table.rs +++ b/src/protocol/parser/type_table.rs @@ -535,7 +535,7 @@ impl TypeTable { // Ensure union names and polymorphic args do not conflict self.check_identifier_collision( - ctx, root_id, &variants, |variant| &variant.identifier, "enum variant" + ctx, root_id, &variants, |variant| &variant.identifier, "union variant" )?; self.check_poly_args_collision(ctx, root_id, &definition.poly_vars)?; @@ -915,7 +915,7 @@ impl TypeTable { } // Found a match, make sure it is a datatype - if !(definition.is_struct() || definition.is_enum()) { + if !(definition.is_struct() || definition.is_enum() || definition.is_union()) { return Err(ParseError::new_error( module_source, symbolic.identifier.position, "Embedded types must be datatypes (structs or enums)" diff --git a/src/protocol/parser/visitor_linker.rs b/src/protocol/parser/visitor_linker.rs index 19978255561c11bebdc20bb7a142129b9180f935..db99c02361888c1e6884fdfeaf6c4e3d46cafb4a 100644 --- a/src/protocol/parser/visitor_linker.rs +++ b/src/protocol/parser/visitor_linker.rs @@ -761,17 +761,24 @@ impl Visitor2 for ValidityAndLinkerVisitor { if !specified.iter().all(|v| *v) { // Some fields were not specified let mut not_specified = String::new(); + let mut num_not_specified = 0; for (def_field_idx, is_specified) in specified.iter().enumerate() { if !is_specified { if !not_specified.is_empty() { not_specified.push_str(", ") } let field_ident = &definition.fields[def_field_idx].identifier; not_specified.push_str(&String::from_utf8_lossy(&field_ident.value)); + num_not_specified += 1; } } + debug_assert!(num_not_specified > 0); + let msg = if num_not_specified == 1 { + format!("Not all fields are specified, '{}' is missing", not_specified) + } else { + format!("Not all fields are specified, [{}] are missing", not_specified) + }; return Err(ParseError::new_error( - &ctx.module.source, literal.identifier.position, - &format!("Not all fields are specified, [{}] are missing", not_specified) + &ctx.module.source, literal.identifier.position, &msg )); } @@ -907,7 +914,7 @@ impl Visitor2 for ValidityAndLinkerVisitor { return Err(ParseError::new_error( &ctx.module.source, literal.identifier.position, &format!( - "This variant '{}' of union '{}' expects {} embedded values, but {} were specified", + "The variant '{}' of union '{}' expects {} embedded values, but {} were specified", variant, &String::from_utf8_lossy(&union_definition.identifier.value), union_variant.embedded.len(), literal.values.len() ), diff --git a/src/protocol/tests/parser_inference.rs b/src/protocol/tests/parser_inference.rs index 6855fa19d14c0253ac287c308363f9cb46b5c279..24cc508e75e7af70f556403233bb27b8c5565652 100644 --- a/src/protocol/tests/parser_inference.rs +++ b/src/protocol/tests/parser_inference.rs @@ -366,8 +366,8 @@ fn test_failed_polymorph_inference() { " ).error(|e| { e .assert_num(2) - .assert_msg_has(0, "the type 'Uninteresting'") - .assert_msg_has(1, "type 'Uninteresting'"); + .assert_any_msg_has("type 'Uninteresting'") + .assert_any_msg_has("type 'Uninteresting'"); }); Tester::new_single_source_expect_err( diff --git a/src/protocol/tests/parser_validation.rs b/src/protocol/tests/parser_validation.rs index 6b058569a03f21230018723fbe973c4f2fc982d1..5af066d56fd571d4c2f7e67ed08978014f580150 100644 --- a/src/protocol/tests/parser_validation.rs +++ b/src/protocol/tests/parser_validation.rs @@ -86,3 +86,255 @@ fn test_correct_struct_instance() { " ); } + +#[test] +fn test_incorrect_struct_instance() { + Tester::new_single_source_expect_err( + "reused field in definition", + "struct Foo{ int a, byte a }" + ).error(|e| { e + .assert_num(2) + .assert_occurs_at(0, "a }") + .assert_msg_has(0, "defined more than once") + .assert_occurs_at(1, "a, ") + .assert_msg_has(1, "other struct field"); + }); + + Tester::new_single_source_expect_err( + "reused field in instance", + " + struct Foo{ int a, int b } + int bar() { + auto foo = Foo{ a: 5, a: 3 }; + return 0; + } + " + ).error(|e| { e + .assert_occurs_at(0, "a: 3") + .assert_msg_has(0, "field is specified more than once"); + }); + + Tester::new_single_source_expect_err( + "missing field", + " + struct Foo { int a, int b } + int bar() { + auto foo = Foo{ a: 2 }; + return 0; + } + " + ).error(|e| { e + .assert_occurs_at(0, "Foo{") + .assert_msg_has(0, "'b' is missing"); + }); + + Tester::new_single_source_expect_err( + "missing fields", + " + struct Foo { int a, int b, int c } + int bar() { + auto foo = Foo{ a: 2 }; + return 0; + } + " + ).error(|e| { e + .assert_occurs_at(0, "Foo{") + .assert_msg_has(0, "[b, c] are missing"); + }); +} + +#[test] +fn test_correct_enum_instance() { + Tester::new_single_source_expect_ok( + "single variant", + " + enum Foo { A } + Foo bar() { return Foo::A; } + " + ); + + Tester::new_single_source_expect_ok( + "multiple variants", + " + enum Foo { A=15, B = 0xF } + Foo bar() { auto a = Foo::A; return Foo::B; } + " + ); + + Tester::new_single_source_expect_ok( + "explicit single polymorph", + " + enum Foo{ A } + Foo bar() { return Foo::A; } + " + ); + + Tester::new_single_source_expect_ok( + "explicit multi-polymorph", + " + enum Foo{ A, B } + Foo bar() { return Foo::B; } + " + ); +} + +#[test] +fn test_incorrect_enum_instance() { + Tester::new_single_source_expect_err( + "variant name reuse", + " + enum Foo { A, A } + Foo bar() { return Foo::A; } + " + ).error(|e| { e + .assert_num(2) + .assert_occurs_at(0, "A }") + .assert_msg_has(0, "defined more than once") + .assert_occurs_at(1, "A, ") + .assert_msg_has(1, "other enum variant is defined here"); + }); + + Tester::new_single_source_expect_err( + "undefined variant", + " + enum Foo { A } + Foo bar() { return Foo::B; } + " + ).error(|e| { e + .assert_num(1) + .assert_msg_has(0, "variant 'B' does not exist on the enum 'Foo'"); + }); +} + +#[test] +fn test_correct_union_instance() { + Tester::new_single_source_expect_ok( + "single tag", + " + union Foo { A } + Foo bar() { return Foo::A; } + " + ); + + Tester::new_single_source_expect_ok( + "multiple tags", + " + union Foo { A, B } + Foo bar() { return Foo::B; } + " + ); + + Tester::new_single_source_expect_ok( + "single embedded", + " + union Foo { A(int) } + Foo bar() { return Foo::A(5); } + " + ); + + Tester::new_single_source_expect_ok( + "multiple embedded", + " + union Foo { A(int), B(byte) } + Foo bar() { return Foo::B(2); } + " + ); + + Tester::new_single_source_expect_ok( + "multiple values in embedded", + " + union Foo { A(int, byte) } + Foo bar() { return Foo::A(0, 2); } + " + ); + + Tester::new_single_source_expect_ok( + "mixed tag/embedded", + " + union OptionInt { None, Some(int) } + OptionInt bar() { return OptionInt::Some(3); } + " + ); + + Tester::new_single_source_expect_ok( + "single polymorphic var", + " + union Option { None, Some(T) } + Option bar() { return Option::Some(3); }" + ); + + Tester::new_single_source_expect_ok( + "multiple polymorphic vars", + " + union Result { Ok(T), Err(E), } + Result bar() { return Result::Ok(3); } + " + ); + + Tester::new_single_source_expect_ok( + "multiple polymorphic in one variant", + " + union MaybePair{ None, Some(T1, T2) } + MaybePair bar() { return MaybePair::Some(1, 2); } + " + ); +} + +#[test] +fn test_incorrect_union_instance() { + Tester::new_single_source_expect_err( + "tag-variant name reuse", + " + union Foo{ A, A } + " + ).error(|e| { e + .assert_num(2) + .assert_occurs_at(0, "A }") + .assert_msg_has(0, "union variant is defined more than once") + .assert_occurs_at(1, "A, ") + .assert_msg_has(1, "other union variant"); + }); + + Tester::new_single_source_expect_err( + "embedded-variant name reuse", + " + union Foo{ A(int), A(byte) } + " + ).error(|e| { e + .assert_num(2) + .assert_occurs_at(0, "A(byte)") + .assert_msg_has(0, "union variant is defined more than once") + .assert_occurs_at(1, "A(int)") + .assert_msg_has(1, "other union variant"); + }); + + Tester::new_single_source_expect_err( + "undefined variant", + " + union Silly{ Thing(byte) } + Silly bar() { return Silly::Undefined(5); } + " + ).error(|e| { e + .assert_msg_has(0, "variant 'Undefined' does not exist on the union 'Silly'"); + }); + + Tester::new_single_source_expect_err( + "using tag instead of embedded", + " + union Foo{ A(int) } + Foo bar() { return Foo::A; } + " + ).error(|e| { e + .assert_msg_has(0, "variant 'A' of union 'Foo' expects 1 embedded values, but 0 were"); + }); + + Tester::new_single_source_expect_err( + "using embedded instead of tag", + " + union Foo{ A } + Foo bar() { return Foo::A(3); } + " + ).error(|e| { e + .assert_msg_has(0, "The variant 'A' of union 'Foo' expects 0"); + }); +} \ No newline at end of file diff --git a/src/protocol/tests/utils.rs b/src/protocol/tests/utils.rs index d024b850a84b2ccec71ed575d289726037684ae6..30fe97d9e73b18ea3e2ed919742cb31db916fa3d 100644 --- a/src/protocol/tests/utils.rs +++ b/src/protocol/tests/utils.rs @@ -656,6 +656,26 @@ impl<'a> ErrorTester<'a> { self } + // TODO: @tokenizer This should really be removed, as compilation should be + // deterministic, but we're currently using rather inefficient hashsets for + // the type inference, so remove once compiler architecture has changed. + pub(crate) fn assert_any_msg_has(self, msg: &str) -> Self { + let mut is_present = false; + for statement in &self.error.statements { + if statement.message.contains(msg) { + is_present = true; + break; + } + } + + assert!( + is_present, "[{}] Expected an error statement to contain '{}' for {}", + self.test_name, msg, self.assert_postfix() + ); + + self + } + /// Seeks the index of the pattern in the context message, then checks if /// the input position corresponds to that index. pub (crate) fn assert_occurs_at(self, idx: usize, pattern: &str) -> Self {