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

fix #32488, type intersection regression #32509

Merged
merged 1 commit into from
Jul 6, 2019
Merged

fix #32488, type intersection regression #32509

merged 1 commit into from
Jul 6, 2019

Conversation

JeffBezanson
Copy link
Member

Issue was caused by 74305bf, so this reverts part of the new logic from there.

There is a loss of precision in one test case, which I easily consider acceptable.

Issue was caused by 74305bf, so this
reverts part of the new logic from there.
@JeffBezanson JeffBezanson added types and dispatch Types, subtyping and method dispatch bugfix This change fixes an existing bug labels Jul 5, 2019
@JeffBezanson JeffBezanson added this to the 1.2 milestone Jul 5, 2019
@JeffBezanson JeffBezanson merged commit a9ad6c2 into master Jul 6, 2019
@JeffBezanson JeffBezanson deleted the jb/fix32488 branch July 6, 2019 15:55
JeffBezanson added a commit that referenced this pull request Jul 17, 2019
Issue was caused by 74305bf, so this
reverts part of the new logic from there.
(cherry picked from commit a9ad6c2)
@KristofferC KristofferC mentioned this pull request Jul 17, 2019
14 tasks
@tkluck
Copy link
Contributor

tkluck commented Jul 21, 2019

This seems to cause a new method ambiguity for me (copied below). I used git bisect to point me to this PR.

The ambiguity looks like a bug, because one of the alternatives doesn't actually apply to the given types. Moreover, using methods shows only a single applicable method.

I haven't succeeded in isolating a MWE yet. The below comes from using the master version of https://github.com/tkluck/PolynomialRings.jl :

julia> using PolynomialRings

julia> R = @ring Int[x]
Int64[x]

julia> convert(R, :x)
ERROR: MethodError: convert(::Type{Int64[x]}, ::Symbol) is ambiguous. Candidates:
  convert(::Type{P}, a::C) where {C, P<:(<undisplayable> where MonomVector where #s23<:PolynomialRings.Monomials.AbstractMonomial{Order} where Order)} in PolynomialRings.Conversions at /home/tkluck/.julia/dev/PolynomialRings/src/PolynomialRings/Conversions.jl:51
  convert(::Type{P}, x::Symbol) where P<:Polynomial in PolynomialRings.EntryPoints at /home/tkluck/.julia/dev/PolynomialRings/src/EntryPoints.jl:25
Possible fix, define
  convert(::Type{P<:(<undisplayable> where C)}, ::C<:Symbol)
Stacktrace:
 [1] top-level scope at REPL[3]:1

julia> methods(convert, Tuple{Type{R}, Symbol})
# 1 method for generic function "convert":
[1] convert(::Type{P}, x::Symbol) where P<:Polynomial in PolynomialRings.EntryPoints at /home/tkluck/.julia/dev/PolynomialRings/src/EntryPoints.jl:25

(to avoid confusion: it says <undisplayable> because I try to pretty-print some types in some cases.)

Does this ring a bell?

@tkluck
Copy link
Contributor

tkluck commented Aug 11, 2019

^^ The issue I'm reporting in my previous comment seems to have been fixed as of a18ab97. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This change fixes an existing bug types and dispatch Types, subtyping and method dispatch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants