GNU bug report logs - #45550
[PATCH] Factor out new function for readability in chartab.c

Previous Next

Full log

View this message in rfc822 format

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 45550 <at> debbugs.gnu.org
Subject: bug#45550: [PATCH] Factor out new function for readability in chartab.c
Date: Wed, 30 Dec 2020 22:15:14 +0200

> From: Stefan Kangas <stefan <at> marxist.se>
> 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.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.