Fix confusing sentence "without assuming architecture dependent alignment of the addresses" (#5342)

[RamType0]

Fixes #5342

[opbld30]

Docs Build status updates of commit b4443d7:

🕙 Pending: waiting for processors (17 builds ahead of you)

⚠️ Docs Build is busy, currently there are 17 builds ahead of this one, for more information you can view the Build queue graph on the Docs Portal.

[dnfadmin]

All CLA requirements met.

[opbld31]

Docs Build status updates of commit b4443d7:

✅ Validation status: passed

File	Status	Preview URL	Details
xml/System.Runtime.CompilerServices/Unsafe.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 in the Docs contributor and Admin Guide
• See the frequently asked questions
• Post your question in the Docs support channel

[GrabYourPitchforks]

I don't think the suggested corrections add clarity. If we wanted to clarify matters, I think this phrasing flows better:

Writes a value of type T to the given location. This write will succeed even if the destination address is not properly aligned for values of type T.

[RamType0]

I don't think the suggested corrections add clarity. If we wanted to clarify matters, I think this phrasing flows better:

Writes a value of type T to the given location. This write will succeed even if the destination address is not properly aligned for values of type T.

Do you mean that "succeed" is more clarity than "correctly" or "not properly aligned" is clarity than "unaligned" ?
I don't think that these are better, but I don't care about replacing these sentence.

Adding "for values of type T" sounds good.

I care about preserving original sentence's replacabillity.
Any of "unaligned" variant's summary was created by just adding "without assuming architecture dependent addresses" to "aligned" method's summary.

In my correction, any of "unaligned" variant's summary was created by just adding "correctly even while we use the platform which requires alignment and (any of) the given location is unaligned" to "aligned" method's summary.

At least, any of new correction(mine or yours) is far much clarity than current document.(IMHO)

[danmoseley]

@RamType0 I believe we should use @GrabYourPitchforks suggestion. The change as is I find more confusing.

[RamType0]

@RamType0 I believe we should use @GrabYourPitchforks suggestion. The change as is I find more confusing.

I think his phrase is OK, but I don't know which phrase of mine making it confusing.
I should know what making it confusing for fixing it correctly.

[RamType0]

Do you think my phrase is more confusing than current?

IMHO, @GrabYourPitchforks's phrase is greater than current, but it is not greater than mine, and not lesser than mine.
And both mine and @GrabYourPitchforks's phrase is far much greater than current.

Is there any non-native English speaker in reviewer?
I'm not a native English speaker, but the document should be easy to understand for both native English speakers and non-native English speakers, ideally.

[GrabYourPitchforks]

I should know what making it confusing for fixing it correctly.

Sure, let's break it down. I've added [ ] brackets around your text to help group constructs.

[Copies bytes from the source address to the destination address correctly] [even while we use the platform which requires alignment] [and any of the given location is unaligned].

The sentence communicates three separate ideas, as bracketed above. For improved readability, it's preferred to separate out the most important idea. Additional sentences can communicate extra information.

This guideline suggests that the first idea should be its own sentence: "Copies bytes from the source address to the destination address correctly." However, it's awkward to describe a method as performing some operation "correctly." Most developers would expect .NET APIs to behave correctly. Let's keep that in mind for now and move on to the next part.

The next two ideas communicate that: (a) some platforms require alignment; and (b) there may be memory locations which are unaligned. These statements feel redundant. If a value is at an unaligned memory location, then by definition, the value's location violates the platform's normal alignment assumptions. This implies that we can eliminate the middle idea ("even while...").

Now we come back to the "correctly" aspect from earlier. The important part of the "correctly" statement isn't that the method behaves correctly. (Developers would naturally expect this.) Instead, the important part is that the method behaves correctly even when the values are not aligned. That's what makes these methods unique compared to standard read / write / copy methods. This implies that the "correctly" word should be joined to the idea that some memory locations are unaligned. Since the first sentence of the text already communicates the core idea that the method reads / writes / copies bytes, we should move the "correctly" word to the second sentence.

With these changes, we end up with (paraphrased):

Copies bytes from the source address to the destination address. Works correctly if any of the given location is unaligned.

The rest is technical cleanup (using more accurate terminology), tailoring the wording for the target audience, and grammatical corrections.

Hope this helps explain things!

[RamType0]

@GrabYourPitchforks Thanks for great description!

BTW, I heard it is ensured that primitive types with size lesser than 4bytes or pointer types are always read or written atomically.
Is this also applied for Unsafe.ReadUnaligned and Unsafe.WriteUnaligned?(I don't think so)
It should be also documented.

[GrabYourPitchforks]

BTW, I heard it is ensured that primitive types with size lesser than 4bytes or pointer types are always read or written atomically. Is this also applied for Unsafe.ReadUnaligned and Unsafe.WriteUnaligned?(I don't think so)

This depends on the processor. For x86 / x64 processors, if the integral value is appropriately aligned for the target address, the read / write is guaranteed atomic.

It should be also documented.

This is not necessary. The APIs discussed in this issue deal only with alignment concerns, not atomicity. The target developer for this API is somebody who needs low-level access to memory. It should be assumed such a person is intimately familiar with memory ordering models, platform alignment requirements, and similar concepts. We don't need to describe every guarantee or non-guarantee provided by these APIs beyond those in the API's immediate purview.

For an example of API documentation which does describe atomicity guarantees, see System.Threading.Interlocked.

[opbld30]

Docs Build status updates of commit 2d7191a:

❌ Validation status: errors

Please follow instructions here which may help to resolve issue.

File	Status	Preview URL	Details
xml/System.Runtime.CompilerServices/Unsafe.xml	❌Error		Details
	❌Error		Details

xml/System.Runtime.CompilerServices/Unsafe.xml

• Line 0, Column 0: [Error-ECMA2Yaml_InternalError]

Intenal Several Error: System.Xml.XmlException: The '%' character, hexadecimal value 0x25, cannot be included in a name. Line 594, position 144.
   at System.Xml.XmlTextReaderImpl.Throw(Exception e)
   at System.Xml.XmlTextReaderImpl.Throw(String res, String[] args)
   at System.Xml.XmlTextReaderImpl.ParseElement()
   at System.Xml.XmlTextReaderImpl.ParseElementContent()
   at System.Xml.Linq.XContainer.ReadContentFrom(XmlReader r)
   at System.Xml.Linq.XDocument.Load(XmlReader reader, LoadOptions options)
   at System.Xml.Linq.XDocument.Parse(String text, LoadOptions options)
   at ECMA2Yaml.ECMALoader.LoadType(FileItem typeFile)
   at ECMA2Yaml.ECMALoader.LoadTypes(String basePath, Namespace ns)

• Line 0, Column 0: [Error-ECMA2Yaml_File_LoadFailed] Failed to load 1 files, aborting...

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

[opbld34]

Docs Build status updates of commit 829715d:

❌ Validation status: errors

Please follow instructions here which may help to resolve issue.

File	Status	Preview URL	Details
xml/System.Runtime.CompilerServices/Unsafe.xml	❌Error		Details
	❌Error		Details

xml/System.Runtime.CompilerServices/Unsafe.xml

• Line 0, Column 0: [Error-ECMA2Yaml_InternalError]

Intenal Several Error: System.Xml.XmlException: 'xref' is an undeclared prefix. Line 594, position 91.
   at System.Xml.XmlTextReaderImpl.Throw(Exception e)
   at System.Xml.XmlTextReaderImpl.LookupNamespace(NodeData node)
   at System.Xml.XmlTextReaderImpl.ElementNamespaceLookup()
   at System.Xml.XmlTextReaderImpl.ParseElement()
   at System.Xml.XmlTextReaderImpl.ParseElementContent()
   at System.Xml.Linq.XContainer.ReadContentFrom(XmlReader r)
   at System.Xml.Linq.XDocument.Load(XmlReader reader, LoadOptions options)
   at System.Xml.Linq.XDocument.Parse(String text, LoadOptions options)
   at ECMA2Yaml.ECMALoader.LoadType(FileItem typeFile)
   at ECMA2Yaml.ECMALoader.LoadTypes(String basePath, Namespace ns)

• Line 0, Column 0: [Error-ECMA2Yaml_File_LoadFailed] Failed to load 1 files, aborting...

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

[RamType0]

I've fixed some sentences with following your instructions.

But I've felt some other missing description in document.

• What is unaligned address for Unsafe.InitBlock or Unsafe.CopyBlock?
  IMHO, pointer is always aligned for values for type byte.
  I also referenced OpCodes.Cpblk, but it just says "the natural size of the machine". I didn't understand it.(Is it means the pointer size?)

• Is it not necessary that Unsafe.CopyBlock doesn't support overlapping copy?
  AFIK,Unsafe.CopyBlock is just a wrapper of cpblk IL instruction, and it doesn't supports overlapping copy(Some regions of source and destination is overlapping).
  But it is not documented.

[RamType0]

Sorry for my bad knowledge, how can I create a link in summary?

Try searching the docs.microsoft.com contributor guides

It is forbidden

Post your question in the Docs support channel

Error occurred while starting Teams web app with following information.

Request Id: 2451e15b-3537-425d-af87-c122e9ba5701
Correlation Id: 3b93a12f-118a-498b-ad08-2098aaaf6118
Timestamp: 2021-03-27T15:56:52Z
Message: AADSTS90002: Tenant '72f98bf-86f1-41af-91ab-2d7cd011db47' not found. This may happen if there are no active subscriptions for the tenant. Check to make sure you have the correct tenant ID. Check with your subscription administrator.

[danmoseley]

@gewarren ?

[gewarren]

Sorry for my bad knowledge, how can I create a link in summary?

The xref style links are for markdown sections only. For XML content, like in the summary, use <see cref> links. I left a suggestion that you can follow for the other summaries.

Also, apologies about those internal help links. I've filed a suggestion that they also include a link to the public contributor's guide in those PR comments. (Although that guide doesn't seem to include any API ref-specific guidance.)

[GrabYourPitchforks]

Unsafe.CopyBlock is just a wrapper of cpblk IL instruction, and it doesn't supports overlapping copy(Some regions of source and destination is overlapping).
But it is not documented.

This is clearly documented. From ECMA-335, III.3.30: "The behavior of cpblk is unspecified if the source and destination areas overlap."

We don't need to mirror the entirety of the ECMA text into our docs here. These APIs are meant for developers who need to emit specific IL instructions into their apps. It's reasonable to expect callers to have read the appropriate specs.

Honestly I think we're starting to go down a rabbit hole here. In an ideal world we'd have all docs easily approachable and understandable by a wide developer audience, but these are not APIs for a broad audience. There is intentionally some amount of gatekeeping in this API surface. We hide these APIs (and name them "unsafe"!) for good reason.

[RamType0]

@GrabYourPitchforks How about just adding link to OpCodes.Cpblk at "See also"?

[learn-build-service-prod]

Learn Build status updates of commit 9c3e312:

❌ Validation status: errors

Please follow instructions here which may help to resolve issue.

File	Status	Preview URL	Details
	❌Error		Details

• [Error: CannotMergeCommit] Cannot merge commit 9c3e31219d14757a88b470dc0c05e28f8558e382 in branch FixConfusingWithoutAssumingAlignment of repository https://github.com/RamType0/dotnet-api-docs into branch main (commit e4e49385568e72204e1f8f6aedbdbb7cd6ce5090). Please follow this documentation: https://help.github.com/articles/resolving-a-merge-conflict-using-the-command-line/ to use git.exe to resolve you content conflicts locally and then push to remote.

For more details, please refer to the build report.

Note: Your PR may contain errors or warnings or suggestions 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 learn.microsoft.com contributor guides
• Post your question in the Learn support channel

[gewarren]

Thank you @RamType0!

Was just looking at one commit instead of all changes. The changes need to be applied to across the PR.