Matching on floating-point literal values is totally allowed and shouldn't be

[carols10cents]

Per this comment on the tracking issue for disallowing the use of constants set to floating point numbers (which errors as expected today), @nikomatsakis says the intention was to disallow floating point literal values in matches, but uh... this is totally accepted by Rust 1.16.0:

fn main() {
    let x = 13.4;

    match x {
        1.0 => println!("one"),
        22.4 => println!("two"),
        3.67 => println!("three"),
        13.4 => println!("thirteen point four"),
        _ => println!("anything"),
    }
}

This prints "thirteen point four". I expected to get a compiler error like the one I get if I change a floating point value in the match to a constant:

const FOO: f64 = 3.67;

fn main() {
    let x = 13.4;

    match x {
        1.0 => println!("one"),
        22.4 => println!("two"),
        FOO => println!("three"),
        13.4 => println!("thirteen point four"),
        _ => println!("anything"),
    }
}

results in:

error: floating point constants cannot be used in patterns, #[deny(illegal_floating_point_constant_pattern)] on by default
 --> src/main.rs:9:9
  |
9 |         FOO => println!("three"),
  |         ^^^
  |
  = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
  = note: for more information, see issue #36890 <https://github.com/rust-lang/rust/issues/36890>

[est31]

There seem to be three options:

• we add literals to the existing compatibility lint without changing the status, this would immediately deny the code above by default
• we add literals to the existing compatibility lint and change it to warn by default, and sometime in the future set it to deny by default again
• we create a new lint and set it to warn by default, making two consecutive breaks

idk maybe a crater run should be done to find out what is best.

[nikomatsakis]

@est31 indeed, a crater run seems warranted.

[nikomatsakis]

Nominating for compiler team discussion on best way to proceed and prioritization. Seems to me we ought to settle this pretty quickly one way or another.

[nikomatsakis]

triage: P-high

This is time-sensitive, so we gotta act if we are ever going to. Assigning to @pnkfelix.

[est31]

I've filed two PRs #41292 for removing the use of a floating point match from the compiler and #41293 for a crater run to test the most strict solution -- to add it to the existing lint without changing the deny-by-default status. Independent from whether that strategy will be chosen in the end, it gives us an initial number of crates that would be affected by the change.

#41293 includes #41292 already to make a crater run possible without #41292 being merged.

[petrochenkov]

This is arguable.

Floats are built-in types known to the compiler, you don't have to run any user-defined code to match on them.
Matching being "bitwise" is also not a goal - references, for example, are matched semantically and not as pointer values, and everyone is happy about it. Floats can be compared semantically as well.

Another analogy is that float literals are the same thing as variants for non-structural_match enums, as opposed to constants. So even if constants are prohibited, variants still can be used:

match xxx {
    NON_STRUCTURAL_MATCH_CONST => {} // Not OK, constant
    NonStructuralMatch { field } => {} // OK, variant
    FLOAT_CONST => {} // Not OK, constant
    1.0 => {} // OK, variant
}

[petrochenkov]

The number of regressions in #41293 is also relatively large.

[nikomatsakis]

@petrochenkov

As I wrote on the comment, I too am second-guessing this rule. I think at this point I'm inclined to say that matching on floats uses == rules (e.g., so that this program continues to execute as today). This seems to imply that there is a disconnect between the semantics of matching (i.e., what we consider equal there) from the semantics of constant evaluation (since presumably we would not want a floating point expression that produces 0.0 and one that produces -0.0 to "unify"?). I am ok with that; we've sort of been conflating them but I'm not sure that they are the same problem.

[aidanhs]

Matching being "bitwise" is also not a goal - references, for example, are matched semantically and not as pointer values, and everyone is happy about it. Floats can be compared semantically as well.

While this is true, most (all?) of the time there is some visual hint that the compiler may insert extra logic. E.g. if I see a & or box I'll be aware of pointer dereferencing and guards and ranges obviously do extra work. But I guess checking two bit patterns (for 0) isn't too bad.

[nikomatsakis]

@aidanhs we'd also have to settle what NaN ought to do (would it ever be a match?). My inclination would be to make floating point matches align with what == does in every respect (it seems very surprising to make the precise bit-pattern significant here).

[aidanhs]

If we had to choose, personally I'd prefer NaN to be matched! My current intuition is that a pattern match is quite distinct from ==, most obviously shown by being able to match on an enum you've defined but not being able to do == without adding the derive. In plain english, my heuristic is "does this pattern visually match the written down form of the value", so Some(NaN) would match Some(NaN).

Of course, there are some annoyances since

1. there are a variety of NaNs (i.e. precise bit pattern would need to not be significant)
2. justifying the difference between if Some(NaN) == Some(NaN) { and if let Some(f64::NAN) = Some(f64::NAN) { may be a tough sell

But...is there actually any need to make a decision for NaN? There's no NaN literal, so there's no way to write it in a pattern in a way that rustc accepts (I think? It's not even rejected under the lint, it's rejected as an irrefutable pattern). And if you can't write it on the left, it doesn't matter if it's ever on the right. Continuing a compile-time error that already exists is definitely my favourite approach for NaN.

[est31]

It's not even rejected under the lint, it's rejected as an irrefutable pattern

If you allow the lint, this compiles: https://is.gd/U8BM1Y

[nikomatsakis]

But...is there actually any need to make a decision for NaN? There's no NaN literal, so there's no way to write it in a pattern in a way that rustc accepts (I think? It's not even rejected under the lint, it's rejected as an irrefutable pattern).

I'd prefer to accept both named constants and constant patterns in a consistent way, myself. I also agree that NaN should match NaN (in general, I think that if you can do both a == C and match a { C => .. } for some constant C, they should do the same thing).

UPDATE: (But I know that nullary enum variants do not necessarily behave this way -- I rationalize this by the idea that matching a variant is distinct from matching a constant, but elsewhere we don't draw that distinction.)

[nikomatsakis]

So @aidanhs tactfully pointed out to me on IRC that I wrote two contradictory things:

I also agree that NaN should match NaN

and

in general, I think that if you can do both a == C and match a { C => .. } for some constant C, they should do the same thing

To be honest, I'm not sure which of these values I hold most dearly anymore! I do have the notion that pattern matching (for enums, as you say) is doing "variant testing", which is a distinct operation from ==. One could hence say the same for floating points. Certainly it seems like being able to match on NaN would be useful, but I guess then that if "match is the non-broken eq", I would expect 0 != -0 as well. Perhaps nobody wants that in practice.

And the bigger question here is when (if ever) we would allow matching constants of generic type (which will start to arise when we get to functions that can be generic over constants). I'd like to allow for that future, which seems to imply that one of three things is true:

• We limit constants appearing in match to known types
  • Viable, but sad. I anticipate people will (eventually) want to write a function that is generic over any integer type T, for example, and have generic constants of type T.
• We have a universal notion of "structural equality" one can apply to all types, or at least all types that generic constants can take on.
  • This sounds similar to the notion of equality that we use for "unification", but I believe they are distinct concepts. That is, the unification equality is probably based on uninterpreted functions, to a certain extent -- i.e., we know that two things are equal if they come from identical source expressions. But matching quality is presumably based on the value ultimately produced by those functions, as a dynamic value.
  • We haven't even begun to tackle making extending this notion to all types in any case.
• We have a new trait StructuralEq (modulo naming) that means "a type that can be compared for equality in a match". The relationship to Eq is unclear (perhaps it is a "marker type" that just indicates that eq behaves in a compatible way with what a match would do?)
  • Viable, but the eq traits are already complex.
• We re-use the existing Eq trait, but just as a marker, with distinct equality semantics.
  • Yuck.
• We re-use the existing Eq trait and define matching a constant to be equivalent to ==.
  • Viable, but matching runs user-defined code and so forth.

So, unless I'm missing something, we basically have to choose between:

• No constants of generic type in match
• Yet Another Equality Trait
• Use Eq semantics in match

[nikomatsakis]

@eddyb @withoutboats -- I'd appreciate your thoughts on this comment, and in particular about the implications for unification. For context, we're talking about the meaning of floating point constants in match, but I'm bringing in more general questions of what it means to have a constant as a pattern anyhow.

In any case, I suppose "no constants of generic type in match" is not the end of the world. After all, one can write the relevant code using pattern guards.

[eddyb]

We could make that trait about pattern-matching instead of "equality" (i.e. if we ever add custom patterns, it could be through that trait), and while it might be customizable, it couldn't escape our restrictions on pattern-matching.

E.g. maybe terrible, but:

trait Inject<T...; const IN: T> {
    const VALUE: Self;
}

struct Point { x: i32, y: i32 }
impl<const x: i32, const y: i32> Inject<i32, i32; {(x, y)}> for Point {
    const VALUE = Point { x, y };
}

This looks worse than refinement typing, but it probably has some usecases.

[nikomatsakis]

@eddyb

We could make that trait about pattern-matching instead of "equality"

I suppose that is another way of looking at it -- that is, instead of "another kind of equality", what we have instead is a trait that defines what pattern matching a value means. This could be more than marker trait -- that is, it could open the door to overloaded pattern matching, which would be potentially quite cool. (This all works better, of course, if pattern variants have their own types.)

I could get behind that story, though we'd have to have some sort of "fallback" behavior -- or default impl -- since we currently permit pattern matching for things that derive Eq.

[eddyb]

I expect the automatic implementation to hook into #[derive(PartialEq, Eq)].

[withoutboats]

I think I'd prefer to just disallow float literals in patterns.

[nikomatsakis]

Nominating for lang team discussion today.

[nikomatsakis]

We discussed this in the @rust-lang/lang team meeting. There was some disagreement about the best path forward, but we agreed that as a compromise we could start issuing warnings (not deny by default) for floating point literals, and see where that takes us. That was the RFC that we merged, after all.

[nikomatsakis]

Sorry, I didn't give many details from our discussion (which actually took quite some time). I don't think we really covered any new ground that's not already in this discussion thread, though.

[SimonSapin]

Was a crater run made, in the end?

This breaks Servo, which unfortunately is not part of Crater.

[nikomatsakis]

@SimonSapin

Was a crater run made, in the end?

Yes.

This breaks Servo, which unfortunately is not part of Crater.

Do you mean "this would break servo if it became a hard error"? (But we should take it to #41620 in any case.)

[SimonSapin]

Do you mean "this would break servo if it became a hard error"?

Yes, I think this particular crate has #![deny(warnings)].

[aturon]

Refiling as T-compiler; what's left here is to move to a hard error. The warning went out in 1.18.

[nikomatsakis]

triage: P-medium

[oli-obk]

Heads up: #46882 removes the hard error for matching on float constants and produces the same future compat warning.

[Mark-Simulacrum]

I'm going to close this in favor of the tracking issue for the lint, since it seems like that's where discussion should move.