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

stackoverflow with Vec2(1,2) #5201

Closed
dinurp opened this issue Dec 20, 2013 · 8 comments
Closed

stackoverflow with Vec2(1,2) #5201

dinurp opened this issue Dec 20, 2013 · 8 comments

Comments

@dinurp
Copy link

dinurp commented Dec 20, 2013

With today's build I am facing the following issue. Because of this Winston has become unusable.

               _
   _       _ _(_)_     |  A fresh approach to technical computing
  (_)     | (_) (_)    |  Documentation: http://docs.julialang.org
   _ _   _| |_  __ _   |  Type "help()" to list help topics
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 0.3.0-prerelease+705 (2013-12-20 09:38 UTC)
 _/ |\__'_|_|_|\__'_|  |  master/b687202 (fork: 105 commits, 0 days)
|__/                   |  i686-w64-mingw32

julia> using Base.Graphics

julia> Vec2(1,2)
ERROR: stack overflow

# Vec2 is defined as below in base/graphics.jl # 53
julia> immutable Vec_
       x::Float64
       y::Float64
       end

julia> Vec_(1,2)
Vec_(1.0,2.0)

# The following overload seems to be problematic
julia> Vec_(x::Real, y::Real) = Vec_(float64(x), float64(y))
Vec_ (constructor with 2 methods)

julia> Vec_(1,2)
ERROR: stack overflow
@ivarne
Copy link
Member

ivarne commented Dec 20, 2013

I can confirm the problem on Ubuntu. I remembered something like this changed recently, and I tested and found out that it was introduced by d7d671d for issue #4026
cc: @JeffBezanson.

@ivarne
Copy link
Member

ivarne commented Dec 20, 2013

A separate issue is your splash screen master/b687202 (fork: 105 commits, 0 days). Do you really have 105 commits that diverge from the JuliaLang/jula master branch, or is my code to detect that sort of thing wrong on your system.

I suspect that the problem is that I compare to the origin/master branch, and you cloned your fork from from github as origin and then added the JuliaLang/julia repository as a remote with another name. I have tried to solve that case also, but the shell script for finding the correct remote ended up more complex than I wanted it to be, and I have not tested and submitted a Pull Request yet.

@pao
Copy link
Member

pao commented Dec 20, 2013

@ivarne also, if at any point the history differed due to an extra commit--even if that commit was accepted later in a pull request, but in a different place than JuliaLang/julia/master--pulls will create merges. I'm not sure there's anything we can do about that other than continue to recommend not developing Julia itself on the master branch.

@ivarne
Copy link
Member

ivarne commented Dec 20, 2013

@pao If you commit something and then merge inn the master branch (without doing a rebase), you get the correct distance of 1 commit. Is your point that if that commit is submitted in a PR, and accepted, you will still have a 1 commit distance when you merge (not rebase) on top of origin/master?

@pao
Copy link
Member

pao commented Dec 20, 2013

@ivarne Yeah, I suppose I don't know how the distance calculation works now that you mention it. I was thinking of the case where your history is different such that every time you pull, you get a new merge commit (as I've seen happen in quite a few pull requests.) But this is pretty off-topic; we should probably take this off of the issue if there's further discussion.

@ivarne
Copy link
Member

ivarne commented Dec 20, 2013

There is a thread in julia-users about this regression also.

@pao I do not think there is anything we can do about people that update their pull requests with merge commits, apart from trying to tell them how to fix it in the most polite way possible.

@JeffBezanson
Copy link
Member

Sorry about that.

This is kind of interesting. Switching to Any for default constructors makes them easier to override, but it becomes harder to know if definitions are non-circular. With the old behavior, you could be sure to call the default constructor by passing the exact right types. That trade-off didn't quite occur to me. The Any behavior is still probably better though.

@ivarne
Copy link
Member

ivarne commented Dec 20, 2013

I would say that Any is definitely better, but then we need some way to ensure that a call with correct types will go to the default constructor.
There is still the issue of Package compatibility https://groups.google.com/forum/#!topic/julia-users/8fYniXykJ9M

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

No branches or pull requests

4 participants