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

Infer a missing method as returning Union{} #20866

Closed
wants to merge 1 commit into from

Conversation

martinholters
Copy link
Member

@martinholters
Copy link
Member Author

Derp. I really should get into the habit of enabling assertions.

@martinholters
Copy link
Member Author

Huh, looks like [Union{}] triggers the assertion, while Any[Union{}] works.

@martinholters
Copy link
Member Author

Looks like I found a reason for better inferring Any myself: #20869.
In the long run, I still think this PR is the way to go, but we need to fix #20869 first.

@StefanKarpinski
Copy link
Sponsor Member

@vtjnash: Jeff wants your opinion on this, but is "generally in favor".

@vtjnash
Copy link
Sponsor Member

vtjnash commented Mar 9, 2017

As the author of this PR noted above, we can't do this until type-intersection is usually correct.

@martinholters
Copy link
Member Author

OTOH, as long as type-intersection may give wrongly empty answers, things can easily break anyway; this PR only makes it even a little easier. Consider

function foo(...)
# ...
end

function bar(...)
    # ...
    foo(...)
end

Now if the inferred types for the arguments to foo in bar and the signature of foo have a wrongly empty intersection computed (i.e. the actual argument types determined at runtime match the signature), then on master, things are fine, while with this PR, calling bar will likely crash as foo unexpectedly returns a result instead of throwing. But now add

foo(...) = error("arguments must ... whatever")

with a signature more general than the first foo method to add a more descriptive error message instead of a MethodError. Now if only this second method has a non-empty intersection, then master will be equally prone to a crash as this PR.

I guess what I'm trying to say is: We really need to make type-intersection reliable; this PR only increases the pressure to do so.

@StefanKarpinski
Copy link
Sponsor Member

In a way, making the process more likely to crash in cases where inference is wrong is helpful for finding cases where it is wrong.

@martinholters
Copy link
Member Author

Superseded by #21892.

@martinholters martinholters deleted the mh/infer_missing_method branch May 19, 2017 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:inference Type inference
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants