Tracking issue for indirect_structural_match compatibility lint

[pnkfelix]

This is the summary issue for the indirect_structural_match future-compatibility warning and other related errors. The goal of this page is describe why this change was made and how you can fix
code that is affected by it. It also provides a place to ask questions or register a complaint if you feel the change should not be made. For more information on the policy around future-compatibility warnings, see our breaking change policy guidelines.

What is this lint about

Uses of a const item in a pattern are currently supposed to not rely on whether the semantics of matching a pattern are based on "structural equality" (i.e. unfolding the value of the const item into the pattern) or "semantic equality" (calling PartialEq::eq). See RFC 1445 for more discussion on this point.

For example, we currently reject the following code, because it could be used to detect which of the two semantics are used for matching patterns (play):

// I am equal to anyone who shares my sum!
struct Plus(i32, i32);

impl PartialEq for Plus {
    fn eq(&self, other: &Self) -> bool {
        self.0 + self.1 == other.0 + other.1
    }
}

impl Eq for Plus { }

const ONE_PLUS_TWO: Plus = Plus(1, 2);

fn main() {
    if let ONE_PLUS_TWO = Plus(3, 0) {
        println!("semantic!");
    } else {
        println!("structural!");
    }
}

However, the code to enforce adherence to RFC 1445 missed some cases. The compiler incorrectly accepted the following variant of the above code (play):

const ONE_PLUS_TWO: & &Plus = & &Plus(1, 2);

fn main() {
    if let ONE_PLUS_TWO = & &Plus(3, 0) {
        println!("semantic!");
    } else {
        println!("structural!");
    }
}

Since we have not yet decided how to resolve this problem for the long term, it is best to alert users that they are using a corner of the language that was not completely specified. (To be clear: The compiler will use either semantic or structural equality, and its choice will not introduce unsoundness; but it may yield very surprising behavior for the user, depending on how they have implemented PartialEq.)

How to fix this warning/error

Here are three options:

1. Change const item itself

Change the const item referenced in the pattern to use only types that derive PartialEq and Eq.

In our running example, that would correspond to changing the code to use derive instead of explicit impl:

// I am equal to anyone who shares my sum!
struct Plus(i32, i32);

impl PartialEq for Plus {
    fn eq(&self, other: &Self) -> bool {
        self.0 + self.1 == other.0 + other.1
    }
}

impl Eq for Plus { }

becomes:

// I am no longer equal to all who share my sum!
#[derive(PartialEq, Eq)]
struct Plus(i32, i32);

Of course, in this particular example, this is a non-semantics preserving change for the program at large, since presumably the original designer wanted the previous semantics for PartialEq. The main reason we point it out (and in fact, point it out first) is that in many cases, switching to #[derive(PartialEq, Eq)] will be both correct and the simplest fix.

2. Change pattern to call == explicitly

If semantic equality is desired, change the pattern to bind the input to a variable and call the == operator (this may require switching from if let to match until let_chains are stabilized).

In our running example (adapted slightly to use match):

    match & &Plus(3, 0) {
        ONE_PLUS_TWO => println!("semantic!"),
        _ => println!("structural!"),
    }

becomes:

    match & &Plus(3, 0) {
        sum if sum == ONE_PLUS_TWO => println!("semantic!"),
        _ => println!("structural!"),
    }

3. Change pattern to inline structural form

If structural equality is desired, inline the right-hand side of the const as an explicit pattern.

In our running example (adapted slightly to use match):

    match & &Plus(3, 0) {
        ONE_PLUS_TWO => println!("semantic!"),
        _ => println!("structural!"),
    }

becomes:

    match & &Plus(3, 0) {
        & &Plus(1, 2) => println!("semantic!"),
        _ => println!("structural!"),
    }

When will this warning become a hard error?

At the beginning of each 6-week release cycle, the Rust compiler team will review the set of outstanding future compatibility warnings and nominate some of them for Final Comment Period. Toward the end of the cycle, we will review any comments and make a final determination whether to convert the warning into a hard error or remove it entirely.

Current status

• PR use visitor for #[structural_match] check #62339 implements the lint as warn by default (in a slightly broken fashion)
• PR downgrade indirect_structural_match lint to allow #62623 changes the lint to be allow by default to work around a bug
• PR Fully destructure constants into patterns #70743 returns the lint to warn by default
• PR ? makes lint deny by default
• PR ? makes lint a hard error.

[pnkfelix]

spawned off of tracking issue #31434, compiler issue #62307, and compiler PR #62339

[seanmonstar]

I've noticed this warning suddenly being triggered on this code using the http crate:

let m = http::Method::GET;

match m {
    http::Method::GET => (),
    _ => ()
}

There is a derive(PartialEq, Eq) on the struct Method definition, so is that a false positive?

[pnkfelix]

@seanmonstar is everything that is recursively teachable from that structure also a #[structural_match] ?

(I have been wondering if there may be subtle cases where this warning arises that are instances of there not being “turtles all the way down”)

[seanmonstar]

I noticed the warning says Box isn't, and one of the variants is Box<[u8]>... That'd be the long-extension variant. I guess a structural match would almost never be true as the pointers would differ... Though, we don't have any constants of that variant and any parsing will always use the constant if it's a match.

So while I guess technically in correct, in this case a structural match would be fine since you can't make a constant of that variant anyway.

[pnkfelix]

Hmm yes. This may be an unfortunate artifact of how this implementation is trying to detect instances of ADT's missing structural match: It traverses the structure of the type itself, which means it visits every variant of an enum type, including variants that are unused by the const item in question.

I suspect we can fix this. I don't think I'm going to be able to do so before I go on leave, but I'll file a bug about it in any case.

[pnkfelix]

@seanmonstar filed as #62614

[pnkfelix]

... and filed PR #62623 as a way to address the problem in the short-term.

[pnkfelix]

I wonder if we can make indirect_structural_match warn-by-default again now that PR #67343 has landed and issue #62614 is closed.

[RalfJung]

What is the current status here? This has been a future-incompat lint for a long time now, can we move towards making it a hard error? Should we at least take a step towards that by having it show up in cargo's future-incompat reports?

I am not sure if our vision for structural-match restrictions is still the same as 4 years ago though. #74446 is still unresolved. My immediate goal would be to move pattern matching away from a "fallback" logic: currently it works by "trying a valtree" and then "falling back to an opaque const with PartialEq"; such fallbacks are typically fragile and hard to reason about. What would it take to decide between "construct transparent pattern via valtree" vs "opaque pattern" up-front (and ICEing if building a valtree fails after we decided that that is the strategy we want to use)? Is it something like self.tcx.mir_const_qualif(instance.def_id()).custom_eq, based on #67343? There are a whole bunch of lints around structural equality (indirect_structural_match, nontrivial_structural_match, pointer_structural_match, illegal_floating_point_literal_pattern) and I don't know which of these have to become hard errors before we can do anything like that. (If #110166 is right then pointer_structural_match might actually not be relevant for this?)

@oli-obk or anyone else who knows the current status here, would be great to get a summary. :)

[RalfJung]

In fact I think I'm uncertain about what the endgame here really is. Is it the fallback plan above? But which code would that even break? With fn ptrs now having PartialEq, I think we don't accept any constants any more that are not PartialEq?

Or is the point that in case we detect custom equality, we're not just falling back to PartialEq, we actually want to reject this outright? IIRC we concluded this would break too much code. So do we have a short list of types that are allowed anyway (raw ptr, fn ptr, float)? That wouldn't work, this gets accepted after all. It doesn't even show a future-incompat warning.

What are examples of code that we still plan to break? Maybe the breaking has nothing at all to do with valtrees or other internals.

[RalfJung]

I laid down some of my thoughts on where we might want to go with all this over here.

[RalfJung]

This is pretty much superseded by #120362.