Expand, rename and improve `incorrect_fn_null_checks` lint #113657

Urgau · 2023-07-13T14:03:14Z

This PR,

firstly, expand the lint by now linting on references
secondly, it renames the lint incorrect_fn_null_checks -> useless_ptr_null_checks
and thirdly it improves the lint by catching ptr::from_mut, ptr::from_ref, as well as <*mut _>::cast and <*const _>::cast_mut

Fixes #113601
cc @est31

rustbot · 2023-07-13T14:03:22Z

r? @wesleywiser

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

rustbot · 2023-07-13T14:03:25Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

est31 · 2023-07-13T18:02:31Z

Very good work, in the future it would be also good to lint on these:

Box::into_raw
NonNull::as_ptr
{str, Vec, String, slice}::as_ptr/{str, Vec, String, slice}::as_mut_ptr, although here we need to be sure they return non-null pointers for empty collections
{Cell,Weak,Rc,Arc}::as_ptr
CStr::as_ptr

etc. Maybe it might make sense to introduce a #[rustc_pointer_non_null] attribute and add it to these functions?

That's more for follow up work though.

When working on #113652 I was also briefly considering using ty_is_known_nonnull from types.rs, but that one is about types and what we really need to check for is conversions, whether it's a cast or one of the known functions. We can't just take any function on these types with a pointer return value, the function might actually do something completely different. In other words, the approach of this PR is good.

est31 · 2023-07-13T18:04:05Z

That's more for follow up work though.

Especially I'd like this to get merged sooner than later as it would be nice if the lint rename wasn't exposed to stable users. Beta has just branched, so we have weeks of time, but still.

compiler/rustc_lint/src/ptr_nulls.rs

est31 · 2023-07-13T18:12:30Z

Approved this but won't r as I think it would be better for someone from the compiler team to approve it -- note that as per @scottmcm this doesn't need a lang team FCP.

bors · 2023-08-01T03:52:35Z

☔ The latest upstream changes (presumably #114310) made this pull request unmergeable. Please resolve the merge conflicts.

compiler/rustc_lint/src/ptr_nulls.rs

cjgillot · 2023-08-03T07:36:20Z

compiler/rustc_lint/src/lints.rs

+pub enum PtrNullChecksDiag<'a> {
+ #[diag(lint_ptr_null_checks_fn_ptr)]
+ #[help(lint_help)]
+ FnPtr,


Should the expression's span and type be added here too, for symmetry?

Don't know, but added anyway since it doesn't make the output any worse.

cjgillot · 2023-08-03T13:21:59Z

@bors r rollup

bors · 2023-08-03T13:22:02Z

📌 Commit ee51953 has been approved by cjgillot

It is now in the queue for this repository.

…ck-lint, r=cjgillot Expand, rename and improve `incorrect_fn_null_checks` lint This PR, - firstly, expand the lint by now linting on references - secondly, it renames the lint `incorrect_fn_null_checks` -> `useless_ptr_null_checks` - and thirdly it improves the lint by catching `ptr::from_mut`, `ptr::from_ref`, as well as `<*mut _>::cast` and `<*const _>::cast_mut` Fixes rust-lang#113601 cc `@est31`

…ck-lint, r=cjgillot Expand, rename and improve `incorrect_fn_null_checks` lint This PR, - firstly, expand the lint by now linting on references - secondly, it renames the lint `incorrect_fn_null_checks` -> `useless_ptr_null_checks` - and thirdly it improves the lint by catching `ptr::from_mut`, `ptr::from_ref`, as well as `<*mut _>::cast` and `<*const _>::cast_mut` Fixes rust-lang#113601 cc ``@est31``

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#113657 (Expand, rename and improve `incorrect_fn_null_checks` lint) - rust-lang#114237 (parser: more friendly hints for handling `async move` in the 2015 edition) - rust-lang#114300 (Suggests turbofish in patterns) - rust-lang#114372 (const validation: point at where we found a pointer but expected an integer) - rust-lang#114395 ([rustc_span][perf] Hoist lookup sorted by words out of the loop.) - rust-lang#114403 (fix the span in the suggestion of remove question mark) - rust-lang#114408 (Temporary remove myself from review rotation) - rust-lang#114415 (Skip checking of `rustc_codegen_gcc` with vendoring enabled) r? `@ghost` `@rustbot` modify labels: rollup

It was added by rust-lang#113657 for its purposes. Now it is not used any more, remove it, as we use the attr now.

…ck-lint, r=cjgillot Expand, rename and improve `incorrect_fn_null_checks` lint This PR, - firstly, expand the lint by now linting on references - secondly, it renames the lint `incorrect_fn_null_checks` -> `useless_ptr_null_checks` - and thirdly it improves the lint by catching `ptr::from_mut`, `ptr::from_ref`, as well as `<*mut _>::cast` and `<*const _>::cast_mut` Fixes rust-lang#113601 cc ```@est31```

…, r=jackh726 Make useless_ptr_null_checks smarter about some std functions This teaches the `useless_ptr_null_checks` lint that some std functions can't ever return null pointers, because they need to point to valid data, get references as input, etc. This is achieved by introducing an `#[rustc_never_returns_null_ptr]` attribute and adding it to these std functions (gated behind bootstrap `cfg_attr`). Later on, the attribute could maybe be used to tell LLVM that the returned pointer is never null. I don't expect much impact of that though, as the functions are pretty shallow and usually the input data is already never null. Follow-up of PR rust-lang#113657 Fixes rust-lang#114442

rustbot assigned wesleywiser Jul 13, 2023

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. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jul 13, 2023

est31 mentioned this pull request Jul 13, 2023

Extend incorrect_fn_null_checks to incorrect_non_null_checks #113652

Closed

This comment has been minimized.

Sign in to view

Urgau force-pushed the expand-incorrect_fn_null_check-lint branch from 71652f6 to bbc85af Compare July 13, 2023 14:19

est31 approved these changes Jul 13, 2023

View reviewed changes

compiler/rustc_lint/src/ptr_nulls.rs Outdated Show resolved Hide resolved

Urgau force-pushed the expand-incorrect_fn_null_check-lint branch from bbc85af to ae36b80 Compare July 13, 2023 18:48

Urgau added 5 commits August 1, 2023 19:35

Expand incorrect_fn_null_check lint with reference null checking

743ae5a

Rename incorrect_fn_null_checks to useless_ptr_null_checks

d2b7c80

Rename incorrect_fn_null_checks to useless_ptr_null_checks (clippy side)

84c5372

Add more tests for useless_ptr_null_checks lint

ef3413d

Add diagnostic items for <*const _>::cast and ptr::from_mut

0b9529c

Urgau force-pushed the expand-incorrect_fn_null_check-lint branch from ae36b80 to de576e1 Compare August 1, 2023 18:04

cjgillot reviewed Aug 3, 2023

View reviewed changes

cjgillot self-assigned this Aug 3, 2023

Urgau added 2 commits August 3, 2023 10:52

Also lint on cast/cast_mut and ptr::from_mut/ptr::from_ref

4b3dadb

Also add label with original type for function pointers

ee51953

Urgau force-pushed the expand-incorrect_fn_null_check-lint branch from de576e1 to ee51953 Compare August 3, 2023 08:58

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 Aug 3, 2023

matthiaskrgr mentioned this pull request Aug 3, 2023

Rollup of 7 pull requests #114419

Closed

This was referenced Aug 3, 2023

Rollup of 8 pull requests #114421

Closed

Rollup of 8 pull requests #114424

Merged

bors merged commit 7518ae5 into rust-lang:master Aug 3, 2023
11 checks passed

rustbot added this to the 1.73.0 milestone Aug 3, 2023

est31 mentioned this pull request Aug 3, 2023

Extend useless_ptr_null_checks to more functions #114442

Closed

Urgau deleted the expand-incorrect_fn_null_check-lint branch August 4, 2023 07:09

est31 mentioned this pull request Aug 5, 2023

Make useless_ptr_null_checks smarter about some std functions #114494

Merged

est31 added a commit to est31/rust that referenced this pull request Aug 5, 2023

Remove ptr_from_mut diagnostic item

8faac74

It was added by rust-lang#113657 for its purposes. Now it is not used any more, remove it, as we use the attr now.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expand, rename and improve `incorrect_fn_null_checks` lint #113657

Expand, rename and improve `incorrect_fn_null_checks` lint #113657

Urgau commented Jul 13, 2023 •

edited

Loading

rustbot commented Jul 13, 2023

rustbot commented Jul 13, 2023

This comment has been minimized.

est31 commented Jul 13, 2023 •

edited

Loading

est31 commented Jul 13, 2023

est31 commented Jul 13, 2023

bors commented Aug 1, 2023

cjgillot Aug 3, 2023

Urgau Aug 3, 2023

cjgillot commented Aug 3, 2023

bors commented Aug 3, 2023

Expand, rename and improve incorrect_fn_null_checks lint #113657

Expand, rename and improve incorrect_fn_null_checks lint #113657

Conversation

Urgau commented Jul 13, 2023 • edited Loading

rustbot commented Jul 13, 2023

rustbot commented Jul 13, 2023

This comment has been minimized.

est31 commented Jul 13, 2023 • edited Loading

est31 commented Jul 13, 2023

est31 commented Jul 13, 2023

bors commented Aug 1, 2023

cjgillot Aug 3, 2023

Choose a reason for hiding this comment

Urgau Aug 3, 2023

Choose a reason for hiding this comment

cjgillot commented Aug 3, 2023

bors commented Aug 3, 2023

Expand, rename and improve `incorrect_fn_null_checks` lint #113657

Expand, rename and improve `incorrect_fn_null_checks` lint #113657

Urgau commented Jul 13, 2023 •

edited

Loading

est31 commented Jul 13, 2023 •

edited

Loading