Can Weak be null?

[est31]

There is a discrepancy between the docs of Weak and its implementation, originally found by @Urgau in #114494 (comment) .

On one hand, the docs of Weak::as_ptr state:

The pointer is valid only if there are some strong references. The pointer may be dangling, unaligned or even null otherwise.

On the other hand, the implementation of Weak uses NonNull:

rust/library/alloc/src/rc.rs

Lines 2681 to 2693 in 1cabb8e

	pub struct Weak<
	T: ?Sized,
	#[unstable(feature = "allocator_api", issue = "32838")] A: Allocator = Global,
	> {
	// This is a `NonNull` to allow optimizing the size of this type in enums,
	// but it is not necessarily a valid pointer.
	// `Weak::new` sets this to `usize::MAX` so that it doesn’t need
	// to allocate space on the heap. That's not a value a real pointer
	// will ever have because RcBox has alignment at least 2.
	// This is only possible when `T: Sized`; unsized `T` never dangle.
	ptr: NonNull<RcBox<T>>,
	alloc: A,
	}

From what I know, it is okay for NonNull pointers to be dangling or unaligned, but, staying true to their name, they are never allowed to be null pointers.

I think this discrepancy should be resolved, but I wonder, which way? Should the docs be adjusted, or the implementation?

The issue exists both with alloc::sync::Weak as well as alloc::rc::Weak.

[est31]

Some research on the history: Weak::as_ptr was introduced by #60766 (under a different name), tracking issue #60728. That PR already included the note that the pointer can't be null, even though already back then, Weak was using a NonNull internally.

[Zalathar]

I don't think there's a discrepancy here?

The documentation says that the method isn't guaranteed to return a non-null pointer.

The current implementation happens to never return a null pointer due to an internal detail (using NonNull), but that detail isn't a documented guarantee, so callers can't rely on it.

(For example, changing the implementation to return a null pointer in some circumstances would not be a breaking change, because the documented behaviour allows that possibility, even though the current implementation doesn't exercise it.)

[RalfJung]

Yeah, the key point is that from_raw does not allow null pointers.

[est31]

but that detail isn't a documented guarantee, so callers can't rely on it.

Good point, makes sense.

from_raw does not allow null pointers.

This is not written anywhere in the docs of from_raw. It only says that the weak has to come from into_raw. The obvious alternatives like as_ptr plus forget (which from_raw is a wrapper of), or into_raw_and_alloc are missing. into_raw mentions nowhere that it can return a null pointer either.

I think there is a small, but non zero chance that when people read that the output of as_ptr can be a null pointer, they could assume that from_raw can take the input of as_ptr as well, and therefore, also a null pointer.

I'll make a PR to improve the docs.

[est31]

PR filed: #114525