Skip to content
New issue

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

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

Already on GitHub? # to your account

Remove T: Sized on ptr::is_null() #46094

Merged
merged 1 commit into from
Nov 28, 2017
Merged

Remove T: Sized on ptr::is_null() #46094

merged 1 commit into from
Nov 28, 2017

Conversation

dtolnay
Copy link
Member

@dtolnay dtolnay commented Nov 19, 2017

Originally from #44932 -- this is purely a revert of the last commit of that PR, which was removing some changes from the previous commits in the PR. So a revert of a revert means this is code written by @cuviper!

@mikeyhew makes a compelling case in rust-lang/rfcs#433 (comment) for why this is the right way to implement is_null for trait objects. And the behavior for slices makes sense to me as well.

  impl<T: ?Sized> *const T {
-     pub fn is_null(self) -> bool where T: Sized;
+     pub fn is_null(self) -> bool;
  }

  impl<T: ?Sized> *mut T {
-     pub fn is_null(self) -> bool where T: Sized;
+     pub fn is_null(self) -> bool;
  }

This reverts commit 604f049.

This is purely a revert of cuviper's revert "Restore `T: Sized` on
`ptr::is_null`". So double revert means this is code written by cuviper!
@dtolnay dtolnay added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Nov 19, 2017
@kennytm kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 19, 2017
@bluss bluss added the relnotes Marks issues that should be documented in the release notes of the next release. label Nov 19, 2017
@dtolnay
Copy link
Member Author

dtolnay commented Nov 21, 2017

@rfcbot fcp merge

@rfcbot
Copy link

rfcbot commented Nov 21, 2017

Team member @dtolnay 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.

@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 Nov 21, 2017
@carols10cents
Copy link
Member

Ping @BurntSushi, waiting on your ticky box here!

@rfcbot
Copy link

rfcbot commented Nov 28, 2017

🔔 This is now entering its final comment period, as per the review above. 🔔

1 similar comment
@rfcbot
Copy link

rfcbot commented Nov 28, 2017

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Nov 28, 2017
@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Nov 28, 2017

📌 Commit e0f58c6 has been approved by alexcrichton

@kennytm kennytm 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 Nov 28, 2017
@bors
Copy link
Contributor

bors commented Nov 28, 2017

⌛ Testing commit e0f58c6 with merge 73bca2b...

bors added a commit that referenced this pull request Nov 28, 2017
Remove `T: Sized` on `ptr::is_null()`

Originally from #44932 -- this is purely a revert of the last commit of that PR, which was removing some changes from the previous commits in the PR. So a revert of a revert means this is code written by @cuviper!

@mikeyhew makes a compelling case in rust-lang/rfcs#433 (comment) for why this is the right way to implement `is_null` for trait objects. And the behavior for slices makes sense to me as well.

```diff
  impl<T: ?Sized> *const T {
-     pub fn is_null(self) -> bool where T: Sized;
+     pub fn is_null(self) -> bool;
  }

  impl<T: ?Sized> *mut T {
-     pub fn is_null(self) -> bool where T: Sized;
+     pub fn is_null(self) -> bool;
  }
@bors
Copy link
Contributor

bors commented Nov 28, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 73bca2b to master...

@bors bors merged commit e0f58c6 into rust-lang:master Nov 28, 2017
@dtolnay dtolnay deleted the is_null branch December 13, 2017 15:03
sgrif added a commit to diesel-rs/diesel that referenced this pull request May 22, 2018
This was already true, as we require
rust-lang/rust#46094 to compile. This commit
ensures that we test for this on CI so we know if this changes in the
future.
@cuviper cuviper mentioned this pull request Nov 11, 2019
@dtolnay dtolnay removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Jan 14, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
relnotes Marks issues that should be documented in the release notes of the next release. 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