Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tracking issue for `illegal_floating_point_constant_pattern` compatibility lint #36890

Closed
petrochenkov opened this Issue Oct 1, 2016 · 9 comments

Comments

Projects
None yet
7 participants
@petrochenkov
Copy link
Contributor

petrochenkov commented Oct 1, 2016

This is the summary issue for the illegal_floating_point_constant_pattern
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 the warning for?

Rust 1.0 accepted a number of patterns -- and in particular accepted constant in patterns --
in a broader range of situations that was originally intended. RFC 1445 spelled out
some restrictions on what kinds of constants can be used in patterns, at least until the
final semantics are affirmatively decided. Among the changes was the ruling that floating
point values should not be usable in patterns.

This means that a match like this:

match foo() {
    3.0 => ...,
    _ => ...
}

should be written to:

match foo() {
    x if x == 3.0 => ...,
    _ => ...
}

Among other things, this clarifies that the semantics are the same as == normally uses. So e.g. 0 and -0 compare equal, and NaN compares unequal to everything (including itself).

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

  • #32199 introduces the illegal_floating_point_constant_pattern lint as warn-by-default
  • #36894 makes the illegal_floating_point_constant_pattern lint deny-by-default
  • #42136 makes the illegal_floating_point_constant_pattern lint a hard error # # #
@petrochenkov

This comment has been minimized.

Copy link
Contributor Author

petrochenkov commented Oct 1, 2016

@nikomatsakis, could you write a description for this?

@steveklabnik steveklabnik added the A-lint label Oct 7, 2016

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Oct 11, 2016

@carols10cents

This comment has been minimized.

Copy link
Member

carols10cents commented Apr 7, 2017

I'm confused... I'm working on documenting patterns in the book. This issue's description seems to indicate that this code:

match foo() {
    3.0 => ...,
    _ => ...
}

should currently cause an error (since the lints have now become hard errors).

However, this code using floating point literals currently works just fine without any errors or warnings and with the expected behavior. It's only if I put the floating point value in a constant that I get the error mentioned in this tracking issue.

So what was the intent here? Only to disallow matching on floating point constants, in which case rustc is correct and this issue's description is incorrect, or to disallow matching on floating point literals, in which case this issue's description is correct and rustc's current implementation is incorrect...?

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Apr 11, 2017

@carols10cents argh, that...seems like a rather glaring oversight! I think the intention was not to accept those examples!

@carols10cents

This comment has been minimized.

Copy link
Member

carols10cents commented Apr 11, 2017

Yay, I found a bug!!! Uhhh... i mean... boo, a bug! ;)

Should I file a new issue or is this issue sufficient...?

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Apr 12, 2017

@carols10cents sigh. I suppose a new issue would be helpful.

@carols10cents

This comment has been minimized.

Copy link
Member

carols10cents commented Apr 12, 2017

Done! #41255

@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented May 10, 2017

RFC 1445 spelled out some restrictions on what kinds of constants can be used in patterns, at least until the final semantics are affirmatively decided.

All the language changes that I can find mandated in https://github.com/rust-lang/rfcs/blob/master/text/1445-restrict-constants-in-patterns.md are about using in patterns const constants of struct or enum types.

Among the changes was the ruling that floating point values should not be usable in patterns.

This breaking change is not part of an accepted RFC, as far as I can tell. The only part of RFC 1445 that mentions struct literals is one example in the motivation section. Am I missing something?

#32199 introduces the illegal_floating_point_constant_pattern lint as warn-by-default
#36894 makes the illegal_floating_point_constant_pattern lint deny-by-default

Is that accurate? These PRs merged in 2016 but Servo’s code that using float literals in pattern started breaking today, presumably with #41293.

@nikomatsakis nikomatsakis added the T-lang label May 25, 2017

bors added a commit that referenced this issue Jun 1, 2017

Auto merge of #42136 - petrochenkov:oldhard, r=nikomatsakis
Turn sufficiently old compatibility lints into hard errors

It's been almost 7 months since #36894 was merged, so it's time to take the next step.

[breaking-change], needs crater run.

PRs/issues submitted to affected crates:
gnzlbg/ctest#17
Sean1708/rusty-cheddar#55
m-r-r/helianto#3
azdle/virgil#1
rust-locale/rust-locale#24
mneumann/acyclic-network-rs#1
reem/rust-typemap#38

cc https://internals.rust-lang.org/t/moving-forward-on-forward-compatibility-lints/4204
cc #34537 #36887
Closes #36886
Closes #36888
Closes #36890
Closes #36891
Closes #36892
r? @nikomatsakis

frewsxcv added a commit to frewsxcv/rust that referenced this issue Jun 1, 2017

@bors bors closed this in #42136 Jun 1, 2017

@leonardo-m

This comment has been minimized.

Copy link

leonardo-m commented Jun 19, 2017

See a problem I'm having: #42663

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.