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

Fix specificity with bound varargs #16249

Merged
merged 1 commit into from
Jun 24, 2016
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 102 additions & 0 deletions src/jltypes.c
Original file line number Diff line number Diff line change
Expand Up @@ -2632,6 +2632,21 @@ static int type_eqv_with_ANY(jl_value_t *a, jl_value_t *b)

static int jl_type_morespecific_(jl_value_t *a, jl_value_t *b, int invariant);

jl_datatype_t *jl_fix_vararg_bound(jl_datatype_t *tt, int nfix)
{
assert(jl_is_va_tuple(tt));
assert(nfix >= 0);
jl_svec_t *tp = tt->parameters;
size_t ntp = jl_svec_len(tp);
jl_value_t *env[2] = {NULL, NULL};
JL_GC_PUSH2(&env[0], &env[1]);
env[0] = jl_tparam1(jl_tparam(tt, ntp-1));
env[1] = jl_box_long(nfix);
jl_datatype_t *ret = (jl_datatype_t*)jl_instantiate_type_with((jl_value_t*)tt, env, 2);
JL_GC_POP();
return ret;
}

static int jl_tuple_morespecific(jl_datatype_t *cdt, jl_datatype_t *pdt, int invariant)
{
size_t clenr = jl_nparams(cdt);
Expand Down Expand Up @@ -2843,6 +2858,93 @@ JL_DLLEXPORT int jl_type_morespecific(jl_value_t *a, jl_value_t *b)
return jl_type_morespecific_(a, b, 0);
}

int jl_args_morespecific_(jl_value_t *a, jl_value_t *b)
{
int msp = jl_type_morespecific(a,b);
int btv = jl_has_typevars(b);
if (btv) {
if (jl_type_match_morespecific(a,b) == (jl_value_t*)jl_false) {
if (jl_has_typevars(a))
return 0;
return msp;
}
if (jl_has_typevars(a)) {
type_match_invariance_mask = 0;
//int result = jl_type_match_morespecific(b,a) == (jl_value_t*)jl_false);
// this rule seems to work better:
int result = jl_type_match(b,a) == (jl_value_t*)jl_false;
type_match_invariance_mask = 1;
if (result)
return 1;
}
int nmsp = jl_type_morespecific(b,a);
if (nmsp == msp)
return 0;
}
if (jl_has_typevars((jl_value_t*)a)) {
int nmsp = jl_type_morespecific(b,a);
if (nmsp && msp)
return 1;
if (!btv && jl_types_equal(a,b))
return 1;
if (jl_type_match_morespecific(b,a) != (jl_value_t*)jl_false)
return 0;
}
return msp;
}

// Called when a is a bound-vararg and b is not a vararg. Sets the
// vararg length in a to match b, as long as this makes some earlier
// argument more specific.
int jl_args_morespecific_fix1(jl_value_t *a, jl_value_t *b, int swap)
{
jl_datatype_t *tta = (jl_datatype_t*)a;
jl_datatype_t *ttb = (jl_datatype_t*)b;
size_t n = jl_nparams(tta);
jl_datatype_t *newtta = jl_fix_vararg_bound(tta, jl_nparams(ttb)-n+1);
int changed = 0;
for (size_t i = 0; i < n-1; i++) {
if (jl_tparam(tta, i) != jl_tparam(newtta, i)) {
changed = 1;
break;
}
}
if (changed) {
JL_GC_PUSH1(&newtta);
int ret;
if (swap)
ret = jl_args_morespecific_(b, (jl_value_t*)newtta);
else
ret = jl_args_morespecific_((jl_value_t*)newtta, b);
JL_GC_POP();
return ret;
}
if (swap)
return jl_args_morespecific_(b, a);
return jl_args_morespecific_(a, b);
}

JL_DLLEXPORT int jl_args_morespecific(jl_value_t *a, jl_value_t *b)
{
if (jl_is_tuple_type(a) && jl_is_tuple_type(b)) {
jl_datatype_t *tta = (jl_datatype_t*)a;
jl_datatype_t *ttb = (jl_datatype_t*)b;
size_t alenf, blenf;
jl_vararg_kind_t akind, bkind;
jl_tuple_lenkind_t alenkind, blenkind;
alenf = tuple_vararg_params(tta->parameters, NULL, &akind, &alenkind);
blenf = tuple_vararg_params(ttb->parameters, NULL, &bkind, &blenkind);
// When one is JL_VARARG_BOUND and the other has fixed length,
// allow the argument length to fix the tvar
if (akind == JL_VARARG_BOUND && blenkind == JL_TUPLE_FIXED && blenf >= alenf) {
return jl_args_morespecific_fix1(a, b, 0);
}
if (bkind == JL_VARARG_BOUND && alenkind == JL_TUPLE_FIXED && alenf >= blenf) {
return jl_args_morespecific_fix1(b, a, 1);
}
}
return jl_args_morespecific_(a, b);
}

// ----------------------------------------------------------------------------

Expand Down
35 changes: 0 additions & 35 deletions src/typemap.c
Original file line number Diff line number Diff line change
Expand Up @@ -884,41 +884,6 @@ jl_typemap_entry_t *jl_typemap_insert(union jl_typemap_t *cache, jl_value_t *par
return newrec;
}

JL_DLLEXPORT int jl_args_morespecific(jl_value_t *a, jl_value_t *b)
{
int msp = jl_type_morespecific(a,b);
int btv = jl_has_typevars(b);
if (btv) {
if (jl_type_match_morespecific(a,b) == (jl_value_t*)jl_false) {
if (jl_has_typevars(a))
return 0;
return msp;
}
if (jl_has_typevars(a)) {
type_match_invariance_mask = 0;
//int result = jl_type_match_morespecific(b,a) == (jl_value_t*)jl_false);
// this rule seems to work better:
int result = jl_type_match(b,a) == (jl_value_t*)jl_false;
type_match_invariance_mask = 1;
if (result)
return 1;
}
int nmsp = jl_type_morespecific(b,a);
if (nmsp == msp)
return 0;
}
if (jl_has_typevars((jl_value_t*)a)) {
int nmsp = jl_type_morespecific(b,a);
if (nmsp && msp)
return 1;
if (!btv && jl_types_equal(a,b))
return 1;
if (jl_type_match_morespecific(b,a) != (jl_value_t*)jl_false)
return 0;
}
return msp;
}

static int has_unions(jl_tupletype_t *type)
Copy link
Member

Choose a reason for hiding this comment

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

Yes, +1 to moving this code.

{
int i;
Expand Down
12 changes: 12 additions & 0 deletions test/core.jl
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,18 @@ let T = TypeVar(:T, Tuple{Vararg{RangeIndex}}, true)
@test args_morespecific(t2, t1)
end

let T = TypeVar(:T, Any, true), N = TypeVar(:N, Any, true)
a = Tuple{Array{T,N}, Vararg{Int,N}}
b = Tuple{Array,Int}
@test args_morespecific(a, b)
@test !args_morespecific(b, a)
a = Tuple{Array, Vararg{Int,N}}
@test !args_morespecific(a, b)
@test args_morespecific(b, a)
end

# with bound varargs

# issue #11840
f11840(::Type) = "Type"
f11840(::DataType) = "DataType"
Expand Down