-
Notifications
You must be signed in to change notification settings - Fork 5
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
Declare OpenCL functions that can lead to undefined behaviour as unsafe #26
Comments
Honestly I'm not a huge user of OpenCL so I don't think I can audit the entire crate. I was porting existing code that made some use of unsafe for performance. Off the top of my head:
|
Thank you @awused. Your answer and @vmx ‘s comment from kenba/opencl3#51:
have made me reconsider this issue. To my mind Rust’s compile time memory safety guarantee is one of the greatest features of the language IMHO, but it is incompatible with a library like Your point about “running a kernel” @awused is particularly relevant because that’s the whole purpose of I want this library and I consider the most appropriate solution to this issue is to add warnings to the descriptions of this library and |
As I said in the other bug, libraries tend towards either making everything unsafe and doing no validation at all or exposing a safe interface with some unsafe escape hatches. OpenCL may be inherently unsafe but that doesn't mean safe abstractions can't be built over it for the vast majority of use cases. Running a kernel involves running C code, so that is necessarily unsafe, but I don't think there's any getting around that and having that isn't likely to prevent people from using the library. But it is possible to reduce unsafe it to the point where running a kernel is the only unsafe needed most of the time. I hesitate far more to use libraries that mark everything as safe out of a desire to be popular or "friendly" when they're unsafe rather than those that just mark everything as unsafe and firmly put the onus on the user to use it properly. Since you asked for my opinion I do think that's the wrong way to go about it. |
The changes to this package, at least, look fine at first glance, though I've not gone through them in detail. While I did experiment with |
I also only had a quick look (and I also still lack quite a bit of OpenCL knowledge), but what changed looks good to me. Thanks @awused for spending so much time on making this library better. |
@vmx and @awused I've now published these and the |
As per @awused comments in kenba/opencl3#51 and kenba/opencl3#52, all OpenCL functions that can lead to undefined behaviour should be declared
unsafe
.This issue is to capture and change all
cl3
functions that should be declaredunsafe
.The text was updated successfully, but these errors were encountered: