From b87f09115cc1a8fd3b55697e0306420b781f7774 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Fri, 29 Jul 2022 11:22:54 -0400 Subject: [PATCH] Report opaques as opaques, not aliases Closes #3313 Closes #3654 --- crates/compiler/can/src/annotation.rs | 1 + crates/compiler/can/src/def.rs | 15 ++++++++++-- crates/compiler/problem/src/can.rs | 3 ++- crates/compiler/types/src/types.rs | 11 +++++++++ crates/reporting/src/error/canonicalize.rs | 13 ++++++---- crates/reporting/src/error/type.rs | 26 ++++++++++++++------ crates/reporting/tests/test_reporting.rs | 28 +++++++++++++++++++--- 7 files changed, 80 insertions(+), 17 deletions(-) diff --git a/crates/compiler/can/src/annotation.rs b/crates/compiler/can/src/annotation.rs index 5d4123c4b1..e0cefdebb5 100644 --- a/crates/compiler/can/src/annotation.rs +++ b/crates/compiler/can/src/annotation.rs @@ -566,6 +566,7 @@ fn can_annotation_help( region, alias_needs: alias.type_variables.len() as u8, type_got: args.len() as u8, + alias_kind: alias.kind, }); return error; } diff --git a/crates/compiler/can/src/def.rs b/crates/compiler/can/src/def.rs index 00a41d12fd..48a5d78e31 100644 --- a/crates/compiler/can/src/def.rs +++ b/crates/compiler/can/src/def.rs @@ -369,6 +369,7 @@ fn canonicalize_alias<'a>( typ: symbol, variable_region: loc_lowercase.region, variable_name: loc_lowercase.value.clone(), + alias_kind: AliasKind::Structural, }); } AliasKind::Opaque => { @@ -2688,6 +2689,7 @@ fn correct_mutual_recursive_type_alias<'a>( env, &mut alias.typ, alias_name, + alias.kind, alias.region, rest, can_still_report_error, @@ -2870,7 +2872,15 @@ fn make_tag_union_recursive_help<'a, 'b>( } _ => { // take care to report a cyclic alias only once (not once for each alias in the cycle) - mark_cyclic_alias(env, typ, symbol, region, others, *can_report_cyclic_error); + mark_cyclic_alias( + env, + typ, + symbol, + alias_kind, + region, + others, + *can_report_cyclic_error, + ); *can_report_cyclic_error = false; Cyclic @@ -2882,6 +2892,7 @@ fn mark_cyclic_alias<'a>( env: &mut Env<'a>, typ: &mut Type, symbol: Symbol, + alias_kind: AliasKind, region: Region, others: Vec, report: bool, @@ -2890,7 +2901,7 @@ fn mark_cyclic_alias<'a>( *typ = Type::Erroneous(problem); if report { - let problem = Problem::CyclicAlias(symbol, region, others); + let problem = Problem::CyclicAlias(symbol, region, others, alias_kind); env.problems.push(problem); } } diff --git a/crates/compiler/problem/src/can.rs b/crates/compiler/problem/src/can.rs index ba956a7f72..82f2a6f35d 100644 --- a/crates/compiler/problem/src/can.rs +++ b/crates/compiler/problem/src/can.rs @@ -45,12 +45,13 @@ pub enum Problem { shadow: Loc, kind: ShadowKind, }, - CyclicAlias(Symbol, Region, Vec), + CyclicAlias(Symbol, Region, Vec, AliasKind), BadRecursion(Vec), PhantomTypeArgument { typ: Symbol, variable_region: Region, variable_name: Lowercase, + alias_kind: AliasKind, }, UnboundTypeVariable { typ: Symbol, diff --git a/crates/compiler/types/src/types.rs b/crates/compiler/types/src/types.rs index 78d5035846..c0fe6c0739 100644 --- a/crates/compiler/types/src/types.rs +++ b/crates/compiler/types/src/types.rs @@ -1322,6 +1322,7 @@ impl Type { region, type_got: args.len() as u8, alias_needs: alias.type_variables.len() as u8, + alias_kind: AliasKind::Structural, }); return; } @@ -2028,6 +2029,15 @@ pub enum AliasKind { Opaque, } +impl AliasKind { + pub fn as_str(&self) -> &'static str { + match self { + AliasKind::Structural => "alias", + AliasKind::Opaque => "opaque", + } + } +} + #[derive(Clone, Debug, PartialEq)] pub struct AliasVar { pub name: Lowercase, @@ -2104,6 +2114,7 @@ pub enum Problem { region: Region, type_got: u8, alias_needs: u8, + alias_kind: AliasKind, }, InvalidModule, SolvedTypeError, diff --git a/crates/reporting/src/error/canonicalize.rs b/crates/reporting/src/error/canonicalize.rs index b6393f3ed6..268def5869 100644 --- a/crates/reporting/src/error/canonicalize.rs +++ b/crates/reporting/src/error/canonicalize.rs @@ -233,8 +233,10 @@ pub fn can_problem<'b>( title = DUPLICATE_NAME.to_string(); severity = Severity::RuntimeError; } - Problem::CyclicAlias(symbol, region, others) => { - let answer = crate::error::r#type::cyclic_alias(alloc, lines, symbol, region, others); + Problem::CyclicAlias(symbol, region, others, alias_kind) => { + let answer = crate::error::r#type::cyclic_alias( + alloc, lines, symbol, region, others, alias_kind, + ); doc = answer.0; title = answer.1; @@ -244,6 +246,7 @@ pub fn can_problem<'b>( typ: alias, variable_region, variable_name, + alias_kind, } => { doc = alloc.stack([ alloc.concat([ @@ -251,10 +254,12 @@ pub fn can_problem<'b>( alloc.type_variable(variable_name), alloc.reflow(" type parameter is not used in the "), alloc.symbol_unqualified(alias), - alloc.reflow(" alias definition:"), + alloc.reflow(" "), + alloc.reflow(alias_kind.as_str()), + alloc.reflow(" definition:"), ]), alloc.region(lines.convert_region(variable_region)), - alloc.reflow("Roc does not allow unused type alias parameters!"), + alloc.reflow("Roc does not allow unused type parameters!"), // TODO add link to this guide section alloc.tip().append(alloc.reflow( "If you want an unused type parameter (a so-called \"phantom type\"), \ diff --git a/crates/reporting/src/error/type.rs b/crates/reporting/src/error/type.rs index 88a4ae1862..0801fa26d3 100644 --- a/crates/reporting/src/error/type.rs +++ b/crates/reporting/src/error/type.rs @@ -77,6 +77,7 @@ pub fn type_problem<'b>( region, type_got, alias_needs, + alias_kind, } => { let needed_arguments = if alias_needs == 1 { alloc.reflow("1 type argument") @@ -92,7 +93,9 @@ pub fn type_problem<'b>( alloc.concat([ alloc.reflow("The "), alloc.symbol_unqualified(symbol), - alloc.reflow(" alias expects "), + alloc.reflow(" "), + alloc.reflow(alias_kind.as_str()), + alloc.reflow(" expects "), needed_arguments, alloc.reflow(", but it got "), found_arguments, @@ -433,16 +436,21 @@ pub fn cyclic_alias<'b>( symbol: Symbol, region: roc_region::all::Region, others: Vec, + alias_kind: AliasKind, ) -> (RocDocBuilder<'b>, String) { let when_is_recursion_legal = - alloc.reflow("Recursion in aliases is only allowed if recursion happens behind a tagged union, at least one variant of which is not recursive."); + alloc.reflow("Recursion in ") + .append(alloc.reflow(alias_kind.as_str())) + .append(alloc.reflow("es is only allowed if recursion happens behind a tagged union, at least one variant of which is not recursive.")); let doc = if others.is_empty() { alloc.stack([ alloc .reflow("The ") .append(alloc.symbol_unqualified(symbol)) - .append(alloc.reflow(" alias is self-recursive in an invalid way:")), + .append(alloc.reflow(" ")) + .append(alloc.reflow(alias_kind.as_str())) + .append(alloc.reflow(" is self-recursive in an invalid way:")), alloc.region(lines.convert_region(region)), when_is_recursion_legal, ]) @@ -451,14 +459,18 @@ pub fn cyclic_alias<'b>( alloc .reflow("The ") .append(alloc.symbol_unqualified(symbol)) - .append(alloc.reflow(" alias is recursive in an invalid way:")), + .append(alloc.reflow(" ")) + .append(alloc.reflow(alias_kind.as_str())) + .append(alloc.reflow(" is recursive in an invalid way:")), alloc.region(lines.convert_region(region)), alloc .reflow("The ") .append(alloc.symbol_unqualified(symbol)) - .append(alloc.reflow( - " alias depends on itself through the following chain of definitions:", - )), + .append(alloc.reflow(" ")) + .append(alloc.reflow(alias_kind.as_str())) + .append( + alloc.reflow(" depends on itself through the following chain of definitions:"), + ), crate::report::cycle( alloc, 4, diff --git a/crates/reporting/tests/test_reporting.rs b/crates/reporting/tests/test_reporting.rs index 8b0b54191e..62de8e4ffb 100644 --- a/crates/reporting/tests/test_reporting.rs +++ b/crates/reporting/tests/test_reporting.rs @@ -3112,7 +3112,7 @@ mod test_reporting { @r###" ── TOO MANY TYPE ARGUMENTS ─────────────────────────────── /code/proj/Main.roc ─ - The `Num` alias expects 1 type argument, but it got 2 instead: + The `Num` opaque expects 1 type argument, but it got 2 instead: 4│ a : Num.Num Num.I64 Num.F64 ^^^^^^^^^^^^^^^^^^^^^^^ @@ -3134,7 +3134,7 @@ mod test_reporting { @r###" ── TOO MANY TYPE ARGUMENTS ─────────────────────────────── /code/proj/Main.roc ─ - The `Num` alias expects 1 type argument, but it got 2 instead: + The `Num` opaque expects 1 type argument, but it got 2 instead: 4│ f : Str -> Num.Num Num.I64 Num.F64 ^^^^^^^^^^^^^^^^^^^^^^^ @@ -3210,7 +3210,7 @@ mod test_reporting { 4│ Foo a : [Foo] ^ - Roc does not allow unused type alias parameters! + Roc does not allow unused type parameters! Tip: If you want an unused type parameter (a so-called "phantom type"), read the guide section on phantom values. @@ -10148,4 +10148,26 @@ All branches in an `if` must have the same type! Tip: Looks like the y field is missing. "### ); + + test_report!( + cyclic_opaque, + indoc!( + r#" + Recursive := [Infinitely Recursive] + + 0 + "# + ), + @r###" + ── CYCLIC ALIAS ────────────────────────────────────────── /code/proj/Main.roc ─ + + The `Recursive` opaque is self-recursive in an invalid way: + + 4│ Recursive := [Infinitely Recursive] + ^^^^^^^^^ + + Recursion in opaquees is only allowed if recursion happens behind a + tagged union, at least one variant of which is not recursive. + "### + ); }