Skip to content

Conversation

@tmiasko
Copy link
Contributor

@tmiasko tmiasko commented Dec 7, 2020

Fixes #79787.

@rust-highfive
Copy link
Contributor

r? @varkor

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 7, 2020
@tmiasko tmiasko marked this pull request as draft December 8, 2020 10:12
@tmiasko tmiasko force-pushed the improper-ctypes-no-niche branch from e6fbfd5 to 416ef64 Compare December 8, 2020 14:59
@tmiasko tmiasko changed the title Check if niche is hidden in improper_ctypes lint Types with a hidden niche are not known to be non-null Dec 8, 2020
@varkor varkor added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 8, 2020
@tmiasko tmiasko marked this pull request as ready for review December 8, 2020 15:27
@tmiasko
Copy link
Contributor Author

tmiasko commented Dec 8, 2020

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 8, 2020
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a comment here explaining why, for someone who may not already be aware when niches are hidden?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a comment mentioning no_niche and UnsafeCell.

@tmiasko tmiasko force-pushed the improper-ctypes-no-niche branch from 416ef64 to 8fc2462 Compare December 10, 2020 13:23
@varkor
Copy link
Contributor

varkor commented Dec 10, 2020

Implementation looks fine, but I'm not familiar enough with the purpose of no_niche to be happy reviewing.

r? @pnkfelix

@rust-highfive rust-highfive assigned pnkfelix and unassigned varkor Dec 10, 2020
@camelid camelid added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jan 15, 2021
@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 1, 2021
@pnkfelix
Copy link
Contributor

sorry it took me so long to get to this. This seems right to me.

@pnkfelix
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Feb 10, 2021

📌 Commit 8fc2462 has been approved by pnkfelix

@bors bors 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 Feb 10, 2021
@Mark-Simulacrum
Copy link
Member

@bors rollup=never - possibly perf sensitive

@bors
Copy link
Collaborator

bors commented Feb 10, 2021

⌛ Testing commit 8fc2462 with merge 07194ff...

@bors
Copy link
Collaborator

bors commented Feb 10, 2021

☀️ Test successful - checks-actions
Approved by: pnkfelix
Pushing 07194ff to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 10, 2021
@bors bors merged commit 07194ff into rust-lang:master Feb 10, 2021
@rustbot rustbot added this to the 1.52.0 milestone Feb 10, 2021
@tmiasko tmiasko deleted the improper-ctypes-no-niche branch February 10, 2021 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ICE: improper_ctypes: Option nonnull optimization not applied?

9 participants