search

JuliaLabs / Cassette.jl

Overdub Your Julia Code
Other
371 stars 35 forks source link

Make offset an argument of overdub_pass! #168

Closed MasonProtter closed 4 years ago

MasonProtter commented 4 years ago

Given the hesitance to merge https://github.com/jrevels/Cassette.jl/pull/157, I'd like to implement ReflectOn in a separate package. However, overdub_pass! is a gigantic function and in order to make ReflectOn work, I'd need to copy paste all of overdub_pass! just so I can add offset=2 for the ReflectOn method. Instead, I think it'd be nice if offset could just be an argument to overdub_pass!. I think this would be beneficial to others who might want to tweak or alter overdub as well in a similar way.

vchuravy commented 4 years ago

I am less opposed of this since it is relatively small. What would actually be rather interesting is to take Shashi's ReflectOn idea and use it to implement Cassette's Tagging as an extension.

MasonProtter commented 4 years ago

I don't know much about tagging, but naïvely, it seems like it's a better candidate for a separate package than ReflectOn is.

RelfectOn seems like an almost quintessential cassette action. Take a method call and force it to go to a different method that may be overly restrictive.

MasonProtter commented 4 years ago

I'm not actually sure yet if this PR would be sufficient to get ReflectOn working in a separate package. I think there's an issue with how overdub is defined blocking my strategy from working, so merging this may be pointless.

vchuravy commented 4 years ago

RelfectOn seems like an almost quintessential cassette action. Take a method call and force it to go to a different method that may be overly restrictive.

Right my reservation is that tagging and ReflectOn are supposed to do pretty much the same thing, but they do it in parallel. I would be happy to make tagging less of a core Cassette thing.

MasonProtter commented 4 years ago

@shashi any thoughts?

MasonProtter commented 4 years ago

Right my reservation is that tagging and ReflectOn are supposed to do pretty much the same thing, but they do it in parallel. I would be happy to make tagging less of a core Cassette thing.

Tagging can only do the same thing as something like RelfectOn if you have an actual value to pass around. If you don't have a legit value to tag, ReflectOn is more general.

shashi commented 4 years ago

Yeah I think in theory we can add ReflectOn here and move the Tagging stuff out into another package and have it rely on ReflectOn. I'm not a 100% sure that that wouldn't need more features from Cassette.