Move null check from Printer to halide_string_to_string() #6467
Conversation
The Printer is (currently) usually inlined into every module, so this check is repeated in multiple chunks of code. Since the goal is to avoid crashing when debugging, let's move it to halide_string_to_string() (which will catch all these, and possibly more) and save some code size. (Further improvements in Printer code size on the way; this change seems worthy of considering separately.)
|
Windows failure appears to be a flake. |
|
Gentle review ping |
|
string_to_string is used in a lot of places where the arg is known to be non-null (e.g. when you use a print expression in the IR). If this is just about moving the check out of the inlined Printer, can that be handled in #6473 ? |
It was originally about that, yes, and yes it can be handled elsewhere, but (again) I'd argue that if we want to put in a guard against crashing due to rogue null string ptrs, this is the better place to do it. |
(I'm not 100% married to this change, just think it's worthy of consideration in the name of defensive coding.) |
|
I don't think anything performance-critical calls this, so sure why not in the name of defensive programming. |
The Printer is (currently) usually inlined into every module, so this check is repeated in multiple chunks of code. Since the goal is to avoid crashing when debugging, let's move it to halide_string_to_string() (which will catch all these, and possibly more) and save some code size.
(Further improvements in Printer code size on the way; this change seems worthy of considering separately.)