Skip to content

/rust

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

Restrict use of constants in patterns (RFC 1445) #31434

Open
nikomatsakis opened this Issue Feb 5, 2016 · 7 comments

Comments

Assignees

@nikomatsakis nikomatsakis

Labels
B-RFC-approved C-tracking-issue T-lang
Projects
None yet
Milestone
No milestone
5 participants
@nikomatsakis @durka @brson @Centril @Mark-Simulacrum
@nikomatsakis
Copy link
Contributor

nikomatsakis commented Feb 5, 2016 •
edited

Tracking issue for rust-lang/rfcs#1445.

Implementation steps:

  • disable pattern expansion for user-defined types unless tagged with #[structural_match]
  • disable pattern matching for floating point constants
  • have #[derive(Eq)] add #[structural_match] attribute
  • exhaustiveness, dead-code integration

@nikomatsakis nikomatsakis added B-RFC-approved T-lang labels Feb 5, 2016

@nikomatsakis nikomatsakis self-assigned this Feb 5, 2016

@durka

This comment has been minimized.

Sign in to view
Copy link
Contributor

durka commented Feb 5, 2016

Why #[derive(Eq)] and not #[derive(PartialEq)]? I see this was brought up in the RFC thread but I didn't really see any reasons given.

@nikomatsakis nikomatsakis referenced this issue Mar 11, 2016

Merged

Restrict constants in patterns #32199

@nikomatsakis

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

nikomatsakis commented Mar 11, 2016

See the PR #32199
@nikomatsakis

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

nikomatsakis commented Mar 11, 2016

@durka

Why #[derive(Eq)] and not #[derive(PartialEq)]? I see this was brought up in the RFC thread but I didn't really see any reasons given.

The primary reason is that types which only implement PartialEq are not really compatible with a "structural" interpretation. For example, imagine you have a match that tests against the floating point value NaN -- should this match, or not? Consider that NaN != NaN (and hence floating point types are not Eq). Currently, we sidestep this by disallowing a constant of NaN, but we won't be able to do that in the future. And there are other weird examples. Consider 0 vs -0 -- these are distinct values (different bitpatterns) and yet they compare equal. So should match foo { 0 => ... } trigger if foo is -0? Today, it does, but that is a special case for floating point values, and we don't afford user-defined types the same courtesy (that is, we use a strict structural match for user-defined types).

If we wind up adopting a "semantic" interpretation, then we could consider loosening to PartialEq.

bors added a commit that referenced this issue Mar 25, 2016

@bors
Auto merge of #32199 - nikomatsakis:limiting-constants-in-patterns-2,… 
… r=pnkfelix

Restrict constants in patterns

This implements [RFC 1445](https://github.com/rust-lang/rfcs/blob/master/text/1445-restrict-constants-in-patterns.md). The primary change is to limit the types of constants used in patterns to those that *derive* `Eq` (note that implementing `Eq` is not sufficient). This has two main effects:

1. Floating point constants are linted, and will eventually be disallowed. This is because floating point constants do not implement `Eq` but only `PartialEq`. This check replaces the existing special case code that aimed to detect the use of `NaN`.
2. Structs and enums must derive `Eq` to be usable within a match.

This is a [breaking-change]: if you encounter a problem, you are most likely using a constant in an expression where the type of the constant is some struct that does not currently implement
`Eq`. Something like the following:

```rust
struct SomeType { ... }
const SOME_CONST: SomeType = ...;

match foo {
    SOME_CONST => ...
}
```

The easiest and most future compatible fix is to annotate the type in question with `#[derive(Eq)]` (note that merely *implementing* `Eq` is not enough, it must be *derived*):

```rust
struct SomeType { ... }
const SOME_CONST: SomeType = ...;

match foo {
    SOME_CONST => ...
}
```

Another good option is to rewrite the match arm to use an `if` condition (this is also particularly good for floating point types, which implement `PartialEq` but not `Eq`):

```rust
match foo {
    c if c == SOME_CONST => ...
}
```

Finally, a third alternative is to tag the type with `#[structural_match]`; but this is not recommended, as the attribute is never expected to be stabilized. Please see RFC #1445 for more details.

cc #31434

r? @pnkfelix
4b59084

bors added a commit that referenced this issue Mar 25, 2016

@bors
Auto merge of #32199 - nikomatsakis:limiting-constants-in-patterns-2,… 
… r=pnkfelix

Restrict constants in patterns

This implements [RFC 1445](https://github.com/rust-lang/rfcs/blob/master/text/1445-restrict-constants-in-patterns.md). The primary change is to limit the types of constants used in patterns to those that *derive* `Eq` (note that implementing `Eq` is not sufficient). This has two main effects:

1. Floating point constants are linted, and will eventually be disallowed. This is because floating point constants do not implement `Eq` but only `PartialEq`. This check replaces the existing special case code that aimed to detect the use of `NaN`.
2. Structs and enums must derive `Eq` to be usable within a match.

This is a [breaking-change]: if you encounter a problem, you are most likely using a constant in an expression where the type of the constant is some struct that does not currently implement
`Eq`. Something like the following:

```rust
struct SomeType { ... }
const SOME_CONST: SomeType = ...;

match foo {
    SOME_CONST => ...
}
```

The easiest and most future compatible fix is to annotate the type in question with `#[derive(Eq)]` (note that merely *implementing* `Eq` is not enough, it must be *derived*):

```rust
struct SomeType { ... }
const SOME_CONST: SomeType = ...;

match foo {
    SOME_CONST => ...
}
```

Another good option is to rewrite the match arm to use an `if` condition (this is also particularly good for floating point types, which implement `PartialEq` but not `Eq`):

```rust
match foo {
    c if c == SOME_CONST => ...
}
```

Finally, a third alternative is to tag the type with `#[structural_match]`; but this is not recommended, as the attribute is never expected to be stabilized. Please see RFC #1445 for more details.

cc #31434

r? @pnkfelix
49e312b

Manishearth added a commit to Manishearth/rust that referenced this issue Mar 26, 2016

@Manishearth
Rollup merge of rust-lang#32199 - nikomatsakis:limiting-constants-in-… 
…patterns-2, r=pnkfelix

Restrict constants in patterns

This implements [RFC 1445](https://github.com/rust-lang/rfcs/blob/master/text/1445-restrict-constants-in-patterns.md). The primary change is to limit the types of constants used in patterns to those that *derive* `Eq` (note that implementing `Eq` is not sufficient). This has two main effects:

1. Floating point constants are linted, and will eventually be disallowed. This is because floating point constants do not implement `Eq` but only `PartialEq`. This check replaces the existing special case code that aimed to detect the use of `NaN`.
2. Structs and enums must derive `Eq` to be usable within a match.

This is a [breaking-change]: if you encounter a problem, you are most likely using a constant in an expression where the type of the constant is some struct that does not currently implement
`Eq`. Something like the following:

```rust
struct SomeType { ... }
const SOME_CONST: SomeType = ...;

match foo {
    SOME_CONST => ...
}
```

The easiest and most future compatible fix is to annotate the type in question with `#[derive(Eq)]` (note that merely *implementing* `Eq` is not enough, it must be *derived*):

```rust
struct SomeType { ... }
const SOME_CONST: SomeType = ...;

match foo {
    SOME_CONST => ...
}
```

Another good option is to rewrite the match arm to use an `if` condition (this is also particularly good for floating point types, which implement `PartialEq` but not `Eq`):

```rust
match foo {
    c if c == SOME_CONST => ...
}
```

Finally, a third alternative is to tag the type with `#[structural_match]`; but this is not recommended, as the attribute is never expected to be stabilized. Please see RFC rust-lang#1445 for more details.

cc rust-lang#31434

r? @pnkfelix
1bf8d69

Manishearth added a commit to Manishearth/rust that referenced this issue Mar 26, 2016

@Manishearth
Rollup merge of rust-lang#32199 - nikomatsakis:limiting-constants-in-… 
…patterns-2, r=pnkfelix

Restrict constants in patterns

This implements [RFC 1445](https://github.com/rust-lang/rfcs/blob/master/text/1445-restrict-constants-in-patterns.md). The primary change is to limit the types of constants used in patterns to those that *derive* `Eq` (note that implementing `Eq` is not sufficient). This has two main effects:

1. Floating point constants are linted, and will eventually be disallowed. This is because floating point constants do not implement `Eq` but only `PartialEq`. This check replaces the existing special case code that aimed to detect the use of `NaN`.
2. Structs and enums must derive `Eq` to be usable within a match.

This is a [breaking-change]: if you encounter a problem, you are most likely using a constant in an expression where the type of the constant is some struct that does not currently implement
`Eq`. Something like the following:

```rust
struct SomeType { ... }
const SOME_CONST: SomeType = ...;

match foo {
    SOME_CONST => ...
}
```

The easiest and most future compatible fix is to annotate the type in question with `#[derive(Eq)]` (note that merely *implementing* `Eq` is not enough, it must be *derived*):

```rust
struct SomeType { ... }
const SOME_CONST: SomeType = ...;

match foo {
    SOME_CONST => ...
}
```

Another good option is to rewrite the match arm to use an `if` condition (this is also particularly good for floating point types, which implement `PartialEq` but not `Eq`):

```rust
match foo {
    c if c == SOME_CONST => ...
}
```

Finally, a third alternative is to tag the type with `#[structural_match]`; but this is not recommended, as the attribute is never expected to be stabilized. Please see RFC rust-lang#1445 for more details.

cc rust-lang#31434

r? @pnkfelix
128b2ad

@brson brson referenced this issue Jul 14, 2016

Closed

Any old type of constant can be used in a pattern #20489

@brson brson added the I-nominated label Jul 14, 2016

@brson

This comment has been minimized.

Sign in to view
Copy link
Contributor

brson commented Jul 14, 2016

Nominating for status update.

@brson brson removed the I-nominated label Jul 14, 2016

@brson

This comment has been minimized.

Sign in to view
Copy link
Contributor

brson commented Jul 14, 2016

@nikomatsakis status update?
@nikomatsakis

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

nikomatsakis commented Jul 26, 2016

@brson this is basically implemented, except that I don't think I did anything clever for the exhaustiveness check. I updated the check marks. The semantics are still (in my mind) basically a kind of temporary hack.

This was referenced Oct 1, 2016

Closed
Closed

@Mark-Simulacrum Mark-Simulacrum added C-tracking-issue and removed C-enhancement C-feature-request labels Jul 24, 2017

@Centril

This comment has been minimized.

Sign in to view
Copy link
Contributor

Centril commented Sep 15, 2018

Triage: @nikomatsakis any movement here?
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.