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

Check for Vararg{T,N} in typemap: sig_match_by_type_simple #16627

Merged
merged 1 commit into from
Jun 1, 2016

Conversation

timholy
Copy link
Member

@timholy timholy commented May 28, 2016

Fixes #16623

@timholy timholy mentioned this pull request May 28, 2016
@timholy
Copy link
Member Author

timholy commented May 28, 2016

I would classify this as a "narrow" fix, but it works in the cases I tested. @vtjnash, since you know typemap.c well, it might make some sense to see if you can think of cases that this simple fix wouldn't catch.

@vtjnash
Copy link
Member

vtjnash commented May 29, 2016

also likely need the same fix @ https://github.com/JuliaLang/julia/pull/16627/files#diff-213db5d6a6c93b12384ade73dbb25e1aR155, although I don't know of any case where that can be triggered (currently). Might also want to check that very long argument list method matches ending in a limited vararg don't get incorrectly truncated as they get inserted into the cache to look like an unlimited vararg.

@timholy
Copy link
Member Author

timholy commented May 29, 2016

Did you mean to link somewhere else? That link is for the diff in this PR.

@vtjnash
Copy link
Member

vtjnash commented May 30, 2016

oh, i forgot that github won't let me link to a line number for a PR. the corrected link to sig_match_simple is

if (va) {

@timholy timholy force-pushed the teh/dispatch_varargN branch from fab4a62 to ea18b98 Compare May 31, 2016 22:31
@timholy timholy merged commit 002f081 into master Jun 1, 2016
@timholy timholy deleted the teh/dispatch_varargN branch June 1, 2016 08:56
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