Skip to content

Commit

Permalink
Auto-recover constructor expressions (#4124)
Browse files Browse the repository at this point in the history
This PR adds checks when assigning a constructor expression or passing one as a parameter to see if it can be auto-recovered, and if so, does not output a subtyping error. E.g. for the following code:

```
actor Main
  new create(env: Env) =>
    Bar.take_foo(Foo(88))
    let bar: Foo iso = Foo(88)

class Foo
  new create(v: U8) =>
    None

primitive Bar
  fun take_foo(foo: Foo iso) =>
    None
```

We'd previously get errors on both lines in Main.create(); whereas with this PR they are now accepted.

Fixes #702


Co-authored-by: Sean T Allen <sean@seantallen.com>
Co-authored-by: Jason Carr <jcarr250@protonmail.com>
  • Loading branch information
3 people authored Jul 5, 2022
1 parent ab6a6a3 commit 7b01704
Show file tree
Hide file tree
Showing 7 changed files with 300 additions and 5 deletions.
24 changes: 24 additions & 0 deletions .release-notes/4124.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
## Allow constructor expressions to be auto-recovered in assignments or arguments

Previously for the following code:

```pony
actor Main
new create(env: Env) =>
Bar.take_foo(Foo(88))
let bar: Foo iso = Foo(88)
class Foo
new create(v: U8) =>
None
primitive Bar
fun take_foo(foo: Foo iso) =>
None
```

You'd get compilation errors "argument not assignable to parameter" and "right side must be a subtype of left side" for the two lines in `Main.create()`.

We've added checks to see if the constructor expressions can be implicitly auto-recovered, and if they can, no compilation error is generated.

This only applies to cases where the type of the parameter (or the `let` binding) is a simple type, i.e. not a union, intersection or tuple type.
57 changes: 56 additions & 1 deletion src/libponyc/expr/call.c
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,54 @@ static bool apply_default_arg(pass_opt_t* opt, ast_t* param, ast_t** argp)
return true;
}

static ast_t* method_receiver_type(ast_t* method);

bool check_auto_recover_newref(ast_t* dest_type, ast_t* ast)
{
// we're not going to try auto-recovering to a complex type
if (ast_id(dest_type) != TK_NOMINAL)
return false;

while (ast != NULL && ast_id(ast) != TK_CALL)
ast = ast_child(ast);

if (ast == NULL)
return false;

AST_GET_CHILDREN(ast, newref, positional, named);
if (ast_id(newref) != TK_NEWREF)
return false;

// sometimes for assignments there's a nested newref
ast_t* child = ast_child(newref);
if (child != NULL && ast_id(child) == TK_NEWREF)
newref = child;

ast_t* receiver_type = method_receiver_type(newref);
token_id rcap = ast_id(cap_fetch(receiver_type));
if (!(rcap == TK_REF || rcap == TK_BOX))
return false;

ast_t* arg = ast_child(positional);
while (arg != NULL)
{
ast_t* arg_type = ast_type(arg);
if (is_typecheck_error(arg_type))
return false;

ast_t* arg_type_aliased = alias(arg_type);
bool ok = safe_to_autorecover(dest_type, arg_type_aliased, WRITE);
ast_free_unattached(arg_type_aliased);

if (!ok)
return false;

arg = ast_sibling(arg);
}

return true;
}

static bool check_arg_types(pass_opt_t* opt, ast_t* params, ast_t* positional,
bool partial, bool is_bare)
{
Expand Down Expand Up @@ -236,8 +284,15 @@ static bool check_arg_types(pass_opt_t* opt, ast_t* params, ast_t* positional,
return false;
}

ast_t* wp_type = consume_type(p_type, TK_NONE, false);
errorframe_t info = NULL;
ast_t* wp_type = consume_type(p_type, TK_NONE, false);
if(check_auto_recover_newref(wp_type, arg))
{
token_id arg_cap = ast_id(cap_fetch(wp_type));
ast_t* recovered_arg_type = recover_type(arg_type, arg_cap);
if (recovered_arg_type)
arg_type = recovered_arg_type;
}

if(wp_type == NULL)
{
Expand Down
1 change: 1 addition & 0 deletions src/libponyc/expr/call.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
PONY_EXTERN_C_BEGIN

bool method_check_type_params(pass_opt_t* opt, ast_t** astp);
bool check_auto_recover_newref(ast_t* dest_type, ast_t* ast);

bool expr_call(pass_opt_t* opt, ast_t** astp);

Expand Down
9 changes: 9 additions & 0 deletions src/libponyc/expr/operator.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include "postfix.h"
#include "control.h"
#include "reference.h"
#include "call.h"
#include "../ast/astbuild.h"
#include "../ast/id.h"
#include "../ast/lexer.h"
Expand Down Expand Up @@ -373,7 +374,15 @@ bool expr_assign(pass_opt_t* opt, ast_t* ast)

default: {}
}

ast_t* wl_type = consume_type(fl_type, TK_NONE, false);
if(check_auto_recover_newref(fl_type, right))
{
token_id left_cap = ast_id(cap_fetch(wl_type));
ast_t* recovered_left_type = recover_type(r_type, left_cap);
if (recovered_left_type)
r_type = recovered_left_type;
}

// Assignment is based on the alias of the right hand side.
errorframe_t info = NULL;
Expand Down
8 changes: 4 additions & 4 deletions src/libponyc/type/cap.c
Original file line number Diff line number Diff line change
Expand Up @@ -950,6 +950,9 @@ bool cap_safetomove(token_id into, token_id cap, direction direction)
break;

case TK_TRN:
// when recovering to val, anything which has no
// writable aliases elsewhere is fine
case TK_VAL:
switch(cap)
{
case TK_ISO:
Expand Down Expand Up @@ -1012,15 +1015,13 @@ bool cap_immutable_or_opaque(token_id cap)
return false;
}



static ast_t* modified_cap_single(ast_t* type, cap_mutation* mutation)
{
ast_t* cap = cap_fetch(type);
ast_t* ephemeral = ast_sibling(cap);

token_id cap_token = ast_id(cap);
token_id eph_token = ast_id(cap);
token_id eph_token = ast_id(ephemeral);

if (mutation(&cap_token, &eph_token))
{
Expand Down Expand Up @@ -1153,4 +1154,3 @@ ast_t* unisolated(ast_t* type)
{
return modified_cap(type, unisolated_mod);
}

38 changes: 38 additions & 0 deletions test/libponyc-run/ctor-autorecover/main.pony
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
actor Main
new create(env: Env) =>
// zero-parameter constructor as argument and assignment
Bar.take_foo(Foo)
Bar.take_foo_val(Foo)
let qux: Foo iso = Foo

// sendable-parameter constructor as argument and assignment
Bar.take_foo(Foo.from_u8(88))
Bar.take_foo_val(Foo.from_u8(88))
let bar: Foo iso = Foo.from_u8(88)

// non-sendable parameter ctor and assignment must fail
// see RecoverTest.CantAutoRecover_CtorArgWithNonSendableArg
// and RecoverTest.CantAutoRecover_CtorAssignmentWithNonSendableArg

//let s: String ref = String
//Bar.take_foo(Foo.from_str(s)) -> argument not assignable to parameter
//let baz: Foo iso = Foo.from_str(s) -> right side must be a subtype of left side

// verify that inheritance hasn't broken
let x = String
let y: String ref = x

class Foo
new create() =>
None
new ref from_u8(v: U8) =>
None
new ref from_str(s: String ref) =>
None

primitive Bar
fun take_foo(foo: Foo iso) =>
None

fun take_foo_val(foo: Foo val) =>
None
168 changes: 168 additions & 0 deletions test/libponyc/recover.cc
Original file line number Diff line number Diff line change
Expand Up @@ -738,3 +738,171 @@ TEST_F(RecoverTest, CanWriteTrn_TrnAutoRecovery)

TEST_COMPILE(src);
}
TEST_F(RecoverTest, LetIso_CtorAutoRecovery)
{
const char* src =
"class A\n"
" new ref create() =>\n"
" None\n"

"actor Main\n"
" new create(env: Env) =>\n"
" let a_iso: A iso = A\n";

TEST_COMPILE(src);
}
TEST_F(RecoverTest, VarIso_CtorAutoRecovery)
{
const char* src =
"class A\n"
" new ref create() =>\n"
" None\n"

"actor Main\n"
" new create(env: Env) =>\n"
" var a_iso: A iso = A\n";

TEST_COMPILE(src);
}
TEST_F(RecoverTest, LetTrn_CtorAutoRecovery)
{
const char* src =
"class A\n"
" new ref create() =>\n"
" None\n"

"actor Main\n"
" new create(env: Env) =>\n"
" var a_trn: A trn = A\n";

TEST_COMPILE(src);
}
TEST_F(RecoverTest, LetVal_RefCtorAutoRecovery)
{
const char* src =
"class A\n"
" new ref create() =>\n"
" None\n"

"actor Main\n"
" new create(env: Env) =>\n"
" var a_val: A val = A\n";

TEST_COMPILE(src);
}
TEST_F(RecoverTest, LetVal_BoxCtorAutoRecovery)
{
const char* src =
"class A\n"
" new box create() =>\n"
" None\n"

"actor Main\n"
" new create(env: Env) =>\n"
" var a_val: A val = A\n";

TEST_COMPILE(src);
}
TEST_F(RecoverTest, LetIso_ArgsCtorAutoRecovery)
{
const char* src =
"class A\n"
" new ref create(s: String box) =>\n"
" None\n"

"actor Main\n"
" new create(env: Env) =>\n"
" let s: String val = \"\"\n"
" let a_iso: A iso = A(s)\n";

TEST_COMPILE(src);
}
TEST_F(RecoverTest, LetTrn_ArgsCtorAutoRecovery)
{
const char* src =
"class A\n"
" new ref create(s: String box) =>\n"
" None\n"

"actor Main\n"
" new create(env: Env) =>\n"
" let s: String val = \"\"\n"
" let a_trn: A trn = A(s)\n";

TEST_COMPILE(src);
}
TEST_F(RecoverTest, LetVal_ArgsCtorAutoRecovery)
{
const char* src =
"class A\n"
" new ref create(s: String box) =>\n"
" None\n"

"actor Main\n"
" new create(env: Env) =>\n"
" let s: String val = \"\"\n"
" let a_val: A val = A(s)\n";

TEST_COMPILE(src);
}


TEST_F(RecoverTest, CantAutoRecover_CtorArgWithNonSendableArg)
{
const char* src =
"actor Main\n"
" new create(env: Env) =>\n"
" let s: String ref = String\n"
" Bar.take_foo(Foo.from_str(s))\n"

"class Foo\n"
" new from_u8(v: U8) =>\n"
" None\n"
" new from_str(s: String ref) =>\n"
" None\n"

"primitive Bar\n"
" fun take_foo(foo: Foo iso) =>\n"
" None\n";

TEST_ERRORS_1(src, "argument not assignable to parameter");
}
TEST_F(RecoverTest, CantAutoRecover_CtorAssignmentWithNonSendableArg)
{
const char* src =
"actor Main\n"
" new create(env: Env) =>\n"
" let s: String ref = String\n"
" let bar: Foo iso = Foo.from_str(s)\n"

"class Foo\n"
" new from_u8(v: U8) =>\n"
" None\n"
" new from_str(s: String ref) =>\n"
" None\n"

"primitive Bar\n"
" fun take_foo(foo: Foo iso) =>\n"
" None\n";

TEST_ERRORS_1(src, "right side must be a subtype of left side");
}
TEST_F(RecoverTest, CantAutoRecover_CtorParamToComplexTypeWithNonSendableArg)
{
const char* src =
"actor Main\n"
" new create(env: Env) =>\n"
" let bar: (Foo iso | String ref) = Foo.from_u8(123)\n"

"class Foo\n"
" new from_u8(v: U8) =>\n"
" None\n"
" new from_str(s: String ref) =>\n"
" None\n"

"primitive Bar\n"
" fun take_foo(foo: Foo iso) =>\n"
" None\n";

TEST_ERRORS_1(src, "right side must be a subtype of left side");
}

0 comments on commit 7b01704

Please sign in to comment.