Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auto-recover constructor expressions #4124

Merged
merged 12 commits into from
Jul 5, 2022
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 @@ -194,6 +194,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)
{
Expand Down Expand Up @@ -235,8 +283,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);
ergl marked this conversation as resolved.
Show resolved Hide resolved

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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have pushed a fix for the incorrect spacing between functions in this diff.

{
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");
}