On Thu, 2023-12-07 at 17:34 -0500, Antoni Boucher wrote: > Hi. > This patch adds checks gcc_jit_block_add_assignment_op to make sure > it > is only ever called on numeric types. > > With the previous patch, this might require a change to also allow > vector types here. > > Thanks for the review.
Thanks for the patch. [...snip...] > @@ -2890,6 +2900,17 @@ gcc_jit_block_add_assignment_op (gcc_jit_block *block, > lvalue->get_type ()->get_debug_string (), > rvalue->get_debug_string (), > rvalue->get_type ()->get_debug_string ()); > + // TODO: check if it is a numeric vector? > + RETURN_IF_FAIL_PRINTF3 ( > + lvalue->get_type ()->is_numeric () && rvalue->get_type ()->is_numeric > (), ctxt, loc, > + "gcc_jit_block_add_assignment_op %s has non-numeric lvalue %s (type: > %s)", > + gcc::jit::binary_op_reproducer_strings[op], > + lvalue->get_debug_string (), lvalue->get_type ()->get_debug_string ()); The condition being tested here should probably just be: lvalue->get_type ()->is_numeric () since otherwise if the lvalue's type is numeric and the rvalue's type fails to be, then the user would incorrectly get a message about the lvalue. > + RETURN_IF_FAIL_PRINTF3 ( > + rvalue->get_type ()->is_numeric () && rvalue->get_type ()->is_numeric > (), ctxt, loc, > + "gcc_jit_block_add_assignment_op %s has non-numeric rvalue %s (type: > %s)", > + gcc::jit::binary_op_reproducer_strings[op], > + rvalue->get_debug_string (), rvalue->get_type ()->get_debug_string ()); The condition being tested here seems to have a redundant repeated: && rvalue->get_type ()->is_numeric () Am I missing something, or is that a typo? [...snip...] The patch is OK otherwise. Thanks Dave
