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

RFC: Macro for delegating methods to the fields of a composite type #3292

Closed
wants to merge 1 commit into from

Conversation

johnmyleswhite
Copy link
Member

As discussed long ago on the mailing list (https://groups.google.com/d/msg/julia-dev/MV7lYRgAcB0/s6_RMeHLpEYJ), I've created a simple @delegate macro that allows one to delegate methods to the fields of a composite types. An example is shown below in which a new container type can delegate methods like size and length to an array contained inside of it:

type MyContainer
    elems::Array
end

importall Base

@delegate MyContainer.elems [size, length]

x = MyContainer([1, 2, 3, 4])

size(x)
length(x)

Right now, this only supports functions for which the first argument is the field which delegated methods need. Before we merge this, I think we'll want to find a clean generalization.

@johnmyleswhite
Copy link
Member Author

@kmsquire: This one will be in RFC for a long time I suspect. It would be good to hear your opinion.

@kmsquire
Copy link
Member

kmsquire commented Jun 5, 2013

For the patch itself, it's easy to understand and very clean--nice job!

I'll try to get an example mocked up of a place where this would be useful--OrderedDicts come to mind. That would enable the discussion to move forward on whether this is the best way to implement wrappers, or if there are better methods.

@diegozea
Copy link
Contributor

+1

@StefanKarpinski
Copy link
Member

@JeffBezanson, any thoughts on this? I'm somehow not entirely sold on this, but I can't put my finger on why.

@StefanKarpinski
Copy link
Member

To be clear, I think this is a good implementation of the delegation approach we discussed. I'm just not entirely certain this is how we should handle delegation in the language. It feels kind of ad hoc and as though we should provide something more powerful in the language.

@johnmyleswhite
Copy link
Member Author

I totally agree. This is way too special-cased. It only looks clean for functions with exactly this argument structure: otherwise it's totally broken. I just wanted to stir up discussion with something definite.

@kmsquire
Copy link
Member

@quinnj
Copy link
Member

quinnj commented Jun 26, 2013

I've been thinking about this a little lately. It seems like it would be more convenient to be able to delegate from within a type definition. I have no idea if this is feasible or potentially destructive, but it'd be convenient to be able to note than a particular field should be "promoted" per se to be used automatically by methods calling the composite type. In the original case here, when length was called on MyContainer and the elems field had been "promoted", it would automatically dispatch on the field instead of saying no method. The obvious restriction here is that you could only "promote" one field of a certain type, otherwise it'd be ambiguous. Anyway, just a few thoughts mulling around.

@malmaud
Copy link
Contributor

malmaud commented Dec 3, 2013

It would be cool if delegation supported a use-case like

immutable PersonID
  id::Int
end

people::Vector{Person} =  ...
ids = [PersonID(1), PersonID(3)]
@delegate PersonID.id :use_as_index
@assert people[ids] == [people[1], people[3]]

It could be done with zero overhead, since we could already do

people[reinterpret(Int, ids)]

and would be convenient over giving a new definition of getindex for each integer index wrapper type.

@stevengj
Copy link
Member

stevengj commented Feb 7, 2014

Couldn't the macro use methodswith to automatically delegate all methods, including multi-argument methods, except where the corresponding methods for MyContainer have already been defined?

@johnmyleswhite
Copy link
Member Author

We could do that. It would certainly make this macro more useful.

@porterjamesj
Copy link
Contributor

+1 for the methodswith autodelegation thing.

@karbarcca I brought up basically that idea with @StefanKarpinski once and he expressed some skepticism, as well as pointing out that this has some implications with respect to type inference b/c calls that you could previously infer to be no method errors now sometimes will succeed.

@stevengj
Copy link
Member

stevengj commented Feb 7, 2014

I confused as to how type inference is affected. The proposed macro is equivalent to just declaring a bunch of methods manually, after all.

@porterjamesj
Copy link
Contributor

@stevengj sorry, I was talking about two different things above, The macro involving methodswith doesn't affect type inference.

The other thing I was talking about was something similar to @karbarcca's idea, which (as I understand it) is to change the language so you can declare some field of a type to be it's delegate, a la:

type MyMatrixWrapper
    delegate data::Matrix
    info::Dict
    # whatever other fields you need here . . .
end

and then whenever a call like:

f(A::MyMatrixWrapper, x::Int, y::Int)

would throw a no method error, this:

f(A.data, x, y)

would be tried instead, which as I understand it would affect type inference because things that used to be inferred to throw no method errors now sometimes will succeed.

@johnmyleswhite
Copy link
Member Author

I also like @stevengj idea: as long as the macro simply expands to a tedious bit of existing code, it can't have any bad effects that weren't already possible. And it will make the simplest kind of delegation pattern, wrapping a single type in a new immutable, a lot easier.

@porterjamesj
Copy link
Contributor

The only potential downside to the methodswith idea is that it only will work for things that are already defined when the macro is called. So something like this could be confusing:

module A:

module A
    type MatrixWrapper
        data::Matrix
        # other fields
    end

    @autodelegate MatrixWrapper :data

    # functions specific to MatrixWrapper
end

module B:

module B
    function f(x::Matrix)
        # some B-specific Matrix operations
    end
end

Here calling f on a MatrixWrapper fails because f wasn't defined on Matrices when the autodelegation was done:

using A
using B

A = MatrixWrapper(...)
f(A) #throws no method

Whereas here it works:

using B
using A

A = MatrixWrapper(...)
f(A) #works as you would expect

This is a pretty small issue I think, but it is potentially confusing.

@toivoh
Copy link
Contributor

toivoh commented Feb 16, 2014

I guess that we are talking about methodswith(T,true) for the T in
question? Otherwise, it seems that we would create a leak in the
abstraction of multiple dispatch.

On Sat, Feb 8, 2014 at 7:15 PM, James J Porter [email protected]:

The only potential downside to the methodswith idea is that it only will
work for things that are already defined when the macro is called. So
something like this could be confusing:

module A:

module A
type MatrixWrapper
data::Matrix
# other fields
end

@autodelegate MatrixWrapper :data

# functions specific to MatrixWrapperend

module B:

module B
function f(x::Matrix)
# some B-specific Matrix operations
endend

Here calling f on a MatrixWrapper fails because f wasn't defined on
Matrices when the autodelegation was done:

using Ausing B
A = MatrixWrapper(...)f(A) #throws no method

Whereas here it works:

using Busing A
A = MatrixWrapper(...)f(A) #works as you would expect

This is a pretty small issue I think, but it is potentially confusing.

Reply to this email directly or view it on GitHubhttps://github.com//pull/3292#issuecomment-34551204
.

@simonster
Copy link
Member

It would be useful for some abstractions (e.g. JuliaData/DataFrames.jl#571) if the following could be made to work:

type Wrapper{T}
    X::T
end
@delegate Wrapper.X
sum(Wrapper([1, 2, 3])) == 6

As far as I can tell, this is different from the above in that we don't know the types that we'll need to delegate until a Wrapper is actually constructed. I don't see how it would be possible to do this with a macro. One approach that I might try is to construct the methods when Wrapper is constructed, but this seems like it will mess up type inference.

A better approach might be to have some kind of catch-all staged method that gets called for a specific method signature regardless of the method name. I'm not sure if that can be done, but it's also another possible approach to #1974 that would admit both Julia-style dispatch and dispatch with the .. operator for PyCall and JavaCall, at the expense of creating functions in some cases where they aren't really useful.

@quinnj
Copy link
Member

quinnj commented Apr 16, 2014

I've been thinking about this a little more lately. I wonder if the concept of typealiases could have a "stronger" brother (typepeer or typewrapper) that could be used to essentially clone a concrete type. So

typepeer MyIntArray Array{Int,1}
a = [1:10]
b = MyIntArray(a)
sum(b) == 55

The idea being that there's an official way to wrap another type and automatically get all it's behavior. It's stronger than a typealias though, because MyIntArray is actually its own standalone type, but has behavior auto-inherited from Array{Int,1} (i.e. where ever a method accepts an Array{Int,1}, it would take MyIntArray). I guess it would kind of be like allowing sub-typing of concrete types, but without the ability to change the type's structure at all (so maybe the notation would just be type MyIntArray <: Array{Int,1} end. This would provide a very easy/intuitive ability to wrap types, define a few custom methods, and be on your way.

Though I pause to wonder if this is actually needed that much and if the subtyping of concrete types would be too heretical.

@porterjamesj
Copy link
Contributor

@karbarcca the problem I see with that approach is that I often find when I write wrappers I want to add an extra field (a prominent example is DataArrays adding the missing data bitmask).

@porterjamesj
Copy link
Contributor

@StefanKarpinski has stated pretty unambiguously in the past that subtyping concrete types will never be allowed since it prevents the compiler from inferring the size of something once it's concrete type is known, which is a good argument. I proposed the approach of declaring one field of a type to be it's delegate, and then having any method lookup that would be fail be retryed using a type's delegate in it's place if the type has one, to which Stefan responded it would be pretty hard to convince him/Jeff it was a good idea, at which point I stopped pushing the issue :)

@quinnj
Copy link
Member

quinnj commented Apr 17, 2014

@porterjamesj, if you're wanting to add a field, then it isn't really a case of the kind of "pure wrapping" that I was aiming at (though I realize this issue is probably more about your case); the case I'm talking about is a pure wrapper, i.e. a type with only one typed concrete field.
I'm aware of the discussions for why subtyping concrete types isn't allowed, but I attempted addressing this when I mention that this kind of concrete type wrapping/subtyping wouldn't allow any changing of type structure (adding/removing fields), thus ensuring the subtype has the same bit layout as it's parent, and allowing the compiler to safely know what it's working with. That's why I started out mentioning the analogue to typealias, but stronger because the new type would come with its own "concreteness" and "dispatchability" even though for all intents and purposes it's the exact same underlying structure.
I realize this doesn't address the delegation issue overall, but I think it would address a fair number of wrapping concerns that have cropped up here and there. But yes, I realize this may go against a core Julia "Stefanism"....

@simonster
Copy link
Member

If I am understanding correctly, the key to this proposal is that MyIntArray is not a subtype of Array{Int,1}, since that would appear to pose problems regardless of whether the bit layout is the same. (If you store a MyInt <: Int in a Vector{Int}, where does the type information go?) Instead, it behaves kind of like a subtype, but only for the purpose of dispatch. Method tables are effectively rewritten so that all occurrences of Array{Int,1} (or its supertypes) become Union(Array{Int,1},MyIntArray). Functions are still specialized differently for Int and MyIntArray, and if you store a MyIntArray in a field that's typed as Array{Int,1}, it gets converted to an Array{Int,1} (perhaps just by the implicit call to convert). While I haven't thought this through in full, it seems tractable to me. My main worry is the general question of whether a new language feature is truly necessary, particularly since this feature alters dispatch in a way that may not be all that intuitive.

@kmsquire
Copy link
Member

@karbarcca, I proposed something similar (though not identical) here: #3427. Jeff initially dismissed it, but seemed more open recently, pending #1470.

@mauro3
Copy link
Contributor

mauro3 commented Apr 17, 2014

@karbarcca there was some discussion about the typepeer idea here https://groups.google.com/d/msg/julia-dev/v8B1tI_NB5E/tk98D0iKopYJ
Jeff reckoned that it is impossible.

@quinnj
Copy link
Member

quinnj commented Apr 17, 2014

Ha, @mauro3, thanks for the link. Crazy that we suggested almost the exact same thing. Thanks for the link @kmsquire as well. It seems like there may be some wiggle room in between a typealias and full blown typecopy to suit some cases here.

I guess the other problem is I don't really understand the implementation details of the type hierarchy and dispatch. It seems the plumbing would be too messed up to flag a concrete type as a parent of another, even if the children were guaranteed the same memory layout/fields/etc.

@johnmyleswhite
Copy link
Member Author

This branch is very badly out-of-date, but I suspect we still are interested in eventually coming up with an effective API for doing delegation. Not sure whether to close or keep open.

@quinnj
Copy link
Member

quinnj commented Aug 14, 2014

Sounds like, even if not immediately, this will be taken care of with #1470.

@kmsquire
Copy link
Member

@quinnj, how's that?

@quinnj
Copy link
Member

quinnj commented Aug 14, 2014

See Stefan's comment here and Jeff's comment here

@kmsquire
Copy link
Member

Yup, thanks Jacob!

@quinnj
Copy link
Member

quinnj commented Aug 20, 2014

I vote we close this.

@johnmyleswhite
Copy link
Member Author

One vote's enough for me.

@benninkrs
Copy link

Sorry to revisit a closed issue, but can anyone say what the status of this is? As far as I can tell, the language features that were envisioned to support delegation (namely the ability to subtype functions and overload call for these functions, see #1470) have not been implemented.

@Ismael-VC
Copy link
Contributor

Ismael-VC commented Nov 22, 2017

@quinnj could we reopen this PR? (the original patch works with only a minor change) Or I can make a new one and add docs and tests. Now that macros are generic, we can cover other corner cases more easily.

@Ismael-VC
Copy link
Contributor

Something like this I mean:

using Base.Meta: quot

macro delegate(source::Expr, targets::Expr)
    source.head  :. && error("syntax: expected `Foo.bar` form")
    targets.head  :vect && error("syntax: expected vector of function names")

    type_name = source.args[1]
    field_name = source.args[2].value
    func_names = targets.args
    n = length(func_names)
    block = Expr(:block)

    for func_name in func_names
        expr = quote
            function $func_name(a::$type_name, args...)
                $func_name(a.$field_name, args...)
            end
        end

        push!(block.args, expr)
    end
    return esc(block)
end

macro delegate(source::Expr, target::Symbol)
    source.head  :. && error("syntax: expected `Foo.bar` form")

    type_name = source.args[1]
    field_name = source.args[2].value

    :($target(a::$type_name, args...) = $target(a.$field_name, args...)) |> esc
end

I'd just need the syntax you guys expect in all corner cases, to implement it. Julia 1.0 is on the horizon and I really think we should make composition via delegation, easier, more julian.

@StefanKarpinski
Copy link
Member

Feature freeze for 1.0 is December 15th, so we're not adding any more features to the release at this point without very significant justification. However, new features can be added in 1.x versions as long as they don't break older code.

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.