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

Request to change default inner constructors #7071

Closed
quinnj opened this issue Jun 1, 2014 · 2 comments
Closed

Request to change default inner constructors #7071

quinnj opened this issue Jun 1, 2014 · 2 comments
Assignees
Labels
needs decision A decision on this change is needed
Milestone

Comments

@quinnj
Copy link
Member

quinnj commented Jun 1, 2014

#4026 changed the default inner constructor to make calls to convert, a la:

type A
    x::Int64
    #implicit inner constructor
    #A(x) = new(convert(Int64,x))
end

The problem I see with this is that it's too easy to "lose" your only inner constructor for your type.

In  [2]: type A
    a::Int64
end

In  [3]: A(1)

Out [3]: A(1)

In  [4]: A(x) = A(do_my_own_fallback_conversion(x))

Out [4]: A (constructor with 1 method)

In  [5]: A(1.0)

Out [5]: ERROR: stack overflow
while loading In[5], in expression starting on line 1

Luckily, it seems like there's a fairly easy way to make this more intuitive:

type A
    x::Int64
    #implicit inner constructors
    #A(x::Int64) = new(x)
    #A(x) = new(convert(Int64,x))
end

Then when a user thinks to define her own fallback constructor, things don't break!

Reference: https://groups.google.com/forum/#!searchin/julia-dev/gotcha/julia-dev/7WU_HimIsJc/5mj-ns-WN_gJ

@JeffBezanson
Copy link
Member

Seems it might be a good idea to provide both by default.

@JeffBezanson JeffBezanson added this to the 0.3 milestone Jun 3, 2014
@JeffBezanson JeffBezanson self-assigned this Jun 4, 2014
@quinnj
Copy link
Member Author

quinnj commented Jun 5, 2014

Thanks Jeff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs decision A decision on this change is needed
Projects
None yet
Development

No branches or pull requests

2 participants