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
22 changes: 22 additions & 0 deletions .release-notes/4124.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
## 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.
55 changes: 54 additions & 1 deletion src/libponyc/expr/call.c
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,51 @@ 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* ast)
{
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 newrefs are nested?
ast_t* child = ast_child(newref);
if (child != NULL && ast_id(child) == TK_NEWREF)
newref = child;

ast_t* receiver_type = method_receiver_type(newref);

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(receiver_type, arg_type_aliased, WRITE);
ast_free_unattached(arg_type_aliased);

if (!ok)
return false;

arg = ast_sibling(arg);
}

token_id tcap = ast_id(cap_fetch(receiver_type));
if (tcap == TK_REF || tcap == TK_BOX)
return true;
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be nice to instead eagerly return false if the capability doesn't match, that way you don't have to check the arguments if the return capability is already wrong.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.


return false;
}

static bool check_arg_types(pass_opt_t* opt, ast_t* params, ast_t* positional,
bool partial)
{
Expand Down Expand Up @@ -235,8 +280,16 @@ 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;
if(check_auto_recover_newref(arg))
{
wp_type = unisolated(p_type);
Copy link
Contributor

@jasoncarr0 jasoncarr0 May 24, 2022

Choose a reason for hiding this comment

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

This isn't quite the right behavior, although it might be on the right track. You should be upgrading the argument/constructor capability instead trying to downgrade the parameter.

That way for instance, a String val argument can be fulfilled by auto-recovery of String (which is a ref constructor). It would also be a bit clearer what's going on that way. Or for another example, the linked issue used array val

Copy link
Member Author

Choose a reason for hiding this comment

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

I believe I've fixed this now.

}
else
{
wp_type = consume_type(p_type, TK_NONE, false);
}

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* ast);

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

Expand Down
12 changes: 11 additions & 1 deletion 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,16 @@ bool expr_assign(pass_opt_t* opt, ast_t* ast)

default: {}
}
ast_t* wl_type = consume_type(fl_type, TK_NONE, false);

ast_t* wl_type;
if(check_auto_recover_newref(right))
{
wl_type = unisolated(fl_type);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar comment to above, the r_type should be upgraded instead, as with recover blocks.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done as above.

else
{
wl_type = consume_type(fl_type, TK_NONE, false);
}

// Assignment is based on the alias of the right hand side.
errorframe_t info = NULL;
Expand Down
5 changes: 1 addition & 4 deletions src/libponyc/type/cap.c
Original file line number Diff line number Diff line change
Expand Up @@ -1012,15 +1012,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 +1151,3 @@ ast_t* unisolated(ast_t* type)
{
return modified_cap(type, unisolated_mod);
}

12 changes: 12 additions & 0 deletions test/libponyc-run/ctor-autorecover/main.pony
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
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