Merge pull request #3048 from rtfeldman/i/2389

Type solving paper cuts
This commit is contained in:
Folkert de Vries 2022-05-10 20:45:45 +02:00 committed by GitHub
commit 89c6f55288
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 320 additions and 111 deletions

View File

@ -1937,7 +1937,7 @@ pub mod test_constrain {
use roc_parse::parser::{SourceError, SyntaxError};
use roc_region::all::Region;
use roc_types::{
pretty_print::{content_to_string, name_all_type_vars},
pretty_print::name_and_print_var,
solved_types::Solved,
subs::{Subs, VarStore, Variable},
};
@ -2050,11 +2050,6 @@ pub mod test_constrain {
let subs = solved.inner_mut();
// name type vars
name_all_type_vars(var, subs);
let content = subs.get_content_without_compacting(var);
// Connect the ModuleId to it's IdentIds
dep_idents.insert(mod_id, env.ident_ids);
@ -2063,7 +2058,7 @@ pub mod test_constrain {
all_ident_ids: dep_idents,
};
let actual_str = content_to_string(content, subs, mod_id, &interns);
let actual_str = name_and_print_var(var, subs, mod_id, &interns);
assert_eq!(actual_str, expected_str);
}

View File

@ -29,7 +29,7 @@ mod test_load {
use roc_reporting::report::RenderTarget;
use roc_reporting::report::RocDocAllocator;
use roc_target::TargetInfo;
use roc_types::pretty_print::{content_to_string, name_all_type_vars};
use roc_types::pretty_print::name_and_print_var;
use roc_types::subs::Subs;
use std::collections::HashMap;
use std::path::{Path, PathBuf};
@ -237,10 +237,7 @@ mod test_load {
expected_types: &mut HashMap<&str, &str>,
) {
for (symbol, expr_var) in &def.pattern_vars {
name_all_type_vars(*expr_var, subs);
let content = subs.get_content_without_compacting(*expr_var);
let actual_str = content_to_string(content, subs, home, interns);
let actual_str = name_and_print_var(*expr_var, subs, home, interns);
let fully_qualified = symbol.fully_qualified(interns, home).to_string();
let expected_type = expected_types
.remove(fully_qualified.as_str())

View File

@ -2685,19 +2685,49 @@ fn adjust_rank_content(
}
}
// THEORY: the recursion var has the same rank as the tag union itself
// The recursion var may have a higher rank than the tag union itself, if it is
// erroneous and escapes into a region where it is let-generalized before it is
// constrained back down to the rank it originated from.
//
// For example, see the `recursion_var_specialization_error` reporting test -
// there, we have
//
// Job a : [ Job (List (Job a)) a ]
//
// job : Job Str
//
// when job is
// Job lst _ -> lst == ""
//
// In this case, `lst` is generalized and has a higher rank for the type
// `(List (Job a)) as a` - notice that only the recursion var `a` is active
// here, not the entire recursive tag union. In the body of this branch, `lst`
// becomes a type error, but the nested recursion var `a` is left untouched,
// because it is nested under the of `lst`, not the surface type that becomes
// an error.
//
// Had this not become a type error, `lst` would then be constrained against
// `job`, and its rank would get pulled back down. So, this can only happen in
// the presence of type errors.
//
// In all other cases, the recursion var has the same rank as the tag union itself
// all types it uses are also in the tags already, so it cannot influence the
// rank
if cfg!(debug_assertions) {
// rank.
if cfg!(debug_assertions)
&& !matches!(
subs.get_content_without_compacting(*rec_var),
Content::Error | Content::FlexVar(..)
)
{
let rec_var_rank =
adjust_rank(subs, young_mark, visit_mark, group_rank, *rec_var);
debug_assert!(
rank >= rec_var_rank,
"rank was {:?} but recursion var {:?} has higher rank {:?}",
"rank was {:?} but recursion var <{:?}>{:?} has higher rank {:?}",
rank,
rec_var,
subs.get_content_without_compacting(*rec_var),
rec_var_rank
);
}

View File

@ -19,7 +19,7 @@ mod solve_expr {
use roc_region::all::{LineColumn, LineColumnRegion, LineInfo, Region};
use roc_reporting::report::{can_problem, type_problem, RocDocAllocator};
use roc_solve::solve::TypeError;
use roc_types::pretty_print::{content_to_string, name_all_type_vars};
use roc_types::pretty_print::name_and_print_var;
use std::path::PathBuf;
// HELPERS
@ -172,18 +172,9 @@ mod solve_expr {
let subs = solved.inner_mut();
// name type vars
for var in exposed_to_host.values() {
name_all_type_vars(*var, subs);
}
let content = {
debug_assert!(exposed_to_host.len() == 1);
let (_symbol, variable) = exposed_to_host.into_iter().next().unwrap();
subs.get_content_without_compacting(variable)
};
let actual_str = content_to_string(content, subs, home, &interns);
debug_assert!(exposed_to_host.len() == 1);
let (_symbol, variable) = exposed_to_host.into_iter().next().unwrap();
let actual_str = name_and_print_var(variable, subs, home, &interns);
Ok((type_problems, can_problems, actual_str))
}
@ -282,9 +273,7 @@ mod solve_expr {
let var = find_type_at(region, &decls)
.expect(&format!("No type for {} ({:?})!", &text, region));
name_all_type_vars(var, subs);
let content = subs.get_content_without_compacting(var);
let actual_str = content_to_string(content, subs, home, &interns);
let actual_str = name_and_print_var(var, subs, home, &interns);
solved_queries.push(format!("{} : {}", text, actual_str));
}
@ -5445,26 +5434,19 @@ mod solve_expr {
}
#[test]
fn copy_vars_referencing_copied_vars_specialized() {
fn generalize_and_specialize_recursion_var() {
infer_eq_without_problem(
indoc!(
r#"
Job a : [ Job [ Command ] (Job a) (List (Job a)) a ]
Job a : [ Job (List (Job a)) a ]
job : Job Str
when job is
Job _ j lst _ ->
when j is
Job _ _ _ s ->
{ j, lst, s }
Job lst s -> P lst s
"#
),
// TODO: this means that we're doing our job correctly, as now both `Job a`s have been
// specialized to the same type, and the second destructuring proves the reified type
// is `Job Str`. But we should just print the structure of the recursive type directly.
// See https://github.com/rtfeldman/roc/issues/2513
"{ j : a, lst : List a, s : Str }",
"[ P (List [ Job (List a) Str ] as a) Str ]*",
)
}
@ -6384,4 +6366,25 @@ mod solve_expr {
],
)
}
#[test]
fn task_wildcard_wildcard() {
infer_eq_without_problem(
indoc!(
r#"
app "test" provides [ tforever ] to "./platform"
Effect a := {} -> a
eforever : Effect a -> Effect b
Task a err : Effect (Result a err)
tforever : Task val err -> Task * *
tforever = \task -> eforever task
"#
),
"Task val err -> Task * *",
);
}
}

View File

@ -3474,3 +3474,22 @@ fn list_map2_conslist() {
RocStr
)
}
#[test]
#[cfg(any(feature = "gen-llvm"))]
fn polymorphic_lambda_captures_polymorphic_value() {
assert_evals_to!(
indoc!(
r#"
x = 2
f1 = \_ -> x
f = if True then f1 else f1
f {}
"#
),
2,
u64
)
}

View File

@ -233,7 +233,11 @@ fn find_names_needed(
}
}
pub fn name_all_type_vars(variable: Variable, subs: &mut Subs) {
struct NamedResult {
recursion_structs_to_expand: Vec<Variable>,
}
fn name_all_type_vars(variable: Variable, subs: &mut Subs) -> NamedResult {
let mut roots = Vec::new();
let mut letters_used = 0;
let mut appearances = MutMap::default();
@ -242,13 +246,30 @@ pub fn name_all_type_vars(variable: Variable, subs: &mut Subs) {
// Populate names_needed
find_names_needed(variable, subs, &mut roots, &mut appearances, &mut taken);
let mut recursion_structs_to_expand = vec![];
for root in roots {
// show the type variable number instead of `*`. useful for debugging
// set_root_name(root, (format!("<{:?}>", root).into()), subs);
if let Some(Appearances::Multiple) = appearances.get(&root) {
letters_used = name_root(letters_used, root, subs, &mut taken);
match appearances.get(&root) {
Some(Appearances::Multiple) => {
letters_used = name_root(letters_used, root, subs, &mut taken);
}
Some(Appearances::Single) => {
if let Content::RecursionVar { structure, .. } =
subs.get_content_without_compacting(root)
{
recursion_structs_to_expand.push(*structure);
letters_used = name_root(letters_used, root, subs, &mut taken);
}
}
_ => {}
}
}
NamedResult {
recursion_structs_to_expand,
}
}
fn name_root(
@ -312,17 +333,22 @@ fn set_root_name(root: Variable, name: Lowercase, subs: &mut Subs) {
#[derive(Default)]
struct Context<'a> {
able_variables: Vec<(&'a str, Symbol)>,
recursion_structs_to_expand: Vec<Variable>,
}
pub fn content_to_string(
fn content_to_string(
content: &Content,
subs: &Subs,
home: ModuleId,
interns: &Interns,
named_result: NamedResult,
) -> String {
let mut buf = String::new();
let env = Env { home, interns };
let mut ctx = Context::default();
let mut ctx = Context {
able_variables: vec![],
recursion_structs_to_expand: named_result.recursion_structs_to_expand,
};
write_content(&env, &mut ctx, content, subs, &mut buf, Parens::Unnecessary);
@ -338,6 +364,17 @@ pub fn content_to_string(
buf
}
pub fn name_and_print_var(
var: Variable,
subs: &mut Subs,
home: ModuleId,
interns: &Interns,
) -> String {
let named_result = name_all_type_vars(var, subs);
let content = subs.get_content_without_compacting(var);
content_to_string(content, subs, home, interns, named_result)
}
pub fn get_single_arg<'a>(subs: &'a Subs, args: &'a AliasVariables) -> &'a Content {
debug_assert_eq!(args.len(), 1);
@ -381,12 +418,34 @@ fn write_content<'a>(
ctx.able_variables.push((name, *ability));
buf.push_str(name);
}
RecursionVar { opt_name, .. } => match opt_name {
RecursionVar {
opt_name,
structure,
} => match opt_name {
Some(name_index) => {
let name = &subs.field_names[name_index.index as usize];
buf.push_str(name.as_str())
if let Some(idx) = ctx
.recursion_structs_to_expand
.iter()
.position(|v| v == structure)
{
ctx.recursion_structs_to_expand.swap_remove(idx);
write_content(
env,
ctx,
subs.get_content_without_compacting(*structure),
subs,
buf,
parens,
);
} else {
let name = &subs.field_names[name_index.index as usize];
buf.push_str(name.as_str())
}
}
None => {
unreachable!("This should always be filled in!")
}
None => buf.push_str(WILDCARD),
},
Structure(flat_type) => write_flat_type(env, ctx, flat_type, subs, buf, parens),
Alias(symbol, args, _actual, _kind) => {

View File

@ -59,9 +59,10 @@ pub enum ErrorTypeContext {
struct ErrorTypeState {
taken: MutSet<Lowercase>,
normals: u32,
letters_used: u32,
problems: Vec<crate::types::Problem>,
context: ErrorTypeContext,
recursive_tag_unions_seen: Vec<Variable>,
}
#[derive(Clone, Copy, Debug)]
@ -1866,9 +1867,10 @@ impl Subs {
let mut state = ErrorTypeState {
taken,
normals: 0,
letters_used: 0,
problems: Vec::new(),
context,
recursive_tag_unions_seen: Vec::new(),
};
(var_to_err_type(self, &mut state, var), state.problems)
@ -2172,23 +2174,6 @@ impl Content {
Content::Structure(FlatType::Apply(Symbol::NUM_NUM, _))
)
}
#[cfg(debug_assertions)]
#[allow(dead_code)]
pub fn dbg(self, subs: &Subs) -> Self {
let home = roc_module::symbol::ModuleIds::default().get_or_insert(&"#Dbg".into());
let interns = roc_module::symbol::Interns {
all_ident_ids: roc_module::symbol::IdentIds::exposed_builtins(0),
..Default::default()
};
eprintln!(
"{}",
crate::pretty_print::content_to_string(&self, subs, home, &interns)
);
self
}
}
#[derive(Clone, Copy, Debug)]
@ -3355,7 +3340,10 @@ fn content_to_err_type(
ErrorType::RigidAbleVar(name, ability)
}
RecursionVar { opt_name, .. } => {
RecursionVar {
opt_name,
structure,
} => {
let name = match opt_name {
Some(name_index) => subs.field_names[name_index.index as usize].clone(),
None => {
@ -3368,7 +3356,11 @@ fn content_to_err_type(
}
};
ErrorType::FlexVar(name)
if state.recursive_tag_unions_seen.contains(&var) {
ErrorType::FlexVar(name)
} else {
var_to_err_type(subs, state, structure)
}
}
Alias(symbol, args, aliased_to, kind) => {
@ -3549,6 +3541,8 @@ fn flat_type_to_err_type(
}
RecursiveTagUnion(rec_var, tags, ext_var) => {
state.recursive_tag_unions_seen.push(rec_var);
let mut err_tags = SendMap::default();
for (name_index, slice_index) in tags.iter_all() {
@ -3599,11 +3593,12 @@ fn flat_type_to_err_type(
}
fn get_fresh_var_name(state: &mut ErrorTypeState) -> Lowercase {
let (name, new_index) = name_type_var(state.normals, &mut state.taken.iter(), |var, str| {
var.as_str() == str
});
let (name, new_index) =
name_type_var(state.letters_used, &mut state.taken.iter(), |var, str| {
var.as_str() == str
});
state.normals = new_index;
state.letters_used = new_index;
state.taken.insert(name.clone());

View File

@ -1358,7 +1358,17 @@ fn maybe_mark_tag_union_recursive(subs: &mut Subs, tag_union_var: Variable) {
}
}
panic!("recursive loop does not contain a tag union")
// Might not be any tag union if we only pass through `Apply`s. Otherwise, we have a bug!
if chain.iter().all(|&v| {
matches!(
subs.get_content_without_compacting(v),
Content::Structure(FlatType::Apply(..))
)
}) {
return;
} else {
internal_error!("recursive loop does not contain a tag union")
}
}
}
}

View File

@ -59,9 +59,9 @@ use roc_collections::all::MutMap;
use roc_module::ident::Lowercase;
use roc_module::symbol::Symbol;
use roc_region::all::Region;
use roc_types::pretty_print::name_and_print_var;
use roc_types::solved_types::Solved;
use roc_types::subs::{Subs, Variable};
use roc_types::{pretty_print::content_to_string, subs::VarStore};
use roc_types::subs::{Subs, VarStore, Variable};
use snafu::OptionExt;
use threadpool::ThreadPool;
use winit::event::VirtualKeyCode;
@ -462,17 +462,10 @@ impl<'a> EdModel<'a> {
let subs = solved.inner_mut();
let content = subs.get_content_without_compacting(var);
let pretty_var =
name_and_print_var(var, subs, self.module.env.home, &self.loaded_module.interns);
PoolStr::new(
&content_to_string(
content,
subs,
self.module.env.home,
&self.loaded_module.interns,
),
self.module.env.pool,
)
PoolStr::new(&pretty_var, self.module.env.pool)
}
fn run_solve(

View File

@ -23,7 +23,7 @@ use roc_repl_eval::{ReplApp, ReplAppMemory};
use roc_reporting::report::DEFAULT_PALETTE;
use roc_std::RocStr;
use roc_target::TargetInfo;
use roc_types::pretty_print::{content_to_string, name_all_type_vars};
use roc_types::pretty_print::name_and_print_var;
const BLUE: &str = "\u{001b}[36m";
const PINK: &str = "\u{001b}[35m";
@ -227,9 +227,8 @@ fn gen_and_eval_llvm<'a>(
let main_fn_var = *main_fn_var;
// pretty-print the expr type string for later.
name_all_type_vars(main_fn_var, &mut subs);
let expr_type_str = name_and_print_var(main_fn_var, &mut subs, home, &interns);
let content = subs.get_content_without_compacting(main_fn_var);
let expr_type_str = content_to_string(content, &subs, home, &interns);
let (_, main_fn_layout) = match procedures.keys().find(|(s, _)| *s == main_fn_symbol) {
Some(layout) => *layout,

View File

@ -12,7 +12,7 @@ use roc_repl_eval::{
};
use roc_reporting::report::DEFAULT_PALETTE_HTML;
use roc_target::TargetInfo;
use roc_types::pretty_print::{content_to_string, name_all_type_vars};
use roc_types::pretty_print::name_and_print_var;
use crate::{js_create_app, js_get_result_and_memory, js_run_app};
@ -184,9 +184,8 @@ pub async fn entrypoint_from_js(src: String) -> Result<String, String> {
let main_fn_var = *main_fn_var;
// pretty-print the expr type string for later.
name_all_type_vars(main_fn_var, &mut subs);
let expr_type_str = name_and_print_var(main_fn_var, &mut subs, module_id, &interns);
let content = subs.get_content_without_compacting(main_fn_var);
let expr_type_str = content_to_string(content, &subs, module_id, &interns);
let (_, main_fn_layout) = match procedures.keys().find(|(s, _)| *s == main_fn_symbol) {
Some(layout) => *layout,

View File

@ -22,7 +22,6 @@ mod test_reporting {
use roc_reporting::report::{RocDocAllocator, RocDocBuilder};
use roc_solve::solve;
use roc_test_utils::assert_multiline_str_eq;
use roc_types::pretty_print::name_all_type_vars;
use roc_types::subs::Subs;
use std::path::PathBuf;
@ -122,20 +121,12 @@ mod test_reporting {
mut can_problems,
mut type_problems,
interns,
mut solved,
exposed_to_host,
..
} = result?;
let can_problems = can_problems.remove(&home).unwrap_or_default();
let type_problems = type_problems.remove(&home).unwrap_or_default();
let subs = solved.inner_mut();
for var in exposed_to_host.values() {
name_all_type_vars(*var, subs);
}
Ok((module_src, type_problems, can_problems, home, interns))
}
@ -233,7 +224,7 @@ mod test_reporting {
let mut unify_problems = Vec::new();
let mut abilities_store = AbilitiesStore::default();
let (_content, mut subs) = infer_expr(
let (_content, _subs) = infer_expr(
subs,
&mut unify_problems,
&constraints,
@ -243,8 +234,6 @@ mod test_reporting {
var,
);
name_all_type_vars(var, &mut subs);
Ok((unify_problems, can_problems, home, interns))
}
@ -3520,8 +3509,9 @@ mod test_reporting {
This `ACons` tag application has the type:
[ ACons (Num (Integer Signed64)) [
BCons (Num (Integer Signed64)) [ ACons Str [ BCons I64 a, BNil ],
ANil ], BNil ], ANil ]
BCons (Num (Integer Signed64)) [ ACons Str [ BCons I64 [
ACons I64 (BList I64 I64), ANil ] as , BNil ], ANil ], BNil ],
ANil ]
But the type annotation on `x` says it should be:
@ -9796,4 +9786,124 @@ I need all branches in an `if` to have the same type!
),
)
}
#[test]
fn recursion_var_specialization_error() {
new_report_problem_as(
"recursion_var_specialization_error",
indoc!(
r#"
Job a : [ Job (List (Job a)) ]
job : Job Str
when job is
Job lst -> lst == ""
"#
),
indoc!(
r#"
TYPE MISMATCH /code/proj/Main.roc
The 2nd argument to `isEq` is not what I expect:
9 Job lst -> lst == ""
^^
This argument is a string of type:
Str
But `isEq` needs the 2nd argument to be:
List [ Job ] as
"#
),
)
}
#[test]
fn type_error_in_apply_is_circular() {
new_report_problem_as(
"type_error_in_apply_is_circular",
indoc!(
r#"
app "test" provides [ go ] to "./platform"
S a : { set : Set a }
go : a, S a -> Result (List a) *
go = \goal, model ->
if goal == goal
then Ok []
else
new = { model & set : Set.remove goal model.set }
go goal new
"#
),
indoc!(
r#"
TYPE MISMATCH /code/proj/Main.roc
The 1st argument to `remove` is not what I expect:
10 new = { model & set : Set.remove goal model.set }
^^^^
This `goal` value is a:
a
But `remove` needs the 1st argument to be:
Set a
Tip: The type annotation uses the type variable `a` to say that this
definition can produce any type of value. But in the body I see that
it will only produce a `Set` value of a single specific type. Maybe
change the type annotation to be more specific? Maybe change the code
to be more general?
CIRCULAR TYPE /code/proj/Main.roc
I'm inferring a weird self-referential type for `new`:
10 new = { model & set : Set.remove goal model.set }
^^^
Here is my best effort at writing down the type. You will see for
parts of the type that repeat something already printed out
infinitely.
{ set : Set }
CIRCULAR TYPE /code/proj/Main.roc
I'm inferring a weird self-referential type for `model`:
6 go = \goal, model ->
^^^^^
Here is my best effort at writing down the type. You will see for
parts of the type that repeat something already printed out
infinitely.
S (Set )
CIRCULAR TYPE /code/proj/Main.roc
I'm inferring a weird self-referential type for `goal`:
6 go = \goal, model ->
^^^^
Here is my best effort at writing down the type. You will see for
parts of the type that repeat something already printed out
infinitely.
Set
"#
),
)
}
}