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.

Sign up for GitHub

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

Jump to bottom

Types with a hidden niche are not known to be non-null #79804

Merged
merged 1 commit into from
Feb 10, 2021
Merged

Types with a hidden niche are not known to be non-null #79804

merged 1 commit into from
Feb 10, 2021

Conversation

tmiasko
Copy link
Contributor

@tmiasko tmiasko commented Dec 7, 2020

Fixes #79787.

@rust-highfive
Copy link
Collaborator

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 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
varkor
varkor reviewed Dec 10, 2020
View reviewed changes
compiler/rustc_lint/src/types.rs
@@ -672,6 +672,10 @@ crate fn ty_is_known_nonnull<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, mode: C
return true;
}

if def.repr.hide_niche() {
Copy link
Member

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.

@varkor
Copy link
Member

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
Member

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

@pnkfelix
Copy link
Member

@bors r+

@bors
Copy link
Contributor

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
Contributor

bors commented Feb 10, 2021

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

@bors
Copy link
Contributor

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
Reviewers

@varkor varkor varkor left review comments

Assignees

@pnkfelix pnkfelix

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
Milestone
1.52.0
Development

Successfully merging this pull request may close these issues.

ICE: improper_ctypes: Option nonnull optimization not applied?
9 participants
@tmiasko @rust-highfive @varkor @pnkfelix @bors @Mark-Simulacrum @JohnCSimon @camelid @rustbot