[CallerMustBeUnsafe] type attribute

[srivatsn]

Ported from dotnet/roslyn#8663

---

From https://github.com/dotnet/coreclr/issues/3143

To mark a function to be only be able to be called in an unsafe block.

It came up as an issue with having an IntPtr based api for Vector.Copy be equally could apply to something like a .ctor where you are passing an internal buffer to use (e.g. https://github.com/dotnet/coreclr/issues/3142)

Or risk of use of buffers with overlapped I/O tasks and dispose dotnet/corefx#5954 (comment)

To indicate that the caller is aware there are risks and to be careful. What I am suggesting is something where .ctor 2 is forced to be unsafe in the same way .ctor 3 is:

public BufferedThing(int bufferSize){}

[CallerMustBeUnsafe]
public BufferedThing(byte[] internalBuffer){}

public BufferedThing(byte* internalBuffer, int bufferLength){}

e.g.

var buffer0 = new BufferedThing(10); // fine

var buffer1 = new BufferedThing(new byte[10]); // compile error

unsafe {
    var buffer2 = new BufferedThing(new byte[10]); // fine
}

unsafe {
    var buffer = new byte[10];
    fixed (byte* pBuffer = &buffer[0]) {
        var buffer3 = new BufferedThing(pBuffer, 10); // fine
    }
}

[dasMulli]

As mentioned in https://github.com/dotnet/corefx/issues/42251#issuecomment-553310965, I don't think mixing language concepts (unsafe keyword) with API level concepts ("unsafe operations" / "unsafe API") is a very good idea.

My primary reason is that it is just asking users to change a setting in their project file and add an unsafe block. This doesn't necessarily make users think about how the API should be used (esp. when there is a StackOverflow answer telling developers how to work around the produced diagnostic).

Truth is there are classes of APIs that are more prone to produce runtime errors (some of which are unrecoverable) and more "safe" APIs. Denoting APIs with UnsafeXYZ or DangerousXYZ is probably a way that makes it more visible in the code base about what is actually a part that can go wrong instead of marking a method with unsafe and then not knowing which part of it is actually "dangerous".

Applied to the API mentioned in the above example, I'd suggest a BufferedThing.DangerousCreateFromInternalBufferArray(byte[]) signature to make a dangerous / unsafe operation visible to consumers and during code reviews.

[sharwell]

I would rather focus energy on analyzers to help users correctly use these APIs. The attribute doesn't seem to add clarity to the code, but rather confuses concepts of pointers with other code and raises a bunch of questions about where the attribute should and should not be applied.