search

JuliaLabs / Cassette.jl

Overdub Your Julia Code
Other
371 stars 35 forks source link

Fix for #89, overdub with invoke and optional arguments #137

Closed tkoolen closed 5 years ago

tkoolen commented 5 years ago

I'm still trying to wrap my head around the package internals, but this seems to fix #89 and I think this kind of makes sense. The old behavior of adding a constant invoke_offset of 2 to all argument slots seemed kind of weird to me, since untagged_args in __overdub_generator__ ends up looking like (typeof(invoke), typeof(f), Type{Tuple{Int64}}, Int64), i.e. the first and third arguments should be skipped, rather than the first two.

tkoolen commented 5 years ago

I suppose the vararg branch should be fixed as well, right?

https://github.com/jrevels/Cassette.jl/blob/427dabc4cb5da00b1c84a2dcdcb9640e0f05e307/src/overdub.jl#L206

tkoolen commented 5 years ago

I've cleaned up the offset calculation code a little (removed the is_calloverload special case). I believe the vararg handling is (and was) right as well.

tkoolen commented 5 years ago

I verified that the example from https://github.com/jrevels/Cassette.jl/issues/127#issue-451158553 still works with this change.

tkoolen commented 5 years ago

Do you need anything else from me?

vchuravy commented 5 years ago

Thanks!