Explore possible suggestions for dangling_pointers_from_temporaries lint

[Urgau]

I wonder if it would make sense to suggest introducing a second binding in those simple let cases, aka.

    let str1 = String::with_capacity(MAX_PATH);
    let str1 = str1.as_mut_ptr();

After looking at the "regressed" code from crater, I can say that a lot of the time this suggestion will not fix the problem, and will just result in code like

let vec = some_iter.collect();
let ptr = vec.as_ptr();
return ptr;

or similar where the pointer still dangles eventually, even though we binded the temporary for now.

Sometimes it would make sense to suggest things like .into_raw_parts() instead of .as_ptr(), but then the user has to remember to do ::from_raw_parts() if they want to avoid leaking...

Most of the places where this lint fires are not trivial to fix and actually require the person writing the code to be more cautious & knowledgeable about what they are doing, which can only be achieved through more experience of writing unsafe, as well as some punches from the compiler, miri, or sanitizers.

I think that not providing this suggestion in places where it would help is less bad than improperly providing it in places where it wouldn't help but would just further obscure and hide the issue.

Originally posted by @GrigorenkoPV in #128985 (comment)

[RalfJung]

Maybe the error could be extended, to also say something along the lines of
"you must make sure that the variable you bind it to lives at least as long as the pointer
(in particular, if this pointer is returned from the current function, binding the String inside the function will not suffice)"

[Urgau]

That would already be a huge improvement over the current diagnostic.

Regarding the diagnostic, I think something like this should do it.

error: a dangling pointer will be produced because the temporary `CString` will be dropped
  --> $DIR/calls.rs:32:29
   |
LL |         let ptr = cstring().as_ptr();
   |                   --------- ^^^^^^ this pointer will immediately be invalid
   |                   |
   |                   this `CString` is deallocated at the end of the statement, bind it to a variable to extend its lifetime
   |
   = note: pointers do not have a lifetime; when calling `as_ptr` the `CString` will be deallocated at the end of the statement because nothing is referencing it as far as the type system is concerned
+  = help: you must make sure that the variable you bind the `CString` to lives at least as long as the pointer returned by the call to `as_ptr`
+  = help: in particular, if this pointer is returned from the current function, binding the `CString` inside the function will not suffice
   = help: for more information, see <https://doc.rust-lang.org/reference/destructors.html>

Note: I've slightly adjusted the wording to refer to the thing directly, instead of "it" or "the pointer"

[RalfJung]

I like it! The part I am most confused about is the (existing?) "pointers do not have a lifetime" -- it seems not directly related to the immediately next sentence? And it's not like the lifetime of a reference would magically make the referent live longer or so. I onder if it would be better to just remove that part entirely? And the part that indicates that "something referencing something" would influence the lifetime of anything. In fact I'd reword the entire first line:

= note: when calling `as_ptr` on a freshly constructed `CString`, it will be deallocated at the end of the statement
= help: you must make sure that the variable you bind the `CString` to lives at least as long as the pointer returned by the call to `as_ptr`
= help: in particular, if this pointer is returned from the current function, binding the `CString` inside the function will not suffice

[Urgau]

Sure, looks got to me. Let's see if someone wants to do it.

Steps:

1. Create new slugs as per Explore possible suggestions for dangling_pointers_from_temporaries lint #132283 (comment), in

   rust/compiler/rustc_lint/messages.ftl

   Lines 208 to 212 in 432972c

   	lint_dangling_pointers_from_temporaries = a dangling pointer will be produced because the temporary `{$ty}` will be dropped
   	.label_ptr = this pointer will immediately be invalid
   	.label_temporary = this `{$ty}` is deallocated at the end of the statement, bind it to a variable to extend its lifetime
   	.note = pointers do not have a lifetime; when calling `{$callee}` the `{$ty}` will be deallocated at the end of the statement because nothing is referencing it as far as the type system is concerned
   	.help = for more information, see <https://doc.rust-lang.org/reference/destructors.html>

   
   I recommend renaming the current help to help_visit and prefixing the new slugs with help_.
   Don't forget to update the current note as well!
2. Adjust the slugs in

   rust/compiler/rustc_lint/src/lints.rs

   Lines 1140 to 1153 in 432972c

   	// dangling.rs
   	#[derive(LintDiagnostic)]
   	#[diag(lint_dangling_pointers_from_temporaries)]
   	#[note]
   	#[help]
   	// FIXME: put #[primary_span] on `ptr_span` once it does not cause conflicts
   	pub(crate) struct DanglingPointersFromTemporaries<'tcx> {
   	pub callee: Symbol,
   	pub ty: Ty<'tcx>,
   	#[label(lint_label_ptr)]
   	pub ptr_span: Span,
   	#[label(lint_label_temporary)]
   	pub temporary_span: Span,
   	}

   
   Add new #[help(lint_NEW_SLUG_NAME)] attributes where NEW_SLUG_NAME is the name of the created slugs above.
3. Bless the dangling_pointers_from_temporaries lint tests in tests/ui/lint/dangling/.
   ./x.py test --stage 1 tests/lint/ --bless should do it

Starting points can be found on the rustc-dev-guide.

Feel free to reach out (to me or others) here or in our Zulip.

@rustbot labels +E-easy +E-mentor

[Anthony-Eid]

Could I please be assigned this issue? I think this would be a great learning opportunity for me!

[Urgau]

Sure. @rustbot assign @Anthony-Eid

[RalfJung]

@Anthony-Eid you can even do that yourself, by writing @rustbot claim :)