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

Implement Eq/Hash/Debug etc. for unsized tuples. #43011

Merged
merged 2 commits into from
Jul 12, 2017

Conversation

qnighy
Copy link
Contributor

@qnighy qnighy commented Jul 2, 2017

As I mentioned in the comment in #18469, the implementations of PartialEq, Eq, PartialOrd, Ord, Debug, Hash can be generalized to unsized tuples.

This is consistent with the derive behavior for unsized structs.

#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug, Default, Hash)]
struct MyTuple<X, Y, Z: ?Sized>(X, Y, Z);

fn f(x: &MyTuple<i32, i32, [i32]>) {
    x == x;
    x < x;
    println!("{:?}", x);
}

Questions:

  • Need an RFC?
  • Need a feature gate? I don't think it does because the unsized tuple coercion Unsized tuple coercions #42527 is feature-gated.
  • I changed builder.field($name); into builder.field(&$name); in the Debug implementation to pass compilation. This won't affect the behavior because Debug for &'a T is a mere redirection to Debug for T. However, I don't know if it affects code size / performance.

@Mark-Simulacrum
Copy link
Member

r? @sfackler

@Mark-Simulacrum Mark-Simulacrum added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 2, 2017
@sfackler sfackler added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jul 2, 2017
@sfackler
Copy link
Member

sfackler commented Jul 2, 2017

No RFC is needed, or a feature - trait implementations can't actually have feature gates applied to them.

@rfcbot fcp merge

@rfcbot rfcbot added the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Jul 2, 2017
@rfcbot
Copy link

rfcbot commented Jul 2, 2017

Team member @sfackler has proposed to merge this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@carols10cents
Copy link
Member

ping @brson, you're the last checkbox needed here!

@aturon
Copy link
Member

aturon commented Jul 10, 2017

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Jul 10, 2017

📌 Commit 728d26c has been approved by aturon

@arielb1 arielb1 added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 11, 2017
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Jul 12, 2017
Implement Eq/Hash/Debug etc. for unsized tuples.

As I mentioned in [the comment in rust-lang#18469](rust-lang#18469 (comment)), the implementations of `PartialEq`, `Eq`, `PartialOrd`, `Ord`, `Debug`, `Hash` can be generalized to unsized tuples.

This is consistent with the `derive` behavior for unsized structs.

```rust
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug, Default, Hash)]
struct MyTuple<X, Y, Z: ?Sized>(X, Y, Z);

fn f(x: &MyTuple<i32, i32, [i32]>) {
    x == x;
    x < x;
    println!("{:?}", x);
}
```

Questions:

- Need an RFC?
- Need a feature gate? I don't think it does because the unsized tuple coercion rust-lang#42527 is feature-gated.
- I changed `builder.field($name);` into `builder.field(&$name);` in the `Debug` implementation to pass compilation. This won't affect the behavior because `Debug for &'a T` is a mere redirection to `Debug for T`. However, I don't know if it affects code size / performance.
bors added a commit that referenced this pull request Jul 12, 2017
Rollup of 8 pull requests

- Successful merges: #42670, #42826, #43000, #43011, #43098, #43100, #43136, #43137
- Failed merges:
@bors bors merged commit 728d26c into rust-lang:master Jul 12, 2017
@rfcbot rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Jul 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants