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

Revert "Added AsRef implementations for Arc and Rc" #26160

Merged
merged 1 commit into from
Jun 13, 2015

Conversation

alexcrichton
Copy link
Member

This is a revert of PR #26008 which caused the unintended breakage reported in #26096. We may want to add these implementations in the long run, but for now this revert allows us to take some more time to evaluate the impact of such a change (e.g. run it through crater).

Closes #26096

@alexcrichton
Copy link
Member Author

r? @aturon

@rust-highfive rust-highfive assigned aturon and unassigned brson Jun 9, 2015
@rust-highfive
Copy link
Collaborator

r? @brson

(rust_highfive has picked a reviewer for you, use r? to override)

@aturon
Copy link
Member

aturon commented Jun 12, 2015

@bors: r+

@bors
Copy link
Contributor

bors commented Jun 12, 2015

📌 Commit 9d6ffbd has been approved by aturon

@bors
Copy link
Contributor

bors commented Jun 12, 2015

⌛ Testing commit 9d6ffbd with merge c6b1483...

bors added a commit that referenced this pull request Jun 12, 2015
This is a revert of PR #26008 which caused the unintended breakage reported in #26096. We may want to add these implementations in the long run, but for now this revert allows us to take some more time to evaluate the impact of such a change (e.g. run it through crater).

Closes #26096
@bors bors merged commit 9d6ffbd into rust-lang:master Jun 13, 2015
@alexcrichton alexcrichton deleted the revert-rc-as-ref branch July 10, 2015 22:32
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Oct 2, 2015
These common traits were left off originally by accident from these smart
pointers, and a past attempt (rust-lang#26008) to add them was later reverted (rust-lang#26160)
due to unexpected breakge (rust-lang#26096) occurring. The specific breakage in worry is
the meaning of this return value changed:

    let a: Box<Option<T>> = ...;
    a.as_ref()

Currently this returns `Option<&T>` but after this change it will return
`&Option<T>` because the `AsRef::as_ref` method shares the same name as
`Option::as_ref`. A [crater report][crater] of this change, however, has shown
that the fallout of this change is quite minimal. These trait implementations
are "the right impls to add" to these smart pointers and would enable various
generalizations such as those in rust-lang#27197.

[crater]: https://gist.github.com/anonymous/0ba4c3512b07641c0f99

This commit is a breaking change for the above reasons mentioned, and the
mitigation strategies look like any of:

    Option::as_ref(&a)
    a.as_ref().as_ref()
    (*a).as_ref()
bors added a commit that referenced this pull request Oct 8, 2015
These common traits were left off originally by accident from these smart
pointers, and a past attempt (#26008) to add them was later reverted (#26160)
due to unexpected breakge (#26096) occurring. The specific breakage in worry is
the meaning of this return value changed:

    let a: Box<Option<T>> = ...;
    a.as_ref()

Currently this returns `Option<&T>` but after this change it will return
`&Option<T>` because the `AsRef::as_ref` method shares the same name as
`Option::as_ref`. A [crater report][crater] of this change, however, has shown
that the fallout of this change is quite minimal. These trait implementations
are "the right impls to add" to these smart pointers and would enable various
generalizations such as those in #27197.

[crater]: https://gist.github.com/anonymous/0ba4c3512b07641c0f99

This commit is a breaking change for the above reasons mentioned, and the
mitigation strategies look like any of:

    Option::as_ref(&a)
    a.as_ref().as_ref()
    (*a).as_ref()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants