Skip to content

Commit

Permalink
Take exhaustive match into account when checking for field initializa…
Browse files Browse the repository at this point in the history
…tion

Previously, exhaustive matches were handled after we verified that an object's fields
were initialized. This lead to the code such as the following not passing compiler
checks:

```pony
primitive Foo
  fun apply() : (U32 | None) => 1

type Bar is (U32 | None)

actor Main
  let bar : Bar

  new create(env : Env) =>
      match Foo()
      | let result : U32 =>
          bar = result
      | None =>
          bar = 0
      end
```

This commit moves field initialization checking to the verify pass so that it
comes after the expression pass when exhaustiveness checking is done. This also
moves code to patchup matches from the refer pass to a new completeness pass
that comes after the expression pass. The new `completeness_match` code is
identical to `refer_match` code except that an else clause with a value of
`TK_NONE` no longer means a clause exists. In the exhaustiveness checks in the
expression pass, any non-exhaustive else clause that was a `TK_NONE` has since
been replaced with a `else None` so, by the time we get to completeness,
`TK_NONE` literally means "there's no else clause".

Closes #3615
  • Loading branch information
SeanTAllen committed Feb 15, 2022
1 parent d330436 commit dda4ee2
Show file tree
Hide file tree
Showing 10 changed files with 173 additions and 45 deletions.
23 changes: 23 additions & 0 deletions .release-notes/issue-3616.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
## Take exhaustive match into account when checking for field initialization

Previously, exhaustive matches were handled after we verified that an object's fields were initialized. This lead to the code such as the following not passing compiler checks:

```pony
primitive Foo
fun apply() : (U32 | None) => 1
type Bar is (U32 | None)
actor Main
let bar : Bar
new create(env : Env) =>
match Foo()
| let result : U32 =>
bar = result
| None =>
bar = 0
end
```

Field initialization is now done in a later pass after exhaustiveness checking has been done.
1 change: 1 addition & 0 deletions src/libponyc/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ add_library(libponyc STATIC
expr/postfix.c
expr/reference.c
options/options.c
pass/completeness.c
pass/docgen.c
pass/expr.c
pass/finalisers.c
Expand Down
53 changes: 53 additions & 0 deletions src/libponyc/pass/completeness.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
#include "completeness.h"
#include "ponyassert.h"

static bool completeness_match(pass_opt_t* opt, ast_t* ast)
{
(void)opt;
pony_assert(ast_id(ast) == TK_MATCH);
AST_GET_CHILDREN(ast, expr, cases, else_clause);

size_t branch_count = 0;

if(!ast_checkflag(cases, AST_FLAG_JUMPS_AWAY))
{
branch_count++;
ast_inheritbranch(ast, cases);
}

// An else_clause of TK_NONE doesn't cont as a branch because, we are after
// exhaustive match checking is done so, an else clause of TK_NONE means that
// we don't need the else and have an exhaustive match.
if(ast_id(else_clause) != TK_NONE &&
!ast_checkflag(else_clause, AST_FLAG_JUMPS_AWAY))
{
branch_count++;
ast_inheritbranch(ast, else_clause);
}

// If all branches jump away with no value, then we do too.
if(branch_count == 0)
ast_setflag(ast, AST_FLAG_JUMPS_AWAY);

ast_consolidate_branches(ast, branch_count);

// Push our symbol status to our parent scope.
ast_inheritstatus(ast_parent(ast), ast);

return true;
}

ast_result_t pass_completeness(ast_t** astp, pass_opt_t* options)
{
ast_t* ast = *astp;

switch(ast_id(ast))
{
case TK_MATCH:
completeness_match(options, ast); break;
default:
{}
}

return AST_OK;
}
14 changes: 14 additions & 0 deletions src/libponyc/pass/completeness.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#ifndef PASS_COMPLETENESS_H
#define PASS_COMPLETENESS_H

#include <platform.h>
#include "../ast/ast.h"
#include "../pass/pass.h"

PONY_EXTERN_C_BEGIN

ast_result_t pass_completeness(ast_t** astp, pass_opt_t* options);

PONY_EXTERN_C_END

#endif
9 changes: 9 additions & 0 deletions src/libponyc/pass/pass.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "traits.h"
#include "refer.h"
#include "expr.h"
#include "completeness.h"
#include "verify.h"
#include "finalisers.h"
#include "serialisers.h"
Expand Down Expand Up @@ -60,6 +61,7 @@ const char* pass_name(pass_id pass)
case PASS_DOCS: return "docs";
case PASS_REFER: return "refer";
case PASS_EXPR: return "expr";
case PASS_COMPLETENESS: return "completeness";
case PASS_VERIFY: return "verify";
case PASS_FINALISER: return "final";
case PASS_SERIALISER: return "serialise";
Expand Down Expand Up @@ -272,6 +274,13 @@ static bool ast_passes(ast_t** astp, pass_opt_t* options, pass_id last)
if(is_program)
plugin_visit_ast(*astp, options, PASS_EXPR);

if(!visit_pass(astp, options, last, &r, PASS_COMPLETENESS, NULL,
pass_completeness))
return r;

if(is_program)
plugin_visit_ast(*astp, options, PASS_COMPLETENESS);

if(!visit_pass(astp, options, last, &r, PASS_VERIFY, NULL, pass_verify))
return r;

Expand Down
10 changes: 10 additions & 0 deletions src/libponyc/pass/pass.h
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,14 @@ Also performs some "sugar" replacements that require knowledge of types.
Mutates the AST extensively.
* Completeness pass (AST)
Does fixup of the AST that needs to be done after the expression pass and before
verification.
Mutates the AST.
* Verify pass (AST)
Perform various checks that are not required for type resolution, and are not
Expand Down Expand Up @@ -218,6 +226,7 @@ typedef enum pass_id
PASS_DOCS,
PASS_REFER,
PASS_EXPR,
PASS_COMPLETENESS,
PASS_VERIFY,
PASS_FINALISER,
PASS_SERIALISER,
Expand Down Expand Up @@ -245,6 +254,7 @@ typedef enum pass_id
" =docs\n" \
" =refer\n" \
" =expr\n" \
" =completeness\n" \
" =verify\n" \
" =final\n" \
" =serialise\n" \
Expand Down
44 changes: 0 additions & 44 deletions src/libponyc/pass/refer.c
Original file line number Diff line number Diff line change
Expand Up @@ -1258,49 +1258,6 @@ static bool refer_pre_new(pass_opt_t* opt, ast_t* ast)
return true;
}

static bool refer_new(pass_opt_t* opt, ast_t* ast)
{
pony_assert(ast_id(ast) == TK_NEW);

ast_t* members = ast_parent(ast);
ast_t* member = ast_child(members);
bool result = true;

while(member != NULL)
{
switch(ast_id(member))
{
case TK_FVAR:
case TK_FLET:
case TK_EMBED:
{
sym_status_t status;
ast_t* id = ast_child(member);
ast_t* def = ast_get(ast, ast_name(id), &status);

if((def != member) || (status != SYM_DEFINED))
{
ast_error(opt->check.errors, def,
"field left undefined in constructor");
result = false;
}

break;
}

default: {}
}

member = ast_sibling(member);
}

if(!result)
ast_error(opt->check.errors, ast,
"constructor with undefined fields is here");

return result;
}

static bool refer_local(pass_opt_t* opt, ast_t* ast)
{
pony_assert(ast != NULL);
Expand Down Expand Up @@ -1843,7 +1800,6 @@ ast_result_t pass_refer(ast_t** astp, pass_opt_t* options)
case TK_CONSUME: r = refer_consume(options, ast); break;

case TK_THIS: r = refer_this(options, ast); break;
case TK_NEW: r = refer_new(options, ast); break;
case TK_VAR:
case TK_LET: r = refer_local(options, ast); break;

Expand Down
48 changes: 47 additions & 1 deletion src/libponyc/verify/fun.c
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,51 @@ static bool show_partiality(pass_opt_t* opt, ast_t* ast)
return false;
}

bool verify_fields_are_defined_in_constructor(pass_opt_t* opt, ast_t* ast)
{
bool result = true;

if(ast_id(ast) != TK_NEW)
return result;

ast_t* members = ast_parent(ast);
ast_t* member = ast_child(members);

while(member != NULL)
{
switch(ast_id(member))
{
case TK_FVAR:
case TK_FLET:
case TK_EMBED:
{
sym_status_t status;
ast_t* id = ast_child(member);
ast_t* def = ast_get(ast, ast_name(id), &status);

if((def != member) || (status != SYM_DEFINED))
{
ast_error(opt->check.errors, def,
"field left undefined in constructor");
result = false;
}

break;
}

default: {}
}

member = ast_sibling(member);
}

if(!result)
ast_error(opt->check.errors, ast,
"constructor with undefined fields is here");

return result;
}

bool verify_fun(pass_opt_t* opt, ast_t* ast)
{
pony_assert((ast_id(ast) == TK_BE) || (ast_id(ast) == TK_FUN) ||
Expand All @@ -498,7 +543,8 @@ bool verify_fun(pass_opt_t* opt, ast_t* ast)
!verify_main_runtime_override_defaults(opt, ast) ||
!verify_primitive_init(opt, ast) ||
!verify_any_final(opt, ast) ||
!verify_any_serialise(opt, ast))
!verify_any_serialise(opt, ast) ||
!verify_fields_are_defined_in_constructor(opt, ast))
return false;

// Check partial functions.
Expand Down
1 change: 1 addition & 0 deletions src/libponyc/verify/fun.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
PONY_EXTERN_C_BEGIN

bool verify_fun(pass_opt_t* opt, ast_t* ast);
bool verify_fields_are_defined_in_constructor(pass_opt_t* opt, ast_t* ast);

PONY_EXTERN_C_END

Expand Down
15 changes: 15 additions & 0 deletions test/libponyc-run/regression-3615/main.pony
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
primitive Foo
fun apply() : (U32 | None) => 1

type Bar is (U32 | None)

actor Main
let bar : Bar

new create(env : Env) =>
match Foo()
| let result : U32 =>
bar = result
| None =>
bar = 0
end

0 comments on commit dda4ee2

Please sign in to comment.