-
Notifications
You must be signed in to change notification settings - Fork 6k
Completing the primer section on unsafe #29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
* Added a code sample taken from CoreFx * Linked to the folder where the source file sits * Reworded blurb to emphasize that it should be discouraged unless necessary, not simply discouraged
LGTM. I was meaning to finish up the unsafe section together with the interoperability stuff, but I guess we can keep them separate as well. |
LGTM. The example is unsafe solely due to the APIs it calls and the one field on the struct. An additional and more expansive use of 'fixed' might be good. See the examples here: https://msdn.microsoft.com/en-us/library/f58wzh21.aspx. I suspect that there are some great examples in the BCL, maybe in something like StringBuilder. |
The rationale here is that people will want to see the use of unsafe code from an algorithmic standpoint more than a systems interop standpoint.
Updated this to use I would like to also highlight a code sample using some systems interop, but I also would like to keep this to just one code sample. Is there any source you know of that uses memory manipulation in an algorithm and does some interop? |
|
||
TODO: Examples. | ||
Taken from the `StringBuilder class in the .NET BCL reference source <http://referencesource.microsoft.com/#mscorlib/system/text/stringbuilder.cs,adf60ee46ebd299f>`_: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This link would probably be more relevant: https://github.com/dotnet/coreclr/blob/master/src/mscorlib/src/System/Text/StringBuilder.cs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
LGTM modulo feedback |
Changed the link. Should be good to merge now. |
|
||
TODO: Examples. | ||
Taken from the `StringBuilder class in the .NET BCL source <https://github.com/dotnet/coreclr/blob/master/src/mscorlib/src/System/Text/StringBuilder.cs#L327>`_: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels like this is missing a bit of an intro, really to clarify that this is an example of unsafe code use.
…-primer-sample Conflicts: docs/concepts/primer.rst
^^^^^^^^^^^ | ||
|
||
The CLR enables the ability to access native memory and do pointer | ||
arithmetic via ``unsafe`` code. These operations are needed for certain algorithms and system interoperability. Although powerful, use of unsafe code is discouraged unless it is necessary to interop with system APIs or implement the most efficient algorithm. Unsafe code may not predictably run in all environments, and also loses the benefits of a garbage collector and type safety. It's recommended to confine unsafe code as much as possible, and test that code thoroughly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still wondering what you mean by "may not predictably run in all environments".
Can you squash and merge? Thanks. |
I dun goofed and messed things up in my environment, preventing me from merging this in properly. Going to open a new PR with the squashed commits. |
Addressing #27.
necessary, not simply discouraged
/cc @blackdwarf @richlander @terrajobst