Slight VersionSet refactoring, test intersect (#20598)

* Enforce VectorSet normalization in constructor (fixes equality
  testing)
* Improve VectorSet normalization code (faster, fewer allocations),
* Test VectorSet intersections (including the empty cases which were
  at the root of issue #20566)
* Change VectorSet printing to avoid unicode on Windows

Author: carlobaldassi

base/pkg/types.jl

@@ -20,8 +20,50 @@ intersect(a::VersionInterval, b::VersionInterval) = VersionInterval(max(a.lower,
 ==(a::VersionInterval, b::VersionInterval) = a.lower == b.lower && a.upper == b.upper
 hash(i::VersionInterval, h::UInt) = hash((i.lower, i.upper), h + (0x0f870a92db508386 % UInt))
 
+function normalize!(ivals::Vector{VersionInterval})
+    # VersionSet internal normalization:
+    # removes empty intervals and fuses intervals without gaps
+    # e.g.:
+    #     [0.0.0,1.0.0) ∪ [1.0.0,1.5.0) ∪ [1.6.0,1.6.0) ∪ [2.0.0,∞)
+    # becomes:
+    #     [0.0.0,1.5.0) ∪ [2.0.0,∞)
+    # (still assumes that lower bounds are sorted, and intervals do
+    # not overlap)
+    l = length(ivals)
+    l == 0 && return ivals
+
+    lo, up, k0 = ivals[1].lower, ivals[1].upper, 1
+    fusing = false
+    for k = 2:l
+        lo1, up1 = ivals[k].lower, ivals[k].upper
+        if lo1 == up
+            up = up1
+            fusing = true
+            continue
+        end
+        if lo < up
+            # The only purpose of the "fusing" check is to avoid
+            # extra allocations
+            ivals[k0] = fusing ? VersionInterval(lo, up) : ivals[k-1]
+            k0 += 1
+        end
+        fusing = false
+        lo, up = lo1, up1
+    end
+    if lo < up
+        ivals[k0] = fusing ? VersionInterval(lo, up) : ivals[l]
+        k0 += 1
+    end
+    resize!(ivals, k0 - 1)
+    return ivals
+end
+
 struct VersionSet
     intervals::Vector{VersionInterval}
+    VersionSet(intervals::Vector{VersionInterval}) = new(normalize!(intervals))
+    # copy is defined inside the struct block to call `new` directly
+    # without going through `normalize!`
+    Base.copy(vset::VersionSet) = new(copy(vset.intervals))
 end
 function VersionSet(versions::Vector{VersionNumber})
     intervals = VersionInterval[]
@@ -37,54 +79,25 @@ function VersionSet(versions::Vector{VersionNumber})
 end
 VersionSet(versions::VersionNumber...) = VersionSet(VersionNumber[versions...])
 
-const empty_versionset = VersionSet([v"0.0",v"0.0"])
+const empty_versionset = VersionSet(VersionInterval[])
 
-function normalize!(A::VersionSet)
-    # removes empty intervals and fuses intervals without gaps
-    # e.g.:
-    #     [0.0.0,1.0.0) ∪ [1.0.0,1.5.0) ∪ [1.6.0,1.6.0) ∪ [2.0.0,∞)
-    # becomes:
-    #     [0.0.0,1.5.0) ∪ [2.0.0,∞)
-    # (still assumes that intervals are properly sorted)
-    ivals = A.intervals
-    l = length(ivals)
-    for k = l:-1:1
-        isempty(ivals[k]) && (splice!(ivals, k); l -= 1)
-    end
-    l > 1 || return A
-    lo, up, k0 = ivals[l].lower, ivals[l].upper, l
-    fuse = false
-    for k = (l-1):-1:1
-        lo1, up1 = ivals[k].lower, ivals[k].upper
-        if up1 == lo
-            fuse = true
-            lo = lo1
-            continue
-        end
-        fuse && splice!(ivals, (k+1):k0, (VersionInterval(lo, up),))
-        fuse = false
-        lo, up = lo1, up1
-        k0 = k
-    end
-    fuse && splice!(ivals, 1:k0, (VersionInterval(lo, up),))
-    return A
-end
-
-show(io::IO, s::VersionSet) = join(io, s.intervals, " ∪ ")
+# Windows console doesn't like Unicode
+const _empty_symbol = @static is_windows() ? "empty" : "∅"
+const _union_symbol = @static is_windows() ? " or " : " ∪ "
+show(io::IO, s::VersionSet) = isempty(s) ? print(io, _empty_symbol) :
+                                           join(io, s.intervals, _union_symbol)
 isempty(s::VersionSet) = all(isempty, s.intervals)
 in(v::VersionNumber, s::VersionSet) = any(i->in(v,i), s.intervals)
 function intersect(A::VersionSet, B::VersionSet)
-    (isempty(A) || isempty(B)) && return normalize!(copy(empty_versionset))
+    (isempty(A) || isempty(B)) && return copy(empty_versionset)
     ivals = [intersect(a,b) for a in A.intervals for b in B.intervals]
-    filter!(i->!isempty(i), ivals)
     sort!(ivals, by=i->i.lower)
     VersionSet(ivals)
 end
-copy(A::VersionSet) = VersionSet(copy(A.intervals))
 
 union(A::VersionSet, B::VersionSet) = union!(copy(A), B)
 function union!(A::VersionSet, B::VersionSet)
-    A == B && return normalize!(A)
+    A == B && return A
     ivals = A.intervals
     for intB in B.intervals
         lB, uB = intB.lower, intB.upper
@@ -105,7 +118,8 @@ function union!(A::VersionSet, B::VersionSet)
             end
         end
     end
-    return normalize!(VersionSet(ivals))
+    normalize!(ivals)
+    return A
 end
 
 ==(A::VersionSet, B::VersionSet) = A.intervals == B.intervals
@@ -223,11 +237,7 @@ function _show(io::IO, ritem::ResolveBacktraceItem, indent::String, seen::Set{Re
             end
             print(io, "required by package $(w[1]), ")
             if isa(w[2].versionreq, VersionSet)
-                if !isempty(w[2].versionreq)
-                    println(io, "whose allowed version range is $(w[2].versionreq):")
-                else
-                    println(io, "whose allowed version range is empty:")
-                end
+                println(io, "whose allowed version range is $(w[2].versionreq):")
             else
                 println(io, "whose only allowed version is $(w[2].versionreq):")
             end

test/version.jl

@@ -255,16 +255,24 @@ function chkint(a::VersionSet)
     return true
 end
 
-# VersionSet unions
+const empty_versionset = VersionSet(VersionInterval[])
+@test isempty(empty_versionset)
+
+# VersionSet intersections and unions
+@test empty_versionset ∩ empty_versionset == empty_versionset
+@test empty_versionset ∪ empty_versionset == empty_versionset
 for t = 1:1_000
-    a = VersionSet(map(x->VersionNumber(x, rand(0:3)), sort!(unique(rand(0:8, rand(0:10)))))...)
-    b = VersionSet(map(x->VersionNumber(x, rand(0:3)), sort!(unique(rand(0:8, rand(0:10)))))...)
+    a = VersionSet(sort!(map(v->VersionNumber(v...), [(rand(0:8),rand(0:3)) for i = 1:rand(0:10)]))...)
+    b = VersionSet(sort!(map(v->VersionNumber(v...), [(rand(0:8),rand(0:3)) for i = 1:rand(0:10)]))...)
     @assert chkint(a)
     @assert chkint(b)
-    u = union(a, b)
+    u = a ∪ b
     @test chkint(u)
+    i = a ∩ b
+    @test chkint(i)
     for vM = 0:9, vm = 0:5
         v = VersionNumber(vM, vm)
         @test (v ∈ a || v ∈ b) ? (v ∈ u) : (v ∉ u)
+        @test (v ∈ a && v ∈ b) ? (v ∈ i) : (v ∉ i)
     end
 end