Fix #10502 (properly cache vararg stagedfunctions)

[mbauman]

Previously, we only checked if the first method in the method table was staged in order to determine whether or not to newly cache a vararg type. This patch changes the cache_method function to get an extra argument - isstaged for the specific method it is caching.

This is a fairly simple fix, but I'd like a second set of eyes to make sure I'm doing this correctly as I'm not as familiar with the C source. cc: @JeffBezanson @Keno

[jakebolewski]

This was how it was originally implemented, but in #8944 @timholy changed it? Maybe the culmination of changes makes this work now.

[mbauman]

I didn't run the whole test suite, but the staged All tests passed on my Mac. We'll see what CI says…

Edit: Linux is good, OS X timed out, like usual. There was a Win32 failure that I hadn't seen before, but the queue was empty so I rebuilt the PR and it passed the second time around. I wish there was a way to search through AppVeyor/Travis logs so I could see if a failure is new or not.

[mbauman]

Very interesting; I didn't know the history here. My patch is different from the inverse of 1c7664a. There, jl_gf_invoke and jl_mt_assoc_by_type called cache_method with an always-false isstaged, whereas I use the method's m->isstaged (simply because it looked like each caller was working with a method m).

[timholy]

Very nice, @mbauman. This definitely looks like the right fix, it combines the best of both worlds.

I say merge, but Jeff knows way more about gf.c than me, so feel free to wait for his comments.

[mbauman]

Thanks for looking at this. I'll hit the button since it's easier to fix the bug than work around it 😄. If this turns out to be the wrong approach, it's very simple to revert or otherwise change these 5 lines.

[JeffBezanson]

Yes this is fine. It really looks like cache_method should take a whole Method object, but that's just a simple refactoring.