Skip to content

Commit

Permalink
Fix over-restrictive rule for generic cap pattern matching
Browse files Browse the repository at this point in the history 
The new rule for generic capability pattern matching introduced in
bab5b3a turned out to be too restrictive and disallowed legitimate
cases.

This changes the rule from

"every possible instantiation of the operand is a subtype of every
possible instantiation of the pattern"

to

"every possible instantiation of the operand is a subtype of some
possible instantiation of the pattern"

Closes ponylang#2584.
  • Loading branch information
Benoit Vey committed Mar 12, 2018
1 parent 41b80ee commit 2b5dc33
Show file tree
Hide file tree
Showing 4 changed files with 188 additions and 3 deletions.
111 changes: 111 additions & 0 deletions src/libponyc/type/cap.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,117 @@ bool is_cap_sub_cap_bound(token_id sub, token_id subalias, token_id super,
return false;
}

bool is_cap_match_cap(token_id operand_cap, token_id operand_eph,
token_id pattern_cap, token_id pattern_eph)
{
// Transform the cap based on the aliasing info.
cap_aliasing(&operand_cap, &operand_eph);
cap_aliasing(&pattern_cap, &pattern_eph);

if(pattern_eph == TK_EPHEMERAL)
{
// Operand must be ephemeral.
if(operand_eph != TK_EPHEMERAL)
return false;
}

if((operand_cap == pattern_cap) || (pattern_cap == TK_TAG))
return true;

// Every possible instantiation of the operand refcap must be a subtype of
// some possible instantiation of the pattern refcap.
switch(operand_cap)
{
case TK_ISO:
switch(pattern_cap)
{
case TK_TRN:
case TK_REF:
case TK_VAL:
case TK_BOX:
case TK_CAP_READ:
case TK_CAP_SHARE:
case TK_CAP_SEND:
case TK_CAP_ALIAS:
case TK_CAP_ANY:
return true;

default: {}
}
break;

case TK_TRN:
switch(pattern_cap)
{
case TK_REF:
case TK_VAL:
case TK_BOX:
case TK_CAP_READ:
case TK_CAP_SHARE:
case TK_CAP_SEND:
case TK_CAP_ALIAS:
case TK_CAP_ANY:
return true;

default: {}
}
break;

case TK_REF:
switch(pattern_cap)
{
case TK_BOX:
case TK_CAP_READ:
case TK_CAP_SHARE:
case TK_CAP_SEND:
case TK_CAP_ALIAS:
case TK_CAP_ANY:
return true;

default: {}
}
break;

case TK_VAL:
case TK_BOX:
case TK_CAP_READ:
switch(pattern_cap)
{
case TK_BOX:
case TK_CAP_READ:
case TK_CAP_SHARE:
case TK_CAP_SEND:
case TK_CAP_ALIAS:
case TK_CAP_ANY:
return true;

default: {}
}
break;

case TK_TAG:
case TK_CAP_SHARE:
case TK_CAP_SEND:
case TK_CAP_ALIAS:
case TK_CAP_ANY:
switch(pattern_cap)
{
case TK_CAP_SHARE:
case TK_CAP_SEND:
case TK_CAP_ALIAS:
case TK_CAP_ANY:
return true;

default: {}
}
break;

default: {}
}

return false;
}

bool is_cap_compat_cap(token_id left_cap, token_id left_eph,
token_id right_cap, token_id right_eph)
{
Expand Down
7 changes: 7 additions & 0 deletions src/libponyc/type/cap.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,13 @@ bool is_cap_sub_cap_constraint(token_id sub, token_id subalias, token_id super,
bool is_cap_sub_cap_bound(token_id sub, token_id subalias, token_id super,
token_id supalias);

/**
* Every possible instantiation of the operand is a subtype of some possible
* instantiation of the pattern.
*/
bool is_cap_match_cap(token_id operand_cap, token_id operand_eph,
token_id pattern_cap, token_id pattern_eph);

/**
* Every possible instantiation of the left side is locally compatible with
* every possible instantiation of the right side. This relationship is
Expand Down
6 changes: 3 additions & 3 deletions src/libponyc/type/matchtype.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ static matchtype_t is_nominal_match_entity(ast_t* operand, ast_t* pattern,

// If the operand does provide the pattern, but the operand refcap can't
// match the pattern refcap, deny the match.
if(!is_cap_sub_cap(ast_id(o_cap), ast_id(o_eph), tcap, teph))
if(!is_cap_match_cap(ast_id(o_cap), ast_id(o_eph), tcap, teph))
return MATCHTYPE_DENY;

// Otherwise, accept the match.
Expand Down Expand Up @@ -320,7 +320,7 @@ static matchtype_t is_entity_match_trait(ast_t* operand, ast_t* pattern,

// If the operand does provide the pattern, but the operand refcap can't
// match the pattern refcap, deny the match.
if(!is_cap_sub_cap(ast_id(o_cap), ast_id(o_eph), tcap, teph))
if(!is_cap_match_cap(ast_id(o_cap), ast_id(o_eph), tcap, teph))
return MATCHTYPE_DENY;

// Otherwise, accept the match.
Expand All @@ -335,7 +335,7 @@ static matchtype_t is_trait_match_trait(ast_t* operand, ast_t* pattern,
AST_GET_CHILDREN(pattern, p_pkg, p_id, p_typeargs, p_cap, p_eph);

// If the operand refcap can't match the pattern refcap, deny the match.
if(!is_cap_sub_cap(ast_id(o_cap), ast_id(o_eph),
if(!is_cap_match_cap(ast_id(o_cap), ast_id(o_eph),
ast_id(p_cap), ast_id(p_eph)))
return MATCHTYPE_DENY;

Expand Down
67 changes: 67 additions & 0 deletions test/libponyc/matchtype.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,8 @@ TEST_F(MatchTypeTest, GenericCap)

// Use ephemeral types for gencaps with unique caps in order to get unaliased
// match operand types.
// Use `cap` as the first argument of `is_matchtype`, and `cap_base` as the
// second argument.
ast_t* send_base = type_of("send");
ast_t* any_base = type_of("any");

Expand All @@ -523,6 +525,19 @@ TEST_F(MatchTypeTest, GenericCap)
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(read, box, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(read, tag, &opt));

ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(iso, read, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(trn, read, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(ref, read, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(val, read, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(box, read, &opt));
ASSERT_EQ(MATCHTYPE_DENY, is_matchtype(tag, read, &opt));

ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(read, read, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(read, send_base, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(read, share, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(read, alias, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(read, any_base, &opt));

// #send {iso, val, tag}
ASSERT_EQ(MATCHTYPE_DENY, is_matchtype(send, iso, &opt));
ASSERT_EQ(MATCHTYPE_DENY, is_matchtype(send, trn, &opt));
Expand All @@ -531,6 +546,19 @@ TEST_F(MatchTypeTest, GenericCap)
ASSERT_EQ(MATCHTYPE_DENY, is_matchtype(send, box, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(send, tag, &opt));

ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(iso, send_base, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(trn, send_base, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(ref, send_base, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(val, send_base, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(box, send_base, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(tag, send_base, &opt));

ASSERT_EQ(MATCHTYPE_DENY, is_matchtype(send, read, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(send, send_base, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(send, share, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(send, alias, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(send, any_base, &opt));

// #share {val, tag}
ASSERT_EQ(MATCHTYPE_DENY, is_matchtype(share, iso, &opt));
ASSERT_EQ(MATCHTYPE_DENY, is_matchtype(share, trn, &opt));
Expand All @@ -539,6 +567,19 @@ TEST_F(MatchTypeTest, GenericCap)
ASSERT_EQ(MATCHTYPE_DENY, is_matchtype(share, box, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(share, tag, &opt));

ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(iso, share, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(trn, share, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(ref, share, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(val, share, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(box, share, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(tag, share, &opt));

ASSERT_EQ(MATCHTYPE_DENY, is_matchtype(share, read, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(share, send_base, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(share, share, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(share, alias, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(share, any_base, &opt));

// #alias {ref, val, box, tag}
ASSERT_EQ(MATCHTYPE_DENY, is_matchtype(alias, iso, &opt));
ASSERT_EQ(MATCHTYPE_DENY, is_matchtype(alias, trn, &opt));
Expand All @@ -547,6 +588,19 @@ TEST_F(MatchTypeTest, GenericCap)
ASSERT_EQ(MATCHTYPE_DENY, is_matchtype(alias, box, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(alias, tag, &opt));

ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(iso, alias, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(trn, alias, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(ref, alias, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(val, alias, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(box, alias, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(tag, alias, &opt));

ASSERT_EQ(MATCHTYPE_DENY, is_matchtype(alias, read, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(alias, send_base, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(alias, share, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(alias, alias, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(alias, any_base, &opt));

// #any {iso, trn, ref, val, box, tag}
ASSERT_EQ(MATCHTYPE_DENY, is_matchtype(any, iso, &opt));
ASSERT_EQ(MATCHTYPE_DENY, is_matchtype(any, trn, &opt));
Expand All @@ -555,6 +609,19 @@ TEST_F(MatchTypeTest, GenericCap)
ASSERT_EQ(MATCHTYPE_DENY, is_matchtype(any, box, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(any, tag, &opt));

ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(iso, any_base, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(trn, any_base, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(ref, any_base, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(val, any_base, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(box, any_base, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(tag, any_base, &opt));

ASSERT_EQ(MATCHTYPE_DENY, is_matchtype(any, read, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(any, send_base, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(any, share, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(any, alias, &opt));
ASSERT_EQ(MATCHTYPE_ACCEPT, is_matchtype(any, any_base, &opt));

if(send != send_base)
ast_free_unattached(send);

Expand Down

0 comments on commit 2b5dc33

Please sign in to comment.