Skip to content

Suggest using ptr::null_mut when user provided ptr::null to a function expecting ptr::null_mut #112302

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

Merged
merged 1 commit into from
Jun 12, 2023

Conversation

jieyouxu
Copy link
Member

@jieyouxu jieyouxu commented Jun 5, 2023

error[E0308]: mismatched types
  --> $DIR/ptr-null-mutability-suggestions.rs:9:24
   |
LL |     expecting_null_mut(ptr::null());
   |     ------------------ ^^^^^^^^^^^
   |     |                  |
   |     |                  types differ in mutability
   |     |                  help: consider using `core::ptr::null_mut` instead: `core::ptr::null_mut()`
   |     arguments to this function are incorrect
   |
   = note: expected raw pointer `*mut u8`
              found raw pointer `*const _`
note: function defined here
  --> $DIR/ptr-null-mutability-suggestions.rs:6:4
   |
LL | fn expecting_null_mut(_: *mut u8) {}
   |    ^^^^^^^^^^^^^^^^^^ ----------

Closes #85184.

@rustbot
Copy link
Collaborator

rustbot commented Jun 5, 2023

r? @TaKO8Ki

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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 5, 2023
@clubby789 clubby789 added the A-diagnostics Area: Messages for errors, warnings, and lints label Jun 5, 2023
Comment on lines 1282 to 1301
if let ty::RawPtr(ty::TypeAndMut { mutbl: expected_mutbl, .. }) = expected_ty.kind()
&& let ty::RawPtr(ty::TypeAndMut { mutbl: provided_mutbl, .. }) = provided_ty.kind()
&& let provided_arg_expr = *provided_args[provided_idx]
&& let hir::ExprKind::Call(callee, _) = provided_arg_expr.kind
&& let hir::ExprKind::Path(hir::QPath::Resolved(_, path)) = callee.kind
&& let Res::Def(_, def_id) = path.res
{
match (expected_mutbl, provided_mutbl) {
(hir::Mutability::Mut, hir::Mutability::Not) => {
if let Some(ptr_null_def_id) = self.tcx.get_diagnostic_item(sym::ptr_null)
&& def_id == ptr_null_def_id
{
// The user provided `ptr::null()`, but the function expects
// `ptr::null_mut()`.
err.subdiagnostic(SuggestPtrNullMut {
span: provided_arg_expr.span
});
}
},
_ => {},
}
}
Copy link
Member

Choose a reason for hiding this comment

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

You can simplify the condition quite a lot, maybe something like

Suggested change
if let ty::RawPtr(ty::TypeAndMut { mutbl: expected_mutbl, .. }) = expected_ty.kind()
&& let ty::RawPtr(ty::TypeAndMut { mutbl: provided_mutbl, .. }) = provided_ty.kind()
&& let provided_arg_expr = *provided_args[provided_idx]
&& let hir::ExprKind::Call(callee, _) = provided_arg_expr.kind
&& let hir::ExprKind::Path(hir::QPath::Resolved(_, path)) = callee.kind
&& let Res::Def(_, def_id) = path.res
{
match (expected_mutbl, provided_mutbl) {
(hir::Mutability::Mut, hir::Mutability::Not) => {
if let Some(ptr_null_def_id) = self.tcx.get_diagnostic_item(sym::ptr_null)
&& def_id == ptr_null_def_id
{
// The user provided `ptr::null()`, but the function expects
// `ptr::null_mut()`.
err.subdiagnostic(SuggestPtrNullMut {
span: provided_arg_expr.span
});
}
},
_ => {},
}
}
if let ty::RawPtr(ty::TypeAndMut { mutbl: hir::Mutability::Mut, .. }) = expected_ty.kind()
&& let ty::RawPtr(ty::TypeAndMut { mutbl: hir::Mutability::Not, .. }) = provided_ty.kind()
&& let provided_arg_expr = *provided_args[provided_idx]
&& let hir::ExprKind::Call(callee, _) = provided_arg_expr.kind
&& let hir::ExprKind::Path(hir::QPath::Resolved(_, path)) = callee.kind
&& let Res::Def(_, def_id) = path.res
&& Some(def_id) == self.tcx.get_diagnostic_item(sym::ptr_null)
{
// The user provided `ptr::null()`, but the function expects
// `ptr::null_mut()`.
err.subdiagnostic(SuggestPtrNullMut {
span: provided_arg_expr.span
});
}

@WaffleLapkin WaffleLapkin assigned WaffleLapkin and unassigned TaKO8Ki Jun 8, 2023
@WaffleLapkin WaffleLapkin 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 Jun 8, 2023
@jieyouxu
Copy link
Member Author

jieyouxu commented Jun 9, 2023

@rustbot ready

@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 Jun 9, 2023
Comment on lines 1296 to 1297
&& *expected_mutbl == hir::Mutability::Mut
&& *provided_mutbl == hir::Mutability::Not
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure why you don't want to just ty::RawPtr(ty::TypeAndMut { mutbl: hir::Mutability::Mut, .. }) in the pattern above, as imo it reads quite good: expected_ty is a raw pointer with mut mutability...

But if you don't like to do it in the pattern, at least use .is_mut()/.is_not() and move those two checks to the top, just after the corresponding let raw_ptr = ty.kind()-ish code.

Comment on lines 1294 to 1295
&& let Some(ptr_null_def_id) = self.tcx.get_diagnostic_item(sym::ptr_null)
&& def_id == ptr_null_def_id
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
&& let Some(ptr_null_def_id) = self.tcx.get_diagnostic_item(sym::ptr_null)
&& def_id == ptr_null_def_id
&& Some(def_id) == self.tcx.get_diagnostic_item(sym::ptr_null)

or maybe

Suggested change
&& let Some(ptr_null_def_id) = self.tcx.get_diagnostic_item(sym::ptr_null)
&& def_id == ptr_null_def_id
&& self.tcx.get_diagnostic_item(sym::ptr_null) == Some(def_id)

@WaffleLapkin
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jun 12, 2023

📌 Commit 432ce39 has been approved by WaffleLapkin

It is now in the queue for this repository.

@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 Jun 12, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 12, 2023
…iaskrgr

Rollup of 4 pull requests

Successful merges:

 - rust-lang#112302 (Suggest using `ptr::null_mut` when user provided `ptr::null` to a function expecting `ptr::null_mut`)
 - rust-lang#112416 (Fix debug ICE for extern type with where clauses)
 - rust-lang#112527 (Add windows_sys type definitions for ARM32 manually)
 - rust-lang#112546 (new solver: extend assert to other aliases)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a5245b5 into rust-lang:master Jun 12, 2023
@rustbot rustbot added this to the 1.72.0 milestone Jun 12, 2023
@jieyouxu jieyouxu deleted the issue-85184 branch October 22, 2024 12:35
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints 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.

Suggestion for diagnostic imoprovement in case of null pointer that differs in mutability
6 participants