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

Make the behavior of promote_noncircular the default? #22801

Closed
timholy opened this issue Jul 13, 2017 · 4 comments
Closed

Make the behavior of promote_noncircular the default? #22801

timholy opened this issue Jul 13, 2017 · 4 comments
Assignees
Labels
needs decision A decision on this change is needed
Milestone

Comments

@timholy
Copy link
Sponsor Member

timholy commented Jul 13, 2017

Before #19918 I used to see a lot more StackOverflowErrors in my own code. Should we consider adopting its behavior for the general promotion mechanism?

To be a little more concrete in my description than I was in #19918, note that we have this:

julia> ap, bp = promote('a', 2)
('a', 2)

i.e., promote does not guarantee that the types will change. So if you employ this common pattern:

foo{T}(x::T, y::T) = # "real" definition
foo(x, y) = foo(promote(x, y)...)  # the fallback definition

then foo('a', 2) will result in a StackOverflowError. Those errors sometimes take a long time to resolve (in complicated cases, I've seen tens of minutes) and the backtraces are not very helpful; it's particularly awful if you're ssh-ing over a slow link and you just watch the text scroll by...

One danger: what if there are genuine applications where you promote, and then test the two types to see if they are the same; if not, do something different. I can't remember seeing code like that, but it could exist in principle.

So alternatively perhaps we should make promote_noncircular an official exported interface, and cross-link the docstrings.

And finally, we could keep the status quo, or eliminate promote_noncircular entirely.

@timholy timholy added the needs decision A decision on this change is needed label Jul 13, 2017
@timholy timholy added this to the 1.0 milestone Jul 13, 2017
@JeffBezanson JeffBezanson self-assigned this Aug 10, 2017
@JeffBezanson
Copy link
Sponsor Member

What this comes down to: should promote(x,y) give an error by default if x and y come out unchanged?

@StefanKarpinski
Copy link
Sponsor Member

promote vs trypromote – the former could error on no promotion vs the latter just leaving alone?

@JeffBezanson
Copy link
Sponsor Member

Do we have any examples of cases where we want the trypromote behavior? I can't find any in Base.

@JeffBezanson
Copy link
Sponsor Member

One case that seems useful is "make sure these have the same type", in which case it's ok if the arguments already have the same types. So maybe that should be allowed by promote as a special case.

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

3 participants