`dangling_pointers_from_temporaries` lint is imprecise (false positives and false negatives)

[jrmuizel]

I tried this code:

use std::ffi::CString;

extern "C" {
    fn gecko_profiler_register_thread(foo: *const std::os::raw::c_char);
}

pub fn main() {
    unsafe {
        gecko_profiler_register_thread(CString::new("foo").unwrap().as_ptr());
    }
}

I get:

warning: getting the inner pointer of a temporary `CString`

 --> <source>:9:69

  |

9 |         gecko_profiler_register_thread(CString::new("foo").unwrap().as_ptr());

  |                                        ---------------------------- ^^^^^^ this pointer will be invalid

  |                                        |

  |                                        this `CString` is deallocated at the end of the statement, bind it to a variable to extend its lifetime

  |

  = note: `#[warn(temporary_cstring_as_ptr)]` on by default

  = 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: for more information, see https://doc.rust-lang.org/reference/destructors.html


warning: 1 warning emitted


Compiler returned: 0

However since the CString is not deallocated until after the function call the code is fine and the warning should not trigger.

See https://rust.godbolt.org/z/vjrvGn for the assembly.

[jyn514]

This is the edge case I mentioned in #75671 (comment). @matklad and few others said

it is false-positive for "this code is UB", as it is not UB. But it absolutely is a true positive for "this code is confusing, because you need to be a language lawyer to understand why it is not UB".

[dtolnay]

I would like if this were fixed. I find the comment about language lawyers uncharitable. Emitting a scary warning like this in correct code just puts even more of a needless barrier toward someone becoming a language lawyer. The relevant piece of the language (temporaries drop at the end of the statement) is quite simple — understanding why the linter is actively misleading you about the correctness of your code is a way bigger source of confusion than the language itself.

If someone wants to keep a lint for that case because they find the code confusing, that lint belongs in Clippy. I believe the uplift to rustc was only meant for known bad code, such as let ptr = CString::new(...).as_ptr().

[joshtriplett]

We discussed this in the @rust-lang/lang meeting today. We agreed that we shouldn't lint on this case; the goal of the lint is to catch known-wrong cases, particularly cases that we've seen commonly used by mistake. We shouldn't have false positives, and it's fine to rely on a temporary lasting until the end of the statement.

We do want to make sure that, in improving this lint to avoid the false positves, we don't stop warning on some true positives.

Here are the cases we want to cover, which we'd like to see in a test case:

• Lint on:
  • let x = CString::new("foo").unwrap().as_ptr(); (the most common case, which this lint was primarily invented for)
  • let x = Some(CString::new("foo").unwrap().as_ptr());
  • let x = Foo { field: CString::new("foo").unwrap().as_ptr() };
  • let x = (CString::new("foo").unwrap().as_ptr(), another_tuple_component)
• Don't lint on:
  • let x = foo(CString::new("foo").unwrap().as_ptr()); (might be an issue if foo stores the pointer somewhere that must outlive the call, but we can't know that, so we can't lint)
  • let x = foo(Some(CString::new("foo").unwrap().as_ptr()));
  • let x = foo(Foo { field: CString::new("foo").unwrap().as_ptr() });
  • let x = foo((CString::new("foo").unwrap().as_ptr(), another_tuple_component));
  • let x = foo(x.as_ptr()) where foo is the identity function

[correabuscar]

Looks like when using the question mark(aka try operator) it correctly doesn't fire:

some_function( CString::new("this correctly doesn't warn")?.as_ptr() );
some_function( CString::new("this warns but it shouldn't").unwrap().as_ptr() );

playground