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.

Sign up for GitHub

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

Add callsuper macro. #20131

Closed
wants to merge 3 commits into from
Closed

Add callsuper macro. #20131

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
236fda2
Add callsuper macro.
simonbyrne Jan 19, 2017
d846bf2
add to docs
simonbyrne Jan 19, 2017
418a830
Merge branch 'master' into sb/callsuper
StefanKarpinski Jan 29, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions base/exports.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1382,6 +1382,7 @@ export
@polly,

@assert,
@callsuper,
@enum,
@label,
@goto,
Expand Down
44 changes: 44 additions & 0 deletions base/util.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -750,3 +750,47 @@ kwdef_val(::Type{Cwstring}) = Cwstring(C_NULL)
kwdef_val{T<:Integer}(::Type{T}) = zero(T)

kwdef_val{T}(::Type{T}) = T()

"""
@callsuper f(x::T, y::S, ...)

Call the method of function `f` with parameter signature specified by `T`, `S`, etc.
"""
macro callsuper(ex)
ex.head == :call || error("@invoke requires a call expression")
fname = ex.args[1]
args = ex.args[2:end]
types = Symbol[]
vals = Symbol[]
kwargs = Any[]
blk = quote end
for arg in args
if isa(arg,Expr) && arg.head == :kw
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not entirely correct. A few cases that should be handled and possibly tested are. (These are the reason I started but didn't finish the pr yet...)

  1. Evaluation order

    Currently :parameters are always evaluated the first. It's not necessarily the best choice but this should be consisent with normal calls.

  2. :parameters can have ...

  3. :parameters can have non :kw in it

  4. positional arguments can also have ...

  5. if parameter only have ...s and all of them are empty, the non-kw version should be called.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what non-:kw can a :parameters have other than ...?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any, iterable objects

julia> f(;kws...) = kws
f (generic function with 1 method)

julia> j = [:j, :k]
2-element Array{Symbol,1}:
 :j
 :k

julia> f(;:a=>:b, (:c, :d), [:e, :f], (:g, :h, :i), j)
5-element Array{Any,1}:
 (:a,:b)
 (:c,:d)
 (:e,:f)
 (:g,:h)
 (:j,:k)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yuyichao, wouldn't be easier all around if you just did this PR? There was a similar back and forth when I proposed a possible implementation

Copy link
Contributor

@yuyichao yuyichao Jan 22, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe. I started an implementation but haven't finished it yet (collecting data of what input should/can be supported and what generic fallback path I can use).....

push!(kwargs, QuoteNode(arg.args[1]))
push!(kwargs, esc(arg.args[2]))
elseif isa(arg,Expr) && arg.head == :parameters
for aarg in arg.args
push!(kwargs, QuoteNode(aarg.args[1]))
push!(kwargs, esc(aarg.args[2]))
end
else
val = gensym()
typ = gensym()
push!(vals, val)
push!(types, typ)
if isa(arg,Expr) && arg.head == :(::) && length(arg.args) == 2
push!(blk.args, :($typ = $(esc(arg.args[2]))))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, val should be evaluated before the type.

push!(blk.args, :($val = $(esc(arg.args[1]))::$typ))
else
push!(blk.args, :($val = $(esc(arg))))
push!(blk.args, :($typ = typeof($val)))
end
end
end
if isempty(kwargs)
push!(blk.args, :(invoke($(esc(fname)), Tuple{$(types...)}, $(vals...))))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And fname should be evaluated before the arguments.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, fname evaluation is a bit weird:

julia> f(args...; kwargs...) = (args,kwargs)
f (generic function with 1 method)

julia> (println("f"); f)((println("a"); "a"); [:y,(println("b"); "b")])
b
f
f
a
(("a",),Any[(:y,"b")])

julia> (println("f"); f)((println("a"); "a"); [:y => (println("b"); "b")]...)
b
f
a
f
f
(("a",),Any[(:y,"b")])

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a lowering bug. The lowering code should emit a temporary variable to hold f if it cannot proof that evaluating f is "pure".

else
push!(blk.args, :(invoke(Core.kwfunc($(esc(fname))), Tuple{Vector{Any}, typeof($(esc(fname))), $(types...)}, [$(kwargs...)], $(esc(fname)), $(vals...))))
end
return blk
end
1 change: 1 addition & 0 deletions doc/src/stdlib/base.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ Core.applicable
Core.invoke
Base.:(|>)
Base.:(∘)
Base.@callsuper
```

## Syntax
Expand Down
13 changes: 13 additions & 0 deletions test/misc.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -599,6 +599,19 @@ let
end
end

@testset "callsuper" begin
f18252(x; y=1) = x+y
f18252(x::Int; y=1) = x-y
@test 0 == @callsuper f18252(1)
@test -1 == @callsuper f18252(1, y=2)
@test -2 == @callsuper f18252(1; y=3)
@test 2 == @callsuper f18252(1::Any)
@test 3 == @callsuper f18252(1::Any, y=2)
@test 4 == @callsuper f18252(1::Any; y=3)
@test_throws MethodError @callsuper f18252(1, y=3, z=2)
@test_throws MethodError @callsuper f18252(1::Any, y=3, z=2)
end

abstract DA_19281{T, N} <: AbstractArray{T, N}
Base.convert{S,T,N}(::Type{Array{S, N}}, ::DA_19281{T, N}) = error()
x_19281 = [(), (1,)]
Expand Down