Description
openedon Mar 16, 2014
In typeck passes, one often needs to do binary operations between two thingy: equality, substitution and what have you. A pattern has been adopted to handle that:
fn some_op(a_is_expected: bool, a: SomeType, b: SomeType) -> SomeResult
Unfortunately there are two interpretations of the signature, one is that the order of a
and b
is significant, the other insignificant. And the caller and callee seem to diverge.
For example, in librustc/middle/typeck/infer/mod.rs
, there's honest one that says "I don't care about the value of a_is_expected":
pub fn mk_subr(cx: &InferCtxt,
_a_is_expected: bool,
origin: SubregionOrigin,
a: ty::Region,
b: ty::Region) { ... }
There's equality test that the order genuinely doesn't matter:
pub fn mk_eqty(cx: &InferCtxt,
a_is_expected: bool,
origin: TypeOrigin,
a: ty::t,
b: ty::t)
-> ures { ... }
And then there's ambiguous one:
pub fn mk_sub_trait_refs(cx: &InferCtxt,
a_is_expected: bool,
origin: TypeOrigin,
a: @ty::TraitRef,
b: @ty::TraitRef) -> ures
{
...
let suber = cx.sub(a_is_expected, trace);
suber.trait_refs(a, b)
...
}
It seems to use parameter a_is_expected
, but only for error messages. The real deal trait_refs
actually dictates a fixed order of a
and b
, which makes calling it like mk_sub_trait_refs(..., false, a, b)
semantically wrong (it should be mk_sub_trait_refs(..., true, b, a)
). Or rather, what caller expects is:
{
if a_is_expected { suber.trait_refs(a, b) }
else { suber.trait_refs(b, a) }
}
I believe that's what leads to bugs like #12223.
So we should check and fix it, either by removing the a_is_expected
all together or by rectifying the order of a
and b
when it is used wrong.