Update documentation of as_ptr function of Atomic$Int to clarify circumstances of usage

[madhav-madhusoodanan]

Context

The documentation of as_ptr mentions that "operations on it must be atomic". This may confuse developers, since non-atomic reads may be safely performed in certain circumstances (eg: when they do not lead to a data race).

The core message is that non-atomic accesses are UB if they cause a data race.

This update ensures such clarity on the circumstances of usage of the as_ptr function.

Associated Issue

• Atomic*::as_ptr unnecessarily restricts operations on the pointer to atomic operations #138246

cc: @RalfJung @briansmith

[rustbot]

r? @thomcc

rustbot has assigned @thomcc.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[briansmith]

Thanks for taking this on.

[madhav-madhusoodanan]

I've added an example where there is no unsafe block being used.
Do let me know if its a good example.

[RalfJung]

@rustbot author

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[madhav-madhusoodanan]

@rustbot ready

[RalfJung]

I don't get the first part of this sentence... what else would they do?

[RalfJung]

Github is being silly and doesn't let me make this a suggestion, so let me post this manually -- how about:

            /// Note that doing non-atomic reads or writes on the resulting integer can be
            /// Undefined Behavior due to a data race; see the [memory model section] for further information.
            ///
            /// This method is mostly useful for FFI, where the function signature may use
            #[doc = concat!("`*mut ", stringify!($int_type), "` instead of `&", stringify!($atomic_type), "`.")]

As I said before, this should link to the module-level memory model docs. You can probably use an intra-doc link like that... crate::sync::atomic#memory-model-for-atomic-accesses should work.

[RalfJung]

@rustbot author

[madhav-madhusoodanan]

Just wanted to clarify one thing, would it be a good idea to note about interior mutability too?

[RalfJung]

No, I would say interior mutability is an implementation detail.

[madhav-madhusoodanan]

Got it.

[madhav-madhusoodanan]

@rustbot ready

[RalfJung]

Suggested change

	/// Undefined Behavior due to a data race; see the [Memory model section] for further information.
	/// Undefined Behavior due to a data race; see the [memory model section] for further information.

Why did you not just copy-paste my suggestion?

[madhav-madhusoodanan]

Apologies.

I take it as an exercise in learning and understanding the nuances of what my changes do, especially when I'm dealing with comments. Hence i usually avoid directly copy pasting.

[madhav-madhusoodanan]

May I ask if intradoc links are case sensitive? I'm slightly new to rustdoc and I wasn't able to notice this detail.

[RalfJung]

This is just standard markdown, not rustdoc: if you write [Memory], it will be shown as "Memory" in the rendered HTML, but we do not want this to be capitalized.

I take it as an exercise in learning and understanding the nuances of what my changes do, especially when I'm dealing with comments. Hence i usually avoid directly copy pasting.

Feel free to do this and then diff the result, but as a reviewer it takes a lot of extra attention if I can't even rely on suggestions being realized correctly. You can of course have a good reason to diverge from my suggestion, but then you should explain why.

[RalfJung]

What is the point of this last sentence? It did not exist in my suggestion.

[madhav-madhusoodanan]

That was not added. That is the consequence of removing the sentences surrounding it in this commit:

1. The "interior mutability" detail
2. The "unsafe" block detail

[RalfJung]

I guess I wasn't clear enough then when posting my suggestion -- please remove this sentence. I've already told you a week ago that by removing the sentence before this, you made this sentence here entirely void of context and confusing.

[RalfJung]

@madhav-madhusoodanan if you prefer I can just post a PR myself for this, that has a lot less friction than me trying to explain to you my suggested edits, in particular since we seem to need 2 round-trips for each suggestion. ;)

[RalfJung]

That odd sentence "All modifications of an atomic change the value through a shared reference, and can do so safely as long as they use atomic operations." is still around. I don't know how to make it any more clear that you should remove it everywhere. It made sense before your PR due to the surrounding context, but you removed that context, and now it's entirely unclear what the point of that sentence is.

@rustbot author