Update Unsafe and MemoryMarshal docs

[GrabYourPitchforks]

Summary

Significantly fleshes out the docs for Unsafe, including:

• Using more accurate terminology, such as using "managed pointer" instead of "reference" in APIs which are performing pointer manipulation. With this change I'm trying to reserve the word "reference" for APIs which are reading & writing values rather than performing pointer manipulation.
• Writing the API's assumptions in the Remarks section, alongside ECMA sections we expect callers to be familiar with.

The changes for MemoryMarshal are a bit tamer by comparison:

• Adding missing "this API can throw OverflowException" lines.
• Removing incorrect references to "T can't contain pointers."
• Removing incorrect references to "this method ensures type safety."

[GrabYourPitchforks]

/cc @dotnet/area-system-runtime-compilerservices for comments on the Unsafe docs changes.

[GrabYourPitchforks]

BTW, I'm totally open to suggestions on how we might flesh out the Unsafe documentation to call out when you'd actually want to use it. If nothing else I can add a "here's how you reverse a span with no bounds checks" sample, which would show GetReference, Add, Subtract, and IsAddressLessThan. But something with a bit more oomph might be better.

[opbld31]

Docs Build status updates of commit d48b85b:

⚠️ Validation status: warnings

File	Status	Preview URL	Details
xml/System.Runtime.CompilerServices/Unsafe.xml	⚠️Warning	View	Details
xml/System.Runtime.InteropServices/MemoryMarshal.xml	✅Succeeded	View

xml/System.Runtime.CompilerServices/Unsafe.xml

• Line 0, Column 0: [Warning: ECMA2Yaml_MemberNameAndSignature_NotUnique] Member InitBlockUnaligned's name and signature is not unique

For more details, please refer to the build report.

If you see build warnings/errors with permission issues, it might be due to single sign-on (SSO) enabled on Microsoft's GitHub organizations. Please follow instructions here to re-authorize your GitHub account to Docs Build.

Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.

Note: Your PR may contain errors or warnings unrelated to the files you changed. This happens when external dependencies like GitHub alias, Microsoft alias, cross repo links are updated. Please use these instructions to resolve them.

For any questions, please:

• Try searching the docs.microsoft.com contributor guides
• Post your question in the Docs support channel

[gewarren]

Should we add a link for the references to ECMA 335? https://www.ecma-international.org/publications-and-standards/standards/ecma-335/

[gewarren]

Suggested change

	<returns>A new managed pointer that reflects the addition of offset to the source pointer.</returns>
	<returns>A new managed pointer that reflects the addition of the specified offset to the source pointer.</returns>

[AaronRobinsonMSFT]

The return here is a ref, so interior pointer? Unsure the language to use to differentiate the ref T and T.

[GrabYourPitchforks]

The pattern I've tried to follow throughout the doc is that I reserve reference for instances where the API shape involves T* or T& and where the operation involves reading or writing a full T at that address. You'll see this in APIs like Copy.

For APIs that perform arithmetic or comparisons against the pointer value rather than dereference it, I use [un]managed pointer, as this terminology matches the ECMA sections listed in the type's Summary section.

For APIs like ReadUnaligned, I take a little bit of license here. Nominally the overloads would've been ReadUnaligned<T>(void*) and ReadUnaligned<T>(void&), which .NET fully supports but which C# disallows. In both of those cases they're taking void pointers, but they're not dereferencing a void. They're instead performing a read or write of some differently-sized data at that address. Since they're not dereferencing a void I opted to use the terminology [un]managed pointer here rather than reference, since IMO it's more accurate.

[gewarren]

Suggested change

	This method is typically used with narrow character strings, such as ANSI or UTF-8 strings. The returned span does not include the `null` terminator, nor does it validate the well-formedness of the narrow character data.
	This method is typically used with narrow character strings, such as ANSI or UTF-8 strings. The returned span does not include the `null` terminator, nor does it validate whether the narrow character data is well-formed.

[gewarren]

Might be better for localization.

[GrabYourPitchforks]

I reworded this sentence entirely. Hopefully it's more clear now. :)

[gfoidl]

Suggested change

	> Most APIs on this type do not perform any argument checking or validation. Incorrect use of these APIs could destabilize the .NET runtime.
	> Most APIs on this type do not perform any argument checking or validation. Incorrect use of these APIs could lead to access violations, GC holes or destabilize the .NET runtime.

Came into my mind when reading this.
In essence it's similar, but to put some emphasis on the AV, and GC holes.

[GrabYourPitchforks]

Wanted to keep the Warnings section somewhat understandable by all devs, so I'll move the mention of GC holes later in the remarks section.

[tannergooding]

I'd argue that if the a dev doesn't understand access violation or gc hole then they shouldn't be using Unsafe.

[gfoidl]

Suggested change

	The `byteOffset` parameter is the number of bytes to add to the `source` pointer. For example, given a source pointer _ptr_ of type `ref int`, the call `Unsafe.AddByteOffset<int>(ref ptr, 20)` will return a new pointer whose address points 20 bytes beyond _ptr_.
	The `byteOffset` parameter is the number of bytes to add to the managed `source` pointer. For example, given a managed source pointer _ptr_ of type `ref int`, the call `Unsafe.AddByteOffset<int>(ref ptr, 20)` will return a new managed pointer whose address points 20 bytes beyond _ptr_.

Is this overkill to add managed to pointer?
To distinguish between native- / raw-pointers (void*) and managed pointers (ref T) here?
Out of the context it's very clear what is meant, and the APIs are targeting advanced users, so this shouldn't be needed.
But as docs they should be very precise, even for people that don't know these differences and to avoid potential confusion.

This would apply to other places as well.

[AaronRobinsonMSFT]

The return here is a ref, so interior pointer? Unsure the language to use to differentiate the ref T and T.

[tannergooding]

if we're touching this anyways, can we order this "properly":

<summary></summary>
<typeparam name=""></typeparam>
<param name=""></param>
<returns></returns>
<remarks></remarks>

[tannergooding]

(Noting "properly" is defined here as the order VS declares these by default when you type /// and matches a good reading order for understanding the method)

[GrabYourPitchforks]

@gewarren Thoughts on this? I'm looking through a small sample of other .xml files in the repo, and the consistent pattern I see is "typeparam, param, summary, returns, remarks." If we wanted to reorder this to be more proper I wonder if that should be something that's done repo-wide rather than as a one-off inside this file.

[gewarren]

Yes, I don't think it's worthwhile to reorder the tags. Also, I have a suspicion that mdoc will just reorder them the next time we update the APIs.

[tannergooding]

Is it worth a tracking issue for mdoc to be updated? It's definitely much harder to read/review these files because of the difference

[gewarren]

Yes - I logged https://dev.azure.com/ceapex/Engineering/_workitems/edit/619653.

[tannergooding]

not changes to Unbox?

[GrabYourPitchforks]

We recently updated the Unbox docs and I couldn't think of anything new to say. If you have suggestions I can definitely fold them in to this PR!

[GrabYourPitchforks]

@gewarren Any idea why the validation system is producing this warning?

Member InitBlockUnaligned's name and signature is not unique

Nothing really jumps out at me. It seems to follow the pattern for CopyBlockUnaligned closely, and that method isn't generating warnings.

[gewarren]

@gewarren Any idea why the validation system is producing this warning?

Member InitBlockUnaligned's name and signature is not unique

Nothing really jumps out at me. It seems to follow the pattern for CopyBlockUnaligned closely, and that method isn't generating warnings.

It should go away the next time the build completes. I believe it was due to your copy/paste error - #8097 (comment).

[opbld33]

Docs Build status updates of commit 28630d1:

⚠️ Validation status: warnings

File	Status	Preview URL	Details
xml/System.Runtime.CompilerServices/Unsafe.xml	⚠️Warning	View	Details
xml/System.Runtime.InteropServices/MemoryMarshal.xml	✅Succeeded	View

xml/System.Runtime.CompilerServices/Unsafe.xml

• Line 0, Column 0: [Warning: xref-not-found] Cross reference not found: 'System.Buffer.MemoryCopy'.
• Line 0, Column 0: [Warning: xref-not-found] Cross reference not found: 'System.Buffer.MemoryCopy'.
• Line 0, Column 0: [Warning: xref-not-found] Cross reference not found: 'System.Buffer.MemoryCopy'.
• Line 0, Column 0: [Warning: xref-not-found] Cross reference not found: 'System.Buffer.MemoryCopy'.

For more details, please refer to the build report.

If you see build warnings/errors with permission issues, it might be due to single sign-on (SSO) enabled on Microsoft's GitHub organizations. Please follow instructions here to re-authorize your GitHub account to Docs Build.

Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.

Note: Your PR may contain errors or warnings unrelated to the files you changed. This happens when external dependencies like GitHub alias, Microsoft alias, cross repo links are updated. Please use these instructions to resolve them.

For any questions, please:

• Try searching the docs.microsoft.com contributor guides
• Post your question in the Docs support channel

[GrabYourPitchforks]

@gewarren I think we're almost there! But the docs engine is complaining about the xref in this sentence:

Consider instead using <xref:System.Buffer.MemoryCopy> or <xref:System.Span%601.CopyTo%2A> for this scenario.

I'm trying to link to https://docs.microsoft.com/dotnet/api/system.buffer.memorycopy. Am I using the wrong syntax here?

[gewarren]

@gewarren I think we're almost there! But the docs engine is complaining about the xref in this sentence:

Consider instead using <xref:System.Buffer.MemoryCopy> or <xref:System.Span%601.CopyTo%2A> for this scenario.

I'm trying to link to https://docs.microsoft.com/dotnet/api/system.buffer.memorycopy. Am I using the wrong syntax here?

Yes, since there's more than one overload, you need to add the asterisk, so <xref:System.Buffer.MemoryCopy%2A>.

[opbld30]

Docs Build status updates of commit 94a22a1:

✅ Validation status: passed

File	Status	Preview URL	Details
xml/System.Runtime.CompilerServices/Unsafe.xml	✅Succeeded	View
xml/System.Runtime.InteropServices/MemoryMarshal.xml	✅Succeeded	View

For more details, please refer to the build report.

Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.

For any questions, please:

• Try searching the docs.microsoft.com contributor guides
• Post your question in the Docs support channel

[GrabYourPitchforks]

Thanks kindly @gewarren @tannergooding @AaronRobinsonMSFT @gfoidl for your input! 🥳

I'm going to merge this shortly because I think all of the high-priority feedback has been addressed. We can send smaller PRs if we find nits later on.