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

For caching staged methods, don't restrict the number of varargs to match #7935

Closed
wants to merge 8 commits into from
Closed

Conversation

timholy
Copy link
Sponsor Member

@timholy timholy commented Aug 9, 2014

Note this is a PR against kf/staged, not master.

This fixes the main remaining issues with splatting:

julia> stagedfunction h(a...)
           @show a
           :nothing
       end
h (generic function with 1 method)

julia> h(1:5, 3, 3:3)
a => (UnitRange{Int64},Int64,UnitRange{Int64})

julia> h(1:5, 3, 3:3)

julia> h(1:5, 3:3, 3)
a => (UnitRange{Int64},UnitRange{Int64},Int64)

julia> h(1:5, 3:3, 3)

julia> h(1:5, 3)   # not correct

julia> h(3, 1:5)
a => (Int64,UnitRange{Int64})

I suspect that one remaining problem is due to this line. What's the reason for allowing a match when n == lensig-1?

Keno and others added 8 commits July 1, 2014 14:53
This will have to be changed once we switch the way we do func_for_method,
but I will await @JeffBezanson's judgement on how to do that best, so I'll
use this to go head first.
…atch

Also adds `stagedfunction` as a keyword to Kate's julia.xml file
@timholy
Copy link
Sponsor Member Author

timholy commented Aug 9, 2014

That relatively complicated conditional can apparently be simplified as in my second commit. But it seems that n+1 is important; if I change it to n and try a clean build, I get this:

...
inference.jl
osutils.jl
Killed
*** This error is usually fixed by running 'make clean'. If the error persists, try 'make cleanall'. ***

Interestingly, if I let the build go through with n+1 and then change it to n and just rebuild libjulia.so, the tests mostly work (until we get to the staged.jl test). Is this for some special case in inference bootstrapping?

@Keno
Copy link
Member

Keno commented Aug 23, 2014

I took these commits when I rebased #7474, so closing this in favor of that.

@Keno Keno closed this Aug 23, 2014
@timholy
Copy link
Sponsor Member Author

timholy commented Aug 24, 2014

Great, thanks.

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.

3 participants