Subject: bug#45550: [PATCH] Factor out new function for readability in chartab.c

From: Eli Zaretskii <[email-masked]>
Date: Wed, 30 Dec 2020 22:15:14 +0200

> From: Stefan Kangas <[email-masked]>
> Date: Wed, 30 Dec 2020 02:56:53 -0600
> 
> While poking around, I found an opportunity for refactoring in
> charctab.c.  I find it makes these functions significantly easier to
> read and understand.
> 
> Please see the attached patch.

Thanks, I have a couple of comments:

> +static Lisp_Object
> +sub_char_table_ref_and_range (Lisp_Object, int, int *, int *,
> +			      Lisp_Object, bool);

There should be no need to declare a static function which is defined
before it is used.

> +static Lisp_Object
> +char_table_ref_simple (Lisp_Object table, int idx, int c, int *from, int *to,
> +		       Lisp_Object defalt, bool is_uniprop, bool is_subtable)
> +{
> +  Lisp_Object val = is_subtable ?
> +    XSUB_CHAR_TABLE (table)->contents[idx]:
> +    XCHAR_TABLE (table)->contents[idx];
> +  if (is_uniprop && UNIPROP_COMPRESSED_FORM_P (val))
> +    val = uniprop_table_uncompress (table, idx);
> +  if (SUB_CHAR_TABLE_P (val))
> +    val = sub_char_table_ref_and_range (val, c, from, to,
> +					defalt, is_uniprop);
> +  else if (NILP (val))
> +    val = defalt;
> +  return val;
> +}
> +

Please make this a macro, not a function.  The code you are factoring
out is called in the innermost loops of the display engine, in bidi.c,
so it must be as fast as possible, even in an unoptimized build, where
static functions aren't inlined.