Skip to content

Commit

Permalink
Revert "ml-matches: update and improve ambiguity computation (#36962)" (
Browse files Browse the repository at this point in the history
#37484)

This reverts commit 21ff6ee.
  • Loading branch information
KristofferC authored Sep 9, 2020
1 parent 01b29ec commit 519b04e
Show file tree
Hide file tree
Showing 8 changed files with 211 additions and 503 deletions.
89 changes: 16 additions & 73 deletions base/reflection.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1363,8 +1363,6 @@ Determine whether two methods `m1` and `m2` may be ambiguous for some call
signature. This test is performed in the context of other methods of the same
function; in isolation, `m1` and `m2` might be ambiguous, but if a third method
resolving the ambiguity has been defined, this returns `false`.
Alternatively, in isolation `m1` and `m2` might be ordered, but if a third
method cannot be sorted with them, they may cause an ambiguity together.
For parametric types, the `ambiguous_bottom` keyword argument controls whether
`Union{}` counts as an ambiguous intersection of type parameters – when `true`,
Expand All @@ -1391,82 +1389,27 @@ false
```
"""
function isambiguous(m1::Method, m2::Method; ambiguous_bottom::Bool=false)
m1 === m2 && return false
# TODO: eagerly returning `morespecific` is wrong, and fails to consider
# the possibility of an ambiguity caused by a third method:
# see the precise algorithm in ml_matches for a more correct computation
if m1 === m2 || morespecific(m1.sig, m2.sig) || morespecific(m2.sig, m1.sig)
return false
end
ti = typeintersect(m1.sig, m2.sig)
(ti <: m1.sig && ti <: m2.sig) || return false # XXX: completely wrong, obviously
ti === Bottom && return false
function inner(ti)
ti === Bottom && return false
if !ambiguous_bottom
has_bottom_parameter(ti) && return false
end
min = UInt[typemin(UInt)]
max = UInt[typemax(UInt)]
has_ambig = Int32[0]
ms = _methods_by_ftype(ti, -1, typemax(UInt), true, min, max, has_ambig)
has_ambig[] == 0 && return false
if !ambiguous_bottom
filter!(ms) do m
return !has_bottom_parameter(m.spec_types)
end
end
# if ml-matches reported the existence of an ambiguity over their
# intersection, see if both m1 and m2 may be involved in it
have_m1 = have_m2 = false
for match in ms
m = match.method
m === m1 && (have_m1 = true)
m === m2 && (have_m2 = true)
end
if !have_m1 || !have_m2
# ml-matches did not need both methods to expose the reported ambiguity
return false
end
if !ambiguous_bottom
# since we're intentionally ignoring certain ambiguities (via the
# filter call above), see if we can now declare the intersection fully
# covered even though it is partially ambiguous over Union{} as a type
# parameter somewhere
minmax = nothing
for match in ms
m = match.method
match.fully_covers || continue
if minmax === nothing || morespecific(m.sig, minmax.sig)
minmax = m
end
end
if minmax === nothing
return true
end
for match in ms
m = match.method
m === minmax && continue
if match.fully_covers
if !morespecific(minmax.sig, m.sig)
return true
end
else
if morespecific(m.sig, minmax.sig)
return true
end
end
end
return false
end
return true
if !ambiguous_bottom
has_bottom_parameter(ti) && return false
end
if !(ti <: m1.sig && ti <: m2.sig)
# When type-intersection fails, it's often also not commutative. Thus
# checking the reverse may allow detecting ambiguity solutions
# correctly in more cases (and faster).
ti2 = typeintersect(m2.sig, m1.sig)
if ti2 <: m1.sig && ti2 <: m2.sig
ti = ti2
elseif ti != ti2
inner(ti2) || return false
matches = _methods_by_ftype(ti, -1, typemax(UInt))
for match in matches
m = match.method
m === m1 && continue
m === m2 && continue
if ti <: m.sig && morespecific(m.sig, m1.sig) && morespecific(m.sig, m2.sig)
return false
end
end
inner(ti) || return false
# otherwise type-intersection reported an ambiguity we couldn't solve
return true
end

Expand Down
6 changes: 3 additions & 3 deletions src/clangsa/GCChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1340,16 +1340,16 @@ bool GCChecker::evalCall(const CallExpr *CE,
return true;
} else if (name == "JL_GC_PUSH1" || name == "JL_GC_PUSH2" ||
name == "JL_GC_PUSH3" || name == "JL_GC_PUSH4" ||
name == "JL_GC_PUSH5" || name == "JL_GC_PUSH6" ||
name == "JL_GC_PUSH7") {
name == "JL_GC_PUSH5" || name == "JL_GC_PUSH6") {
ProgramStateRef State = C.getState();
// Transform slots to roots, transform values to rooted
unsigned NumArgs = CE->getNumArgs();
for (unsigned i = 0; i < NumArgs; ++i) {
SVal V = C.getSVal(CE->getArg(i));
auto MRV = V.getAs<loc::MemRegionVal>();
if (!MRV) {
report_error(C, "JL_GC_PUSH with something other than a local variable");
report_error(C,
"JL_GC_PUSH with something other than a local variable");
return true;
}
const MemRegion *Region = MRV->getRegion();
Expand Down
Loading

0 comments on commit 519b04e

Please sign in to comment.