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

RFC: add a typemap level specifically for Any #16081

Merged
merged 1 commit into from
May 13, 2016
Merged

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented Apr 27, 2016

I don't think this comes up often in the cache, but I think it may be beneficial in a few cases. specifically, it ensures the following sort of cache signature can hit one of the optimized caches:

f(a::ANY, b) = ...

or the following set of methods gets a fast lookup path instead of the linear list scan:

for i = 1:100; @eval f(a, b::Val{i}) = i; end

this reveals a couple of missing ambiguity warnings stemming from call -> convert translation. I'm assuming that fallback method will go away so I'm not working around them even though it's probably now calling the wrong method. (except I had to work around the one that made the core test fail).
edit: that was from a serious bug in the PR. those methods are probably ambiguous, but the detection I thought I was seeing in this PR was an implementation mistake

@vtjnash vtjnash force-pushed the jn/typemap-any branch 4 times, most recently from 56fd188 to 3249dbc Compare May 10, 2016 17:51
@@ -16,6 +16,11 @@
extern "C" {
#endif

static int jl_is_any(jl_value_t *t1)
{
return (t1 == (jl_value_t*)jl_any_type || (jl_is_typevar(t1) && ((jl_tvar_t*)t1)->ub == (jl_value_t*)jl_any_type && !((jl_tvar_t*)t1)->bound));
Copy link
Contributor

Choose a reason for hiding this comment

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

wrap this

@vtjnash vtjnash merged commit d93e3e4 into master May 13, 2016
@vtjnash vtjnash deleted the jn/typemap-any branch May 13, 2016 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants