Annotate all runtime functions with HALIDE_MUST_USE_RESULT #6388
Conversation
All the halide runtime functions (and function pointers) that return interesting status results (int or bool) should be annotated with HALIDE_MUST_USE_RESULT (we have evidence that ignoring return codes has led to hard-to-diagnose errors for downstream users). Also update all the internal call sites to report and/or ignore errors as appropriate.
|
Things like halide_do_par_for can't actually return non-zero in these tests. They only return non-zero values if the user supplies a halide_error that returns. It's a bit weird to require checking a return value that's always zero for most users. |
~All interactive uses of Halide pretty much require this, so I'd argue it's not uncommon at all. (The default halide_error() that simply aborts is useful for learning and testing and not much else.) |
|
I think this will make it substantially more annoying for people learning, experimenting, or iterating on Halide code. Personally, I'm not a fan of this class of compiler enforced hygiene ( |
This cherry-picks just the changes to callsites internal to Halide (and tests) from #6388. (It doesn't attempt to annotate runtime functions to enforce checking the results.)
|
Based on comments received here and via other forums, I'm going to drop this PR rather than try to argue in favor of it more, because it seems unlikely that we'll get to consensus on it. For the record, I feel very strongly that this is the wrong decision, and that making it easy to ignore error conditions is a bad design decision that will come back to bite us in the future. |
* Check results of all runtime function calls This cherry-picks just the changes to callsites internal to Halide (and tests) from #6388. (It doesn't attempt to annotate runtime functions to enforce checking the results.) * Update write_debug_image.cpp * Add checks + comment to buffer_copy_aottest * Add comment to gpu_object_lifetime_aottest * Update memory_profiler_mandelbrot_aottest.cpp * Update user_context_insanity_aottest.cpp * Update process.cpp
All the halide runtime functions (and function pointers) that return interesting status results (int or bool) should be annotated with HALIDE_MUST_USE_RESULT (we have evidence that ignoring return codes has led to hard-to-diagnose errors for downstream users). Also update all the internal call sites to report and/or ignore errors as appropriate.