Address PR comments, add syntax tests

This commit is contained in:
Sam Mohr 2024-10-21 03:06:43 -07:00
parent b3e60f9d3a
commit 7518a2c5ab
No known key found for this signature in database
GPG key ID: EA41D161A3C1BC99
28 changed files with 772 additions and 34 deletions

View file

@ -2640,7 +2640,13 @@ fn canonicalize_pending_body<'a>(
// The closure is self tail recursive iff it tail calls itself (by defined name).
let is_recursive = match can_output.tail_call {
Some(tail_symbol) if tail_symbol == *defined_symbol => Recursive::TailRecursive,
Some(tail_symbol) if tail_symbol == *defined_symbol => {
if closure_data.early_returns.is_empty() {
Recursive::TailRecursive
} else {
Recursive::Recursive
}
}
_ => Recursive::NotRecursive,
};

View file

@ -1017,7 +1017,7 @@ pub fn canonicalize_expr<'a>(
}
ast::Expr::Defs(loc_defs, loc_ret) => {
// The body expression gets a new scope for canonicalization,
scope.inner_scope(false, |inner_scope| {
scope.inner_def_scope(|inner_scope| {
let defs: Defs = (*loc_defs).clone();
can_defs_with_return(env, var_store, inner_scope, env.arena.alloc(defs), loc_ret)
})
@ -1049,17 +1049,16 @@ pub fn canonicalize_expr<'a>(
let mut can_branches = Vec::with_capacity(branches.len());
for branch in branches.iter() {
let (can_when_branch, branch_references) =
scope.inner_scope(false, |inner_scope| {
canonicalize_when_branch(
env,
var_store,
inner_scope,
region,
branch,
&mut output,
)
});
let (can_when_branch, branch_references) = scope.inner_def_scope(|inner_scope| {
canonicalize_when_branch(
env,
var_store,
inner_scope,
region,
branch,
&mut output,
)
});
output.references.union_mut(&branch_references);
@ -1534,7 +1533,7 @@ pub fn canonicalize_closure<'a>(
loc_body_expr: &'a Loc<ast::Expr<'a>>,
opt_def_name: Option<Symbol>,
) -> (ClosureData, Output) {
scope.inner_scope(true, |inner_scope| {
scope.inner_function_scope(|inner_scope| {
canonicalize_closure_body(
env,
var_store,

View file

@ -432,7 +432,8 @@ impl Scope {
self.aliases.contains_key(&name)
}
pub fn inner_scope<F, T>(&mut self, entering_function: bool, f: F) -> T
/// Enter an inner scope within a definition, e.g. a def or when block.
pub fn inner_def_scope<F, T>(&mut self, f: F) -> T
where
F: FnOnce(&mut Scope) -> T,
{
@ -449,11 +450,6 @@ impl Scope {
let locals_snapshot = self.locals.in_scope.len();
let imported_symbols_snapshot = self.imported_symbols.len();
let imported_modules_snapshot = self.modules.len();
let early_returns_snapshot = if entering_function {
std::mem::replace(&mut self.early_returns, Vec::new())
} else {
Vec::new()
};
let result = f(self);
@ -461,9 +457,6 @@ impl Scope {
self.ignored_locals.truncate(ignored_locals_count);
self.imported_symbols.truncate(imported_symbols_snapshot);
self.modules.truncate(imported_modules_snapshot);
if entering_function {
self.early_returns = early_returns_snapshot;
}
// anything added in the inner scope is no longer in scope now
for i in locals_snapshot..self.locals.in_scope.len() {
@ -473,6 +466,20 @@ impl Scope {
result
}
/// Enter an inner scope within a child function, e.g. a closure body.
pub fn inner_function_scope<F, T>(&mut self, f: F) -> T
where
F: FnOnce(&mut Scope) -> T,
{
let early_returns_snapshot = std::mem::replace(&mut self.early_returns, Vec::new());
let result = self.inner_def_scope(f);
self.early_returns = early_returns_snapshot;
result
}
pub fn register_debug_idents(&self) {
self.home.register_debug_idents(&self.locals.ident_ids)
}
@ -879,7 +886,7 @@ mod test {
}
#[test]
fn inner_scope_does_not_influence_outer() {
fn inner_def_scope_does_not_influence_outer() {
let _register_module_debug_names = ModuleIds::default();
let mut scope = Scope::new(
ModuleId::ATTR,
@ -893,7 +900,7 @@ mod test {
assert!(scope.lookup(&ident, region).is_err());
scope.inner_scope(false, |inner| {
scope.inner_def_scope(|inner| {
assert!(inner.introduce(ident.clone(), region).is_ok());
});
@ -919,7 +926,7 @@ mod test {
}
#[test]
fn idents_with_inner_scope() {
fn idents_with_inner_def_scope() {
let _register_module_debug_names = ModuleIds::default();
let mut scope = Scope::new(
ModuleId::ATTR,
@ -954,7 +961,7 @@ mod test {
&[ident1.clone(), ident2.clone(), ident3.clone(),]
);
scope.inner_scope(false, |inner| {
scope.inner_def_scope(|inner| {
let ident4 = Ident::from("Ångström");
let ident5 = Ident::from("Sirály");

View file

@ -136,7 +136,7 @@ fn constrain_untyped_closure(
vars.push(fn_var);
let body_type = constraints.push_expected_type(ForReason(
Reason::Return,
Reason::FunctionOutput,
return_type_index,
loc_body_expr.region,
));
@ -1409,7 +1409,7 @@ pub fn constrain_expr(
let return_type_index = constraints.push_variable(*return_var);
let expected_return_value = constraints.push_expected_type(ForReason(
Reason::Return,
Reason::FunctionOutput,
return_type_index,
return_value.region,
));

View file

@ -1085,7 +1085,13 @@ fn fmt_return<'a>(
buf.spaces(1);
return_value.format(buf, indent);
let return_indent = if return_value.is_multiline() {
indent + INDENT
} else {
indent
};
return_value.format(buf, return_indent);
if let Some(after_return) = after_return {
after_return.format_with_options(buf, parens, newlines, indent);

View file

@ -182,7 +182,7 @@ fn remove_for_reason(
}
| Reason::CrashArg
| Reason::ImportParams(_)
| Reason::Return => {}
| Reason::FunctionOutput => {}
}
}

View file

@ -0,0 +1 @@
Expr(Return(IndentReturnValue(@6), @0), @0)

View file

@ -0,0 +1 @@
return

View file

@ -0,0 +1,10 @@
maybeEarlyReturn = \x ->
y =
if x > 5 then
return "abc"
else
x + 2
Num.toStr y
maybeEarlyReturn 10

View file

@ -0,0 +1,167 @@
SpaceAfter(
Defs(
Defs {
tags: [
Index(2147483648),
],
regions: [
@0-127,
],
space_before: [
Slice(start = 0, length = 0),
],
space_after: [
Slice(start = 0, length = 0),
],
spaces: [],
type_defs: [],
value_defs: [
Body(
@0-16 Identifier {
ident: "maybeEarlyReturn",
},
@19-127 Closure(
[
@20-21 Identifier {
ident: "x",
},
],
@29-127 SpaceBefore(
Defs(
Defs {
tags: [
Index(2147483648),
],
regions: [
@29-110,
],
space_before: [
Slice(start = 0, length = 0),
],
space_after: [
Slice(start = 0, length = 0),
],
spaces: [],
type_defs: [],
value_defs: [
Body(
@29-30 Identifier {
ident: "y",
},
@41-110 SpaceBefore(
If {
if_thens: [
(
@44-49 BinOps(
[
(
@44-45 Var {
module_name: "",
ident: "x",
},
@46-47 GreaterThan,
),
],
@48-49 Num(
"5",
),
),
@67-79 SpaceBefore(
SpaceAfter(
Return(
@67-79 Str(
PlainLine(
"abc",
),
),
None,
),
[
Newline,
],
),
[
Newline,
],
),
),
],
final_else: @105-110 SpaceBefore(
BinOps(
[
(
@105-106 Var {
module_name: "",
ident: "x",
},
@107-108 Plus,
),
],
@109-110 Num(
"2",
),
),
[
Newline,
],
),
indented_else: false,
},
[
Newline,
],
),
),
],
},
@116-127 SpaceBefore(
Apply(
@116-125 Var {
module_name: "Num",
ident: "toStr",
},
[
@126-127 Var {
module_name: "",
ident: "y",
},
],
Space,
),
[
Newline,
Newline,
],
),
),
[
Newline,
],
),
),
),
],
},
@129-148 SpaceBefore(
Apply(
@129-145 Var {
module_name: "",
ident: "maybeEarlyReturn",
},
[
@146-148 Num(
"10",
),
],
Space,
),
[
Newline,
Newline,
],
),
),
[
Newline,
],
)

View file

@ -0,0 +1,10 @@
maybeEarlyReturn = \x ->
y =
if x > 5 then
return "abc"
else
x + 2
Num.toStr y
maybeEarlyReturn 10

View file

@ -0,0 +1,11 @@
staticValueDef =
someVal =
if 10 > 5 then
x = 5
return x
else
6
someVal + 2
staticValueDef

View file

@ -0,0 +1,169 @@
SpaceAfter(
Defs(
Defs {
tags: [
Index(2147483648),
],
regions: [
@0-142,
],
space_before: [
Slice(start = 0, length = 0),
],
space_after: [
Slice(start = 0, length = 0),
],
spaces: [],
type_defs: [],
value_defs: [
Body(
@0-14 Identifier {
ident: "staticValueDef",
},
@21-142 SpaceBefore(
Defs(
Defs {
tags: [
Index(2147483648),
],
regions: [
@21-125,
],
space_before: [
Slice(start = 0, length = 0),
],
space_after: [
Slice(start = 0, length = 0),
],
spaces: [],
type_defs: [],
value_defs: [
Body(
@21-28 Identifier {
ident: "someVal",
},
@39-125 SpaceBefore(
If {
if_thens: [
(
@42-48 BinOps(
[
(
@42-44 Num(
"10",
),
@45-46 GreaterThan,
),
],
@47-48 Num(
"5",
),
),
@67-97 SpaceBefore(
SpaceAfter(
Defs(
Defs {
tags: [
Index(2147483648),
],
regions: [
@67-72,
],
space_before: [
Slice(start = 0, length = 0),
],
space_after: [
Slice(start = 0, length = 0),
],
spaces: [],
type_defs: [],
value_defs: [
Body(
@67-68 Identifier {
ident: "x",
},
@71-72 Num(
"5",
),
),
],
},
Return(
@86-97 Var {
module_name: "",
ident: "x",
},
None,
),
),
[
Newline,
],
),
[
Newline,
],
),
),
],
final_else: @124-125 SpaceBefore(
Num(
"6",
),
[
Newline,
],
),
indented_else: false,
},
[
Newline,
],
),
),
],
},
@131-142 SpaceBefore(
BinOps(
[
(
@131-138 Var {
module_name: "",
ident: "someVal",
},
@139-140 Plus,
),
],
@141-142 Num(
"2",
),
),
[
Newline,
Newline,
],
),
),
[
Newline,
],
),
),
],
},
@145-159 SpaceBefore(
Var {
module_name: "",
ident: "staticValueDef",
},
[
Newline,
Newline,
Newline,
],
),
),
[
Newline,
],
)

View file

@ -0,0 +1,12 @@
staticValueDef =
someVal =
if 10 > 5 then
x = 5
return x
else
6
someVal + 2
staticValueDef

View file

@ -0,0 +1,11 @@
maybeEarlyReturn = \x ->
y =
when x is
5 ->
return "abc"
_ -> x + 2
Num.toStr y
maybeEarlyRetun 3

View file

@ -0,0 +1,177 @@
SpaceAfter(
Defs(
Defs {
tags: [
Index(2147483648),
],
regions: [
@0-154,
],
space_before: [
Slice(start = 0, length = 0),
],
space_after: [
Slice(start = 0, length = 0),
],
spaces: [],
type_defs: [],
value_defs: [
Body(
@0-16 Identifier {
ident: "maybeEarlyReturn",
},
@19-154 Closure(
[
@20-21 Identifier {
ident: "x",
},
],
@29-154 SpaceBefore(
Defs(
Defs {
tags: [
Index(2147483648),
],
regions: [
@29-136,
],
space_before: [
Slice(start = 0, length = 0),
],
space_after: [
Slice(start = 0, length = 0),
],
spaces: [],
type_defs: [],
value_defs: [
Body(
@29-30 Identifier {
ident: "y",
},
@37-136 SpaceBefore(
When(
@42-43 Var {
module_name: "",
ident: "x",
},
[
WhenBranch {
patterns: [
@55-56 SpaceBefore(
NumLiteral(
"5",
),
[
Newline,
],
),
],
value: @80-116 SpaceBefore(
Return(
@80-116 SpaceBefore(
Str(
PlainLine(
"abc",
),
),
[
Newline,
],
),
None,
),
[
Newline,
],
),
guard: None,
},
WhenBranch {
patterns: [
@126-127 SpaceBefore(
Underscore(
"",
),
[
Newline,
Newline,
],
),
],
value: @131-136 BinOps(
[
(
@131-132 Var {
module_name: "",
ident: "x",
},
@133-134 Plus,
),
],
@135-136 Num(
"2",
),
),
guard: None,
},
],
),
[
Newline,
],
),
),
],
},
@143-154 SpaceBefore(
Apply(
@143-152 Var {
module_name: "Num",
ident: "toStr",
},
[
@153-154 Var {
module_name: "",
ident: "y",
},
],
Space,
),
[
Newline,
Newline,
Newline,
],
),
),
[
Newline,
],
),
),
),
],
},
@156-173 SpaceBefore(
Apply(
@156-171 Var {
module_name: "",
ident: "maybeEarlyRetun",
},
[
@172-173 Num(
"3",
),
],
Space,
),
[
Newline,
Newline,
],
),
),
[
Newline,
],
)

View file

@ -0,0 +1,13 @@
maybeEarlyReturn = \x ->
y =
when x is
5 ->
return
"abc"
_ -> x + 2
Num.toStr y
maybeEarlyRetun 3

View file

@ -0,0 +1,3 @@
return something
|> pipeToFunction
|> andAnother

View file

@ -0,0 +1,45 @@
SpaceAfter(
Return(
@0-84 SpaceBefore(
BinOps(
[
(
@15-24 SpaceAfter(
Var {
module_name: "",
ident: "something",
},
[
Newline,
],
),
@37-39 Pizza,
),
(
@40-54 SpaceAfter(
Var {
module_name: "",
ident: "pipeToFunction",
},
[
Newline,
],
),
@71-73 Pizza,
),
],
@74-84 Var {
module_name: "",
ident: "andAnother",
},
),
[
Newline,
],
),
None,
),
[
Newline,
],
)

View file

@ -0,0 +1,4 @@
return
something
|> pipeToFunction
|> andAnother

View file

@ -0,0 +1,4 @@
identityFn = \x ->
return x
identityFn 45

View file

@ -0,0 +1,68 @@
SpaceAfter(
Defs(
Defs {
tags: [
Index(2147483648),
],
regions: [
@0-33,
],
space_before: [
Slice(start = 0, length = 0),
],
space_after: [
Slice(start = 0, length = 0),
],
spaces: [],
type_defs: [],
value_defs: [
Body(
@0-10 Identifier {
ident: "identityFn",
},
@13-33 Closure(
[
@14-15 Identifier {
ident: "x",
},
],
@21-33 SpaceBefore(
Return(
@21-33 Var {
module_name: "",
ident: "x",
},
None,
),
[
Newline,
],
),
),
),
],
},
@36-49 SpaceBefore(
Apply(
@36-46 Var {
module_name: "",
ident: "identityFn",
},
[
@47-49 Num(
"45",
),
],
Space,
),
[
Newline,
Newline,
Newline,
],
),
),
[
Newline,
],
)

View file

@ -0,0 +1,5 @@
identityFn = \x ->
return x
identityFn 45

View file

@ -196,6 +196,7 @@ mod test_snapshots {
fail/double_plus.expr,
fail/elm_function_syntax.expr,
fail/empty_or_pattern.expr,
fail/empty_return.expr,
fail/error_inline_alias_argument_uppercase.expr,
fail/error_inline_alias_not_an_alias.expr,
fail/error_inline_alias_qualified.expr,
@ -233,6 +234,7 @@ mod test_snapshots {
fail/record_type_open.expr,
fail/record_type_open_indent.expr,
fail/record_type_tab.expr,
fail/return_as_single_line_expr.expr,
fail/single_no_end.expr,
fail/tab_crash.header,
fail/tag_union_end.expr,
@ -463,6 +465,11 @@ mod test_snapshots {
pass/record_updater_var_apply.expr,
pass/record_with_if.expr,
pass/requires_type.header,
pass/return_in_if.expr,
pass/return_in_static_def.expr,
pass/return_in_when.expr,
pass/return_multiline.expr,
pass/return_only_statement.expr,
pass/separate_defs.moduledefs,
pass/single_arg_closure.expr,
pass/single_underscore_closure.expr,

View file

@ -3425,7 +3425,7 @@ pub enum Reason {
},
CrashArg,
ImportParams(ModuleId),
Return,
FunctionOutput,
}
#[derive(PartialEq, Eq, Debug, Clone)]

View file

@ -918,7 +918,7 @@ fn to_expr_report<'b>(
alloc.reflow("But I need every "),
alloc.keyword("expect"),
alloc.reflow(" condition to evaluate to a "),
alloc.reflow("—either "),
alloc.reflow("Bool—either "),
alloc.tag("Bool.true".into()),
alloc.reflow(" or "),
alloc.tag("Bool.false".into()),
@ -1642,7 +1642,7 @@ fn to_expr_report<'b>(
unimplemented!("record default field is not implemented yet")
}
Reason::ImportParams(_) => unreachable!(),
Reason::Return => {
Reason::FunctionOutput => {
let problem = alloc.concat([
alloc.text("This "),
alloc.keyword("return"),