Skip to content

typeck: audit the use of a_is_expected pattern #12934

Closed

Description

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions