Skip to content

Commit

Permalink
fix call overloading for type constructors, add some tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@stevengj
stevengj committed Aug 12, 2014
1 parent 4a58fa4 commit 5b0aedd
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 5 deletions.
5 changes: 4 additions & 1 deletion base/base.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ using Core: Intrinsics, arraylen, arrayref, arrayset, arraysize,
tuplelen, tupleref, convert_default, kwcall,
typeassert, apply_type

import Core.Array # to add methods
import Core: Array, call # to add methods

const NonTupleType = Union(DataType,UnionType,TypeConstructor)

Expand All @@ -15,6 +15,9 @@ convert(T, x) = convert_default(T, x, convert)
convert(::(), ::()) = ()
convert(::Type{Tuple}, x::Tuple) = x

# allow convert to be called as if it were a single-argument constructor
call{T}(::Type{T}, x) = convert(T, x)

argtail(x, rest...) = rest
tupletail(x::Tuple) = argtail(x...)

Expand Down
1 change: 0 additions & 1 deletion base/boot.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,4 +245,3 @@ typealias ByteString Union(ASCIIString,UTF8String)
include(fname::ByteString) = ccall(:jl_load_, Any, (Any,), fname)

call(f::Function, args...; kws...) = f(args...; kws...)
call{T}(::Type{T}, x) = convert(T, x)
11 changes: 9 additions & 2 deletions src/codegen.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2201,9 +2201,16 @@ static Value *emit_call(jl_value_t **args, size_t arglen, jl_codectx_t *ctx,

jl_function_t *f = (jl_function_t*)static_eval(a0, ctx, true);
if (f != NULL) {
Value *result;
headIsGlobal = true;
Value *result = emit_known_call((jl_value_t*)f, args, nargs, ctx,
&theFptr, &f, expr);
if (f->fptr != jl_f_no_function) {
result = emit_known_call((jl_value_t*)f, args, nargs, ctx,
&theFptr, &f, expr);
}
else {
result = emit_known_call((jl_value_t*)jl_call_func,
--args, ++nargs, ctx, &theFptr, &f, expr);
}
if (result != NULL) return result;
}
bool specialized = true;
Expand Down
2 changes: 1 addition & 1 deletion src/interpreter.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ static jl_value_t *eval(jl_value_t *e, jl_value_t **locals, size_t nl)
}
}
jl_function_t *f = (jl_function_t*)eval(args[0], locals, nl);
if (jl_is_func(f))
if (jl_is_func(f) && f->fptr != jl_f_no_function)
return do_call(f, &args[1], nargs-1, NULL, locals, nl);
else
return do_call(jl_call_func, args, nargs, (jl_value_t*)f,
Expand Down
18 changes: 18 additions & 0 deletions test/core.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1829,3 +1829,21 @@ end

# issue #7582
aₜ = "a variable using Unicode 6"

# call and constructor overloading (#1470, #2403)
typealias FooInt32 Int32
@test isa(FooInt32(3), FooInt32)
Base.call(x::Int, y::Int) = x + 3y
issue2403func(f) = f(7)
let x = 10
@test x(3) == 19
@test issue2403func(x) == 31
end
type Issue2403
x
end
Base.call(i::Issue2403, y) = i.x + 2y
let x = Issue2403(20)
@test x(3) == 26
# @test issue2403func(x) == 34 -- FIXME

This comment has been minimized.

Sign in to view

Copy link
@stevengj

stevengj Aug 12, 2014

Author Member

@JeffBezanson, this line gives

ERROR: type: issue2403func: in apply, expected Function, got Issue2403
 in issue2403func at none:1

but the other tests work. What am I missing?

(On the phone, you mentioned something about having to change type inference somewhere, but I'm not quite sure what you were referring to. Something in inference.jl?)
end

5 comments on commit 5b0aedd

@stevengj
Copy link
Member Author

Choose a reason for hiding this comment

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

Another tricky case:

julia> type Foo
       x
       Base.convert(::Type{Foo}, x) = new(int(x))
       end

julia> convert(Foo, 7)
Foo(7)

julia> Foo(3.2)
ERROR: `Foo` has no method matching Foo(::Float64)

julia> call(Foo, 3.2)
Foo(3)

julia> methods(Foo)
#0 methods for generic function "Foo":

The cleanest way to resolve this might be to get rid of constructors as "generic functions" per se, and just make them methods added to call. That way there is no question whether you want to dispatch on call or on Foo methods.

@JeffBezanson
Copy link
Member

Choose a reason for hiding this comment

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

Yes, that's what I was saying yesterday --- constructors should be dispatched on types by call the way convert is. This is internally simpler and more general as well.

@stevengj
Copy link
Member Author

Choose a reason for hiding this comment

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

Here's a weird behavior:

julia> Base.call(x::Int, y::Int) = x + 3y
call (generic function with 3 methods)

julia> let x=10
       x(3) == 19
       end
true

julia> let x=10
       Base.Test.@test x(3) == 19
       end
ERROR: test error during x(3) == 19
type: anonymous: in apply, expected Function, got Int64
 in anonymous at test.jl:62
 in do_test at test.jl:37
 in anonymous at no file:2

@JeffBezanson, how is macro expansion of @test triggering a different code path here?

Update: seems to be a problem with globals in functions?

julia> Base.call(x::Int, y::Int) = x + 3y
call (generic function with 3 methods)

julia> x = 3
3

julia> x(7)
24

julia> g = () -> x(3)
(anonymous function)

julia> g()
ERROR: type: anonymous: in apply, expected Function, got Int64
 in anonymous at none:1

julia> function h()
       x(3)
       end
h (generic function with 1 method)

julia> h()
ERROR: type: h: in apply, expected Function, got Int64
 in h at none:2

@stevengj
Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, I've just added support for switching over to call from jl_f_apply and jl_f_kwcall, but it's still not working, so I must be missing another place. I still haven't changed applicable and invoke, but I don't think it could be one of those because the error says in apply.

@stevengj
Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, the problem is that emit_func_check needs to go away...

I tried to do this with 762a727, but it broke something else.

Please sign in to comment.