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

Error on unreachable cases in match expressions and illegal as expressions. #2289

Merged
merged 2 commits into from Oct 27, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 3 additions & 4 deletions packages/collections/persistent/_map_node.pony
Expand Up @@ -81,7 +81,7 @@ class val _MapNode[K: Any #share, V: Any #share, H: mut.HashFunction[K] val]
// update sub-node
let sn = _entries(i.usize_unsafe())? as _MapNode[K, V, H]
let es = recover _entries.clone() end
(let sn', let u) = sn.update(hash, leaf)? as (_MapNode[K, V, H], Bool)
(let sn', let u) = sn.update(hash, leaf)?
es(i.usize_unsafe())? = sn'
(create(consume es, _nodemap, _datamap, _level), u)
end
Expand All @@ -107,9 +107,8 @@ class val _MapNode[K: Any #share, V: Any #share, H: mut.HashFunction[K] val]
else
// create new sub-node
var sn = empty(_level + 1)
(sn, _) =
sn.update(H.hash(old._1).u32(), old)? as (_MapNode[K, V, H], Bool)
(sn, _) = sn.update(hash, leaf)? as (_MapNode[K, V, H], Bool)
(sn, _) = sn.update(H.hash(old._1).u32(), old)?
(sn, _) = sn.update(hash, leaf)?
let es = recover _entries.clone() end
let nm = _Bits.set_bit(_nodemap, idx)
let dm = _Bits.clear_bit(_datamap, idx)
Expand Down
27 changes: 13 additions & 14 deletions packages/files/file_path.pony
Expand Up @@ -34,23 +34,22 @@ class val FilePath
"""
caps.union(caps')

match base
| let b: FilePath =>
if not b.caps(FileLookup) then
error
end
path = match base
| let b: FilePath =>
if not b.caps(FileLookup) then
error
end

path = Path.join(b.path, path')
caps.intersect(b.caps)
let tmp_path = Path.join(b.path, path')
caps.intersect(b.caps)

if not path.at(b.path, 0) then
error
if not tmp_path.at(b.path, 0) then
error
end
tmp_path
| let b: AmbientAuth =>
Path.abs(path')
end
| let b: AmbientAuth =>
path = Path.abs(path')
else
error
end

new val mkdtemp(
base: (FilePath | AmbientAuth),
Expand Down
4 changes: 2 additions & 2 deletions packages/itertools/iter.pony
Expand Up @@ -386,7 +386,7 @@ class Iter[A] is Iterator[A]
"""
filter_stateful({(a: A!): Bool ? => f(a)? })

fun ref find(f: {(A!): Bool ?} box, n: USize = 1): A ? =>
fun ref find(f: {(A!): Bool ?} box, n: USize = 1): A! ? =>
"""
Return the nth value in the iterator that satisfies the predicate `f`.

Expand All @@ -408,7 +408,7 @@ class Iter[A] is Iterator[A]
for x in _iter do
if try f(x)? else false end then
if c == 1 then
return consume x as A
return x
else
c = c - 1
end
Expand Down
1 change: 0 additions & 1 deletion packages/process/_test.pony
Expand Up @@ -434,7 +434,6 @@ class _ProcessClient is ProcessNotify
| KillError => _h.fail("ProcessError: KillError")
| Unsupported => _h.fail("ProcessError: Unsupported")
| CapError => _h.complete(true) // used in _TestFileExec
else _h.fail("Unknown ProcessError!")
end

fun ref dispose(process: ProcessMonitor ref, child_exit_code: I32) =>
Expand Down
101 changes: 70 additions & 31 deletions src/libponyc/expr/match.c
Expand Up @@ -89,18 +89,24 @@ static bool case_expr_matches_type_alone(pass_opt_t* opt, ast_t* case_expr)
return true;
}

static bool is_match_exhaustive(pass_opt_t* opt, ast_t* expr_type, ast_t* cases)
/**
* return a pointer to the case expr at which the match can be considered
* exhaustive or NULL if it is not exhaustive.
**/
static ast_t* is_match_exhaustive(pass_opt_t* opt, ast_t* expr_type,
ast_t* cases)
{
pony_assert(expr_type != NULL);
pony_assert(ast_id(cases) == TK_CASES);

// Exhaustive match not yet supported for matches where all cases "jump away".
// The return/error/break/continue should be moved to outside the match.
if(ast_checkflag(cases, AST_FLAG_JUMPS_AWAY))
return false;
return NULL;

// Construct a union of all pattern types that count toward exhaustive match.
ast_t* cases_union_type = ast_from(cases, TK_UNIONTYPE);
ast_t* result = NULL;

for(ast_t* c = ast_child(cases); c != NULL; c = ast_sibling(c))
{
Expand All @@ -110,8 +116,10 @@ static bool is_match_exhaustive(pass_opt_t* opt, ast_t* expr_type, ast_t* cases)
// if any case is a `_` we have an exhaustive match
// and we can shortcut here
if(ast_id(case_expr) == TK_DONTCAREREF)
return true;

{
result = c;
break;
}
// Only cases with no guard clause can count toward exhaustive match,
// because the truth of a guard clause can't be statically evaluated.
// So, for the purposes of exhaustive match, we ignore those cases.
Expand All @@ -126,18 +134,21 @@ static bool is_match_exhaustive(pass_opt_t* opt, ast_t* expr_type, ast_t* cases)

// It counts, so add this pattern type to our running union type.
ast_add(cases_union_type, pattern_type);
}

// If our cases types union is a supertype of the match expression type,
// then the match must be exhaustive, because all types are covered by cases.
bool ret = (ast_childcount(cases_union_type) > 0) &&
is_subtype(expr_type, cases_union_type, NULL, opt);
// If our cases types union is a supertype of the match expression type,
// then the match must be exhaustive, because all types are covered by cases.
if(is_subtype(expr_type, cases_union_type, NULL, opt))
{
result = c;
break;
}
}

ast_free_unattached(cases_union_type);

return ret;
return result;
}


bool expr_match(pass_opt_t* opt, ast_t* ast)
{
pony_assert(ast_id(ast) == TK_MATCH);
Expand Down Expand Up @@ -176,32 +187,60 @@ bool expr_match(pass_opt_t* opt, ast_t* ast)
type = control_type_add_branch(opt, type, cases);
}

// If we have no else clause, and the match is not found to be exhaustive,
// we must generate an implicit else clause that returns None as the value.
if((ast_id(else_clause) == TK_NONE) &&
!is_match_exhaustive(opt, expr_type, cases))
{
ast_scope(else_clause);
ast_setid(else_clause, TK_SEQ);
// analyze exhaustiveness
ast_t* exhaustive_at = is_match_exhaustive(opt, expr_type, cases);

BUILD(ref, else_clause,
NODE(TK_TYPEREF,
NONE
ID("None")
NONE));
ast_add(else_clause, ref);

if(!expr_typeref(opt, &ref) || !expr_seq(opt, else_clause))
if(exhaustive_at == NULL)
{
// match might not be exhaustive
if ((ast_id(else_clause) == TK_NONE))
{
// If we have no else clause, and the match is not found to be exhaustive,
// we must generate an implicit else clause that returns None as the value.
ast_scope(else_clause);
ast_setid(else_clause, TK_SEQ);

BUILD(ref, else_clause,
NODE(TK_TYPEREF,
NONE
ID("None")
NONE));
ast_add(else_clause, ref);

if(!expr_typeref(opt, &ref) || !expr_seq(opt, else_clause))
return false;
}
}
else
{
// match is exhaustive
if(ast_sibling(exhaustive_at) != NULL)
{
// we have unreachable cases
ast_error(opt->check.errors, ast, "match contains unreachable cases");
ast_error_continue(opt->check.errors, ast_sibling(exhaustive_at),
"first unreachable case expression");
return false;
}
else if((ast_id(else_clause) != TK_NONE))
{
ast_error(opt->check.errors, ast,
"match is exhaustive, the else clause is unreachable");
ast_error_continue(opt->check.errors, else_clause,
"unreachable code");
return false;
}
}

if((ast_id(else_clause) != TK_NONE) &&
!ast_checkflag(else_clause, AST_FLAG_JUMPS_AWAY))
if((ast_id(else_clause) != TK_NONE))
{
if(is_typecheck_error(ast_type(else_clause)))
return false;
if (!ast_checkflag(else_clause, AST_FLAG_JUMPS_AWAY))
{
if(is_typecheck_error(ast_type(else_clause)))
return false;

type = control_type_add_branch(opt, type, else_clause);
type = control_type_add_branch(opt, type, else_clause);
}
}

if((type == NULL) && (ast_sibling(ast) != NULL))
Expand Down
17 changes: 17 additions & 0 deletions src/libponyc/expr/operator.c
Expand Up @@ -549,6 +549,23 @@ bool expr_as(pass_opt_t* opt, ast_t** astp)
return false;
}

if(is_subtype(expr_type, type, NULL, opt))
{
if(is_subtype(type, expr_type, NULL, opt))
{
ast_error(opt->check.errors, ast, "Cannot cast to same type");
ast_error_continue(opt->check.errors, expr,
"Expression is already of type %s", ast_print_type(type));
}
else
{
ast_error(opt->check.errors, ast, "Cannot cast to subtype");
ast_error_continue(opt->check.errors, expr,
"%s is a subtype of this Expression. 'as' is not needed here.", ast_print_type(type));
}
return false;
}

ast_t* pattern_root = ast_from(type, TK_LEX_ERROR);
ast_t* body_root = ast_from(type, TK_LEX_ERROR);
if(!add_as_type(opt, ast, expr, type, pattern_root, body_root))
Expand Down
2 changes: 1 addition & 1 deletion test/libponyc/badpony.cc
Expand Up @@ -675,7 +675,7 @@ TEST_F(BadPonyTest, AsUnaliased)
"class trn Foo\n"

"actor Main\n"
" let foo: Foo = Foo\n"
" let foo: (Foo|None) = Foo\n"

" new create(env: Env) => None\n"

Expand Down
4 changes: 2 additions & 2 deletions test/libponyc/dontcare.cc
Expand Up @@ -181,9 +181,9 @@ TEST_F(DontcareTest, CannotUseDontcareInCaseExpression)
{
const char* src =
"class C\n"
" fun c_ase(x: I32): I32 =>\n"
" fun c_ase(x: (I32 | None)): I32 =>\n"
" match x\n"
" | let x1: I32 => -1\n"
" | let x1: I32 => x1\n"
" | _ => 42\n"
" end";
TEST_ERRORS_1(src, "can't have a case with `_` only, use an else clause");
Expand Down
2 changes: 1 addition & 1 deletion test/libponyc/sugar.cc
Expand Up @@ -936,7 +936,7 @@ TEST_F(SugarTest, MatchWithNoElse)
" var create: U32\n"
" fun f(): U32 val =>\n"
" match(x)\n"
" |1=> 2\n"
" |1 => 2\n"
" end";

const char* full_form =
Expand Down