Move unexpected params warning to solve

This commit is contained in:
Agus Zubiaga 2024-07-06 21:36:26 -03:00
parent 63e722f61d
commit 0cbb352a89
No known key found for this signature in database
18 changed files with 115 additions and 169 deletions

View file

@ -297,12 +297,13 @@ fn deep_copy_expr_help<C: CopyEnv>(env: &mut C, copied: &mut Vec<Variable>, expr
params: *params,
var: sub!(*var),
},
ImportParams(loc_expr, var, module_id) => ImportParams(
Box::new(loc_expr.map(|e| go_help!(e))),
sub!(*var),
ImportParams(module_id, region, opt_provided) => ImportParams(
*module_id,
*region,
opt_provided
.as_ref()
.map(|(var, expr)| (sub!(*var), Box::new(go_help!(&expr)))),
),
MissingImportParams(module_id, region) => MissingImportParams(*module_id, *region),
&AbilityMember(sym, specialization, specialization_var) => {
AbilityMember(sym, specialization, sub!(specialization_var))
}

View file

@ -209,9 +209,9 @@ fn expr<'a>(c: &Ctx, p: EPrec, f: &'a Arena<'a>, e: &'a Expr) -> DocBuilder<'a,
Var(sym, _) | ParamsVar { symbol: sym, .. } | AbilityMember(sym, _, _) => {
pp_sym(c, f, *sym)
}
ImportParams(loc_expr, _, _) => expr(c, p, f, &loc_expr.value),
MissingImportParams(module_id, _) => {
text!(f, "<missing params for {:?}>", module_id)
ImportParams(_, _, Some((_, params_expr))) => expr(c, p, f, params_expr),
ImportParams(module_id, _, None) => {
text!(f, "<no params for {:?}>", module_id)
}
When {
loc_cond, branches, ..

View file

@ -1153,21 +1153,12 @@ fn canonicalize_value_defs<'a>(
exposed_symbols,
});
match params {
PendingModuleImportParams::None => {}
PendingModuleImportParams::Required {
symbol,
loc_pattern,
opt_provided,
} => {
pending_value_defs.push(PendingValueDef::ImportParams {
symbol,
loc_pattern,
module_id,
opt_provided,
});
}
}
pending_value_defs.push(PendingValueDef::ImportParams {
symbol: params.symbol,
loc_pattern: params.loc_pattern,
opt_provided: params.opt_provided,
module_id,
});
}
PendingValue::InvalidIngestedFile => { /* skip */ }
PendingValue::ImportNameConflict => { /* skip */ }
@ -2421,7 +2412,7 @@ fn canonicalize_pending_value_def<'a>(
.references
.insert_value_lookup(symbol, QualifiedReference::Unqualified);
let (expr, references) = match opt_provided {
let (opt_var_record, references) = match opt_provided {
Some(params) => {
let (record, can_output) =
canonicalize_record(env, var_store, scope, loc_pattern.region, params);
@ -2429,20 +2420,15 @@ fn canonicalize_pending_value_def<'a>(
let references = can_output.references.clone();
output.union(can_output);
let loc_record = Loc::at(loc_pattern.region, record);
let expr =
Expr::ImportParams(Box::new(loc_record), var_store.fresh(), module_id);
(expr, references)
(Some((var_store.fresh(), Box::new(record))), references)
}
None => (
Expr::MissingImportParams(module_id, loc_pattern.region),
References::new(),
),
None => (None, References::new()),
};
let loc_expr = Loc::at(loc_pattern.region, expr);
let loc_expr = Loc::at(
loc_pattern.region,
Expr::ImportParams(module_id, loc_pattern.region, opt_var_record),
);
let def = single_can_def(
loc_pattern,
@ -3017,15 +3003,10 @@ struct PendingModuleImport<'a> {
params: PendingModuleImportParams<'a>,
}
enum PendingModuleImportParams<'a> {
/// The module does not require any params
None,
/// The module requires params, they may or may not have been provided
Required {
symbol: Symbol,
loc_pattern: Loc<Pattern>,
opt_provided: Option<ast::Collection<'a, Loc<AssignedField<'a, ast::Expr<'a>>>>>,
},
struct PendingModuleImportParams<'a> {
symbol: Symbol,
loc_pattern: Loc<Pattern>,
opt_provided: Option<ast::Collection<'a, Loc<AssignedField<'a, ast::Expr<'a>>>>>,
}
pub struct IntroducedImport {
@ -3178,45 +3159,27 @@ fn to_pending_value_def<'a>(
None => module_name.clone(),
};
let params = if env.modules_expecting_params.contains(&module_id) {
let name_str = name_with_alias.as_str();
let sym_region = module_import.params.map(|p| p.params.region).unwrap_or(region);
match scope.introduce_str(format!("#{name_str}").as_str(), sym_region) {
Ok(symbol) => {
let loc_pattern = Loc::at(sym_region, Pattern::Identifier(symbol));
PendingModuleImportParams::Required {
symbol,
loc_pattern,
opt_provided: module_import.params.map(|p| p.params.value),
}
},
Err(_) => {
// Ignore conflict, it will be handled as a duplicate import
PendingModuleImportParams::None
},
}
// Generate a symbol for the module params def
// We do this even if params weren't provided so that solve can report if they are missing
let params_sym = scope.gen_unique_symbol();
let params_region = module_import.params.map(|p| p.params.region).unwrap_or(region);
let params =
PendingModuleImportParams {
symbol: params_sym,
loc_pattern: Loc::at(params_region, Pattern::Identifier(params_sym)),
opt_provided: module_import.params.map(|p| p.params.value),
};
let provided_params_sym = if module_import.params.is_some() {
// Only add params to scope if they are provided
Some(params_sym)
} else {
if let Some(params) = module_import.params {
env.problems.push(Problem::UnexpectedParams {
module_id,
region: params.params.region,
});
}
PendingModuleImportParams::None
};
let params_sym = match params {
PendingModuleImportParams::Required { symbol, .. } => Some(symbol),
PendingModuleImportParams::None => None,
None
};
if let Err(existing_import) =
scope
.modules
.insert(name_with_alias.clone(), module_id, params_sym, region)
.insert(name_with_alias.clone(), module_id, provided_params_sym, region)
{
env.problems.push(Problem::ImportNameConflict {
name: name_with_alias,

View file

@ -21,8 +21,6 @@ pub struct Env<'a> {
pub qualified_module_ids: &'a PackageModuleIds<'a>,
pub modules_expecting_params: VecSet<ModuleId>,
/// Problems we've encountered along the way, which will be reported to the user at the end.
pub problems: Vec<Problem>,
@ -51,7 +49,6 @@ impl<'a> Env<'a> {
home: ModuleId,
module_path: &'a Path,
dep_idents: &'a IdentIdsByModule,
modules_expecting_params: VecSet<ModuleId>,
qualified_module_ids: &'a PackageModuleIds<'a>,
opt_shorthand: Option<&'a str>,
) -> Env<'a> {
@ -60,7 +57,6 @@ impl<'a> Env<'a> {
home,
module_path,
dep_idents,
modules_expecting_params,
qualified_module_ids,
problems: Vec::new(),
closures: MutMap::default(),

View file

@ -185,10 +185,7 @@ pub enum Expr {
},
/// Module params expression in import
ImportParams(Box<Loc<Expr>>, Variable, ModuleId),
/// The imported module requires params but none were provided
/// We delay this error until solve, so we can report the expected type
MissingImportParams(ModuleId, Region),
ImportParams(ModuleId, Region, Option<(Variable, Box<Expr>)>),
/// The "crash" keyword
Crash {
@ -342,8 +339,8 @@ impl Expr {
Self::RecordAccessor(data) => Category::Accessor(data.field.clone()),
Self::TupleAccess { index, .. } => Category::TupleAccess(*index),
Self::RecordUpdate { .. } => Category::Record,
Self::ImportParams(loc_expr, _, _) => loc_expr.value.category(),
Self::MissingImportParams(_, _) => Category::Unknown,
Self::ImportParams(_, _, Some((_, expr))) => expr.category(),
Self::ImportParams(_, _, None) => Category::Unknown,
Self::Tag {
name, arguments, ..
} => Category::TagApply {
@ -1976,7 +1973,6 @@ pub fn inline_calls(var_store: &mut VarStore, expr: Expr) -> Expr {
| other @ TypedHole { .. }
| other @ ForeignCall { .. }
| other @ OpaqueWrapFunction(_)
| other @ MissingImportParams(_, _)
| other @ Crash { .. } => other,
List {
@ -2235,10 +2231,13 @@ pub fn inline_calls(var_store: &mut VarStore, expr: Expr) -> Expr {
);
}
ImportParams(loc_expr, var, module_id) => {
let loc_expr = Loc::at(loc_expr.region, inline_calls(var_store, loc_expr.value));
ImportParams(Box::new(loc_expr), var, module_id)
}
ImportParams(module_id, region, Some((var, expr))) => ImportParams(
module_id,
region,
Some((var, Box::new(inline_calls(var_store, *expr)))),
),
ImportParams(module_id, region, None) => ImportParams(module_id, region, None),
RecordAccess {
record_var,
@ -3252,8 +3251,8 @@ pub(crate) fn get_lookup_symbols(expr: &Expr) -> Vec<ExpectLookup> {
Expr::Tuple { elems, .. } => {
stack.extend(elems.iter().map(|(_, elem)| &elem.value));
}
Expr::ImportParams(loc_expr, _, _) => {
stack.push(&loc_expr.value);
Expr::ImportParams(_, _, Some((_, expr))) => {
stack.push(expr);
}
Expr::Expect {
loc_continuation, ..
@ -3281,7 +3280,7 @@ pub(crate) fn get_lookup_symbols(expr: &Expr) -> Vec<ExpectLookup> {
| Expr::EmptyRecord
| Expr::TypedHole(_)
| Expr::RuntimeError(_)
| Expr::MissingImportParams(_, _)
| Expr::ImportParams(_, _, None)
| Expr::OpaqueWrapFunction(_) => {}
}
}

View file

@ -286,7 +286,6 @@ pub fn canonicalize_module_defs<'a>(
qualified_module_ids: &'a PackageModuleIds<'a>,
exposed_ident_ids: IdentIds,
dep_idents: &'a IdentIdsByModule,
modules_expecting_params: VecSet<ModuleId>,
aliases: MutMap<Symbol, Alias>,
imported_abilities_state: PendingAbilitiesStore,
initial_scope: MutMap<Ident, (Symbol, Region)>,
@ -312,7 +311,6 @@ pub fn canonicalize_module_defs<'a>(
home,
arena.alloc(Path::new(module_path)),
dep_idents,
modules_expecting_params,
qualified_module_ids,
opt_shorthand,
);
@ -1153,7 +1151,6 @@ fn fix_values_captured_in_closure_expr(
| TypedHole { .. }
| RuntimeError(_)
| ZeroArgumentTag { .. }
| MissingImportParams(_, _)
| RecordAccessor { .. } => {}
List { loc_elems, .. } => {
@ -1260,14 +1257,12 @@ fn fix_values_captured_in_closure_expr(
}
}
ImportParams(loc_expr, _, _) => {
fix_values_captured_in_closure_expr(
&mut loc_expr.value,
no_capture_symbols,
closure_captures,
);
ImportParams(_, _, Some((_, expr))) => {
fix_values_captured_in_closure_expr(expr, no_capture_symbols, closure_captures);
}
ImportParams(_, _, None) => {}
Tuple { elems, .. } => {
for (_var, expr) in elems.iter_mut() {
fix_values_captured_in_closure_expr(

View file

@ -318,7 +318,8 @@ pub fn walk_expr<V: Visitor>(visitor: &mut V, expr: &Expr, var: Variable) {
.iter()
.for_each(|(var, elem)| visitor.visit_expr(&elem.value, elem.region, *var)),
Expr::EmptyRecord => { /* terminal */ }
Expr::ImportParams(expr, _, _) => visitor.visit_expr(&expr.value, expr.region, var),
Expr::ImportParams(_, region, Some((_, expr))) => visitor.visit_expr(expr, *region, var),
Expr::ImportParams(_, _, None) => { /* terminal */ }
Expr::RecordAccess {
field_var,
loc_expr,
@ -404,7 +405,6 @@ pub fn walk_expr<V: Visitor>(visitor: &mut V, expr: &Expr, var: Variable) {
}
Expr::TypedHole(_) => { /* terminal */ }
Expr::RuntimeError(..) => { /* terminal */ }
Expr::MissingImportParams(..) => { /* terminal */ }
}
}

View file

@ -80,13 +80,11 @@ pub fn can_expr_with(arena: &Bump, home: ModuleId, expr_str: &str) -> CanExprOut
);
let dep_idents = IdentIds::exposed_builtins(0);
let modules_expecting_params = Default::default();
let mut env = Env::new(
arena,
home,
Path::new("Test.roc"),
&dep_idents,
modules_expecting_params,
&qualified_module_ids,
None,
);

View file

@ -580,26 +580,20 @@ pub fn constrain_expr(
constraints.and_constraint([store_expected, lookup_constr])
}
ImportParams(params, var, module_id) => {
ImportParams(module_id, region, Some((var, params))) => {
let index = constraints.push_variable(*var);
let expected_params = constraints.push_expected_type(Expected::ForReason(
Reason::ImportParams(*module_id),
index,
params.region,
*region,
));
let expr_con = constrain_expr(
types,
constraints,
env,
params.region,
&params.value,
expected_params,
);
let params_con = constraints.import_params(Some(index), *module_id, params.region);
let expr_con =
constrain_expr(types, constraints, env, *region, params, expected_params);
let params_con = constraints.import_params(Some(index), *module_id, *region);
let expr_and_params = constraints.and_constraint([expr_con, params_con]);
constraints.exists([*var], expr_and_params)
}
MissingImportParams(module_id, region) => {
ImportParams(module_id, region, None) => {
constraints.import_params(None, *module_id, *region)
}
&AbilityMember(symbol, specialization_id, specialization_var) => {
@ -4116,7 +4110,8 @@ fn is_generalizable_expr(mut expr: &Expr) -> bool {
return true;
}
OpaqueRef { argument, .. } => expr = &argument.1.value,
ImportParams(loc_expr, _, _) => expr = &loc_expr.value,
ImportParams(_, _, Some((_, params))) => expr = params,
ImportParams(_, _, None) => return false,
Str(_)
| IngestedFile(..)
| List { .. }
@ -4139,7 +4134,6 @@ fn is_generalizable_expr(mut expr: &Expr) -> bool {
| ExpectFx { .. }
| Dbg { .. }
| TypedHole(_)
| MissingImportParams(_, _)
| RuntimeError(..)
| ZeroArgumentTag { .. }
| Tag { .. }

View file

@ -189,7 +189,6 @@ pub fn can_expr_with<'a>(
home,
Path::new("Test.roc"),
&dep_idents,
Default::default(),
&module_ids,
None,
);

View file

@ -250,7 +250,6 @@ fn start_phase<'a>(
.clone();
let mut aliases = MutMap::default();
let mut modules_expecting_params = VecSet::default();
let mut abilities_store = PendingAbilitiesStore::default();
for imported in parsed.available_modules.keys() {
@ -294,10 +293,6 @@ fn start_phase<'a>(
.union(import_store.closure_from_imported(exposed_symbols));
}
}
if state.module_cache.param_patterns.contains_key(imported) {
modules_expecting_params.insert(*imported);
}
}
let skip_constraint_gen = {
@ -309,7 +304,6 @@ fn start_phase<'a>(
BuildTask::CanonicalizeAndConstrain {
parsed,
dep_idents,
modules_expecting_params,
exposed_symbols,
qualified_module_ids,
aliases,
@ -889,7 +883,6 @@ enum BuildTask<'a> {
CanonicalizeAndConstrain {
parsed: ParsedModule<'a>,
qualified_module_ids: PackageModuleIds<'a>,
modules_expecting_params: VecSet<ModuleId>,
dep_idents: IdentIdsByModule,
exposed_symbols: VecSet<Symbol>,
aliases: MutMap<Symbol, Alias>,
@ -5001,7 +4994,6 @@ fn canonicalize_and_constrain<'a>(
arena: &'a Bump,
qualified_module_ids: &'a PackageModuleIds<'a>,
dep_idents: IdentIdsByModule,
modules_expecting_params: VecSet<ModuleId>,
exposed_symbols: VecSet<Symbol>,
aliases: MutMap<Symbol, Alias>,
imported_abilities_state: PendingAbilitiesStore,
@ -5044,7 +5036,6 @@ fn canonicalize_and_constrain<'a>(
qualified_module_ids,
exposed_ident_ids,
&dep_idents,
modules_expecting_params,
aliases,
imported_abilities_state,
initial_scope,
@ -6207,7 +6198,6 @@ fn run_task<'a>(
parsed,
qualified_module_ids,
dep_idents,
modules_expecting_params,
exposed_symbols,
aliases,
abilities_store,
@ -6218,7 +6208,6 @@ fn run_task<'a>(
arena,
&qualified_module_ids,
dep_idents,
modules_expecting_params,
exposed_symbols,
aliases,
abilities_store,

View file

@ -315,6 +315,13 @@ fn expect_types(mut loaded_module: LoadedModule, mut expected_types: HashMap<&st
match declarations.declarations[index] {
Value | Function(_) | Recursive(_) | TailRecursive(_) => {
let body = declarations.expressions[index].clone();
if let roc_can::expr::Expr::ImportParams(_, _, None) = body.value {
// Skip import defs without params
continue;
}
let symbol = declarations.symbols[index].value;
let expr_var = declarations.variables[index];
@ -443,7 +450,13 @@ fn module_with_deps() {
match declarations.declarations[index] {
Value | Function(_) | Recursive(_) | TailRecursive(_) => {
def_count += 1;
let body = declarations.expressions[index].clone();
if let roc_can::expr::Expr::ImportParams(_, _, None) = body.value {
// Skip import defs without params
} else {
def_count += 1;
}
}
Destructure(_) => {
def_count += 1;

View file

@ -4425,15 +4425,12 @@ pub fn with_hole<'a>(
ParamsVar { .. } => {
unimplemented!("module params code generation")
}
ImportParams(loc_expr, _, _) => with_hole(
env,
loc_expr.value,
variable,
procs,
layout_cache,
assigned,
hole,
),
ImportParams(_, _, Some((_, value))) => {
with_hole(env, *value, variable, procs, layout_cache, assigned, hole)
}
ImportParams(_, _, None) => {
internal_error!("Missing module params should've been dropped by now");
}
AbilityMember(member, specialization_id, specialization_var) => {
let specialization_symbol = late_resolve_ability_specialization(
env,
@ -5883,7 +5880,6 @@ pub fn with_hole<'a>(
}
TypedHole(_) => runtime_error(env, "Hit a blank"),
RuntimeError(e) => runtime_error(env, env.arena.alloc(e.runtime_message())),
MissingImportParams(_, _) => runtime_error(env, env.arena.alloc("Missing import params")),
Crash { msg, ret_var: _ } => {
let msg_sym = possible_reuse_symbol_or_specialize(
env,

View file

@ -236,10 +236,6 @@ pub enum Problem {
one_occurrence: Region,
kind: AliasKind,
},
UnexpectedParams {
module_id: ModuleId,
region: Region,
},
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
@ -319,7 +315,6 @@ impl Problem {
Problem::OverAppliedCrash { .. } => RuntimeError,
Problem::DefsOnlyUsedInRecursion(_, _) => Warning,
Problem::FileProblem { .. } => Fatal,
Problem::UnexpectedParams { .. } => Warning,
}
}
@ -476,8 +471,7 @@ impl Problem {
| Problem::UnnecessaryOutputWildcard { region }
| Problem::OverAppliedCrash { region }
| Problem::UnappliedCrash { region }
| Problem::DefsOnlyUsedInRecursion(_, region)
| Problem::UnexpectedParams { region, .. } => Some(*region),
| Problem::DefsOnlyUsedInRecursion(_, region) => Some(*region),
Problem::RuntimeError(RuntimeError::CircularDef(cycle_entries))
| Problem::BadRecursion(cycle_entries) => {
cycle_entries.first().map(|entry| entry.expr_region)

View file

@ -1472,11 +1472,12 @@ fn solve(
state
}
(None, Some(_)) | (None, None) => {
// Module does not expect params.
// If provided still, canonicalization will produce a warning.
(None, Some(_)) => {
problems.push(TypeError::UnexpectedImportParams(*region, *module_id));
state
}
(None, None) => state,
}
}
};

View file

@ -36,6 +36,7 @@ pub enum TypeError {
},
IngestedFileBadUtf8(Box<PathBuf>, Utf8Error),
IngestedFileUnsupportedType(Box<PathBuf>, ErrorType),
UnexpectedImportParams(Region, ModuleId),
MissingImportParams(Region, ModuleId, ErrorType),
ImportParamsMismatch(Region, ModuleId, ErrorType, ErrorType),
}
@ -57,6 +58,7 @@ impl TypeError {
TypeError::Exhaustive(exhtv) => exhtv.severity(),
TypeError::StructuralSpecialization { .. } => RuntimeError,
TypeError::WrongSpecialization { .. } => RuntimeError,
TypeError::UnexpectedImportParams(..) => Warning,
TypeError::MissingImportParams(..) => RuntimeError,
TypeError::ImportParamsMismatch(..) => RuntimeError,
TypeError::IngestedFileBadUtf8(..) => Fatal,
@ -74,6 +76,7 @@ impl TypeError {
| TypeError::StructuralSpecialization { region, .. }
| TypeError::WrongSpecialization { region, .. }
| TypeError::BadPatternMissingAbility(region, ..)
| TypeError::UnexpectedImportParams(region, ..)
| TypeError::MissingImportParams(region, ..)
| TypeError::ImportParamsMismatch(region, ..) => Some(*region),
TypeError::UnfulfilledAbility(ab, ..) => ab.region(),

View file

@ -64,7 +64,6 @@ const ABILITY_IMPLEMENTATION_NOT_IDENTIFIER: &str = "ABILITY IMPLEMENTATION NOT
const DUPLICATE_IMPLEMENTATION: &str = "DUPLICATE IMPLEMENTATION";
const UNNECESSARY_IMPLEMENTATIONS: &str = "UNNECESSARY IMPLEMENTATIONS";
const INCOMPLETE_ABILITY_IMPLEMENTATION: &str = "INCOMPLETE ABILITY IMPLEMENTATION";
const UNEXPECTED_PARAMS: &str = "UNEXPECTED PARAMS";
pub fn can_problem<'b>(
alloc: &'b RocDocAllocator<'b>,
@ -1313,20 +1312,6 @@ pub fn can_problem<'b>(
doc = report.doc;
title = report.title;
}
Problem::UnexpectedParams { module_id, region } => {
doc = alloc.stack([
alloc.reflow("This import specifies module params:"),
alloc.region(lines.convert_region(region), severity),
alloc.concat([
alloc.reflow("However, "),
alloc.module(module_id),
alloc.reflow(
" does not expect any. Did you intend to import a different module?",
),
]),
]);
title = UNEXPECTED_PARAMS.to_string();
}
};
Report {

View file

@ -247,6 +247,26 @@ pub fn type_problem<'b>(
severity,
})
}
UnexpectedImportParams(region, module_id) => {
let stack = [
alloc.reflow("This import specifies module params:"),
alloc.region(lines.convert_region(region), severity),
alloc.concat([
alloc.reflow("However, "),
alloc.module(module_id),
alloc.reflow(
" does not expect any. Did you intend to import a different module?",
),
]),
];
Some(Report {
title: "UNEXPECTED PARAMS".to_string(),
filename,
doc: alloc.stack(stack),
severity,
})
}
MissingImportParams(region, module_id, expected) => {
let stack = [
alloc.reflow("This import specifies no module params:"),