Avoid throw/catch exceptions when requesting optional buffer through IBufferProtocol

[BCSharp]

It turns out that GetBufferNoThrow is quite popular. However, it entailed a try/catch of BufferException, which incurs a performance penalty. In CPython, it turns out it is fairly common to try to get a buffer that is not supported , and then fall back on some more supported versions, since the error is inexpensive to return. However, it requires checking every return value, which is not convenient to the users of the interface. Besides, it is the provider, not the user, that can describe the error best.

This PR keeps the current of the usage interface the same, while keeping the complexity of implementing IBufferProtocol at a similar level, and avoiding the try/catch of the exception for unsupported calls.

[slozier]

throwOnError feels like kind of an odd pattern for a public API, but I couldn't think of anything much better.

An alternate pattern could be:

[return: NotNullIfNotNull(nameof(error))]
public IPythonBuffer? TryGetBuffer(BufferFlags flags, out string? error);

and then you could use it like this:

var buffer = TryGetBuffer(BufferFlags.Simple, out string? error) ?? throw PythonOps.BufferError(error);

[slozier]

Why a TypeError on this one?

[BCSharp]

Unintentional. VS Code trying to be helpful…

[BCSharp]

An alternate pattern could be:

Interesting, but frankly, this pattern looks to me even more unusual to as the one I proposed. Also, my intention was to put as little burden as possible on the implementers of the interface. So throwOnError is advisory, the implementer can ignore it and simply always return null on error to use a generic error message (without needing to initialize error to null). Like in "if you ain't got nothin' useful to say, then don't say nothin". Maybe I should clarify that in doc comments.

Also as I understand NotNullIfNotNull, it does the opposite than what is expected, doesn't it? It is either return is not null or the out is not null, not both at the same time. NullIfNotNull or NotNulIfNull would be more accurate, but I don't see it defined anywhere…

[BCSharp]

BTW, the proposed pattern is not so unusual, I knew I've seen it somewhere before:

public virtual Type? GetType (string name, bool throwOnError);

Anyway, I'm checking in some changes to the BufferProtocol public API so I am not merging the PR but give you the chance to review it again.

[slozier]

Interesting, but frankly, this pattern looks to me even more unusual to as the one I proposed. Also, my intention was to put as little burden as possible on the implementers of the interface. So throwOnError is advisory, the implementer can ignore it and simply always return null on error to use a generic error message (without needing to initialize error to null). Like in "if you ain't got nothin' useful to say, then don't say nothin". Maybe I should clarify that in doc comments.

BTW, the proposed pattern is not so unusual, I knew I've seen it somewhere before:

Well, the GetType seems a bit more clear cut in that the documentation makes it sound like you won't get null if throwOnError is true. Although it's not annotated as such so who knows...

Anyway, I guess the code contract can be whatever we make it and I don't have better names/patterns to propose (preferThrowOnError, maybeThrowOnError, throwOnErrorIfYouFeelLikeIt 😉).

Also as I understand NotNullIfNotNull, it does the opposite than what is expected, doesn't it? It is either return is not null or the out is not null, not both at the same time. NullIfNotNull or NotNulIfNull would be more accurate, but I don't see it defined anywhere…

Ugh yeah, I need more sleep. 😄