[Guile-commits] 01/01: Reimplement SCM_MAKE_CHAR to evaluate its argument only once.

[Mark H. Weaver]

mhw pushed a commit to branch stable-2.2
in repository guile.

commit 63818453ad226cd3c2d1fd8ade12e3d7c1d43c05
Author: Mark H Weaver <address@hidden>
Date:   Tue Apr 16 23:11:42 2019 -0400

    Reimplement SCM_MAKE_CHAR to evaluate its argument only once.
    
    The motivation for this change is that SCM_MAKE_CHAR is sometimes passed
    an expression that involves a procedure call that is not always trivial.
    In other cases, the results are not guaranteed to be the same both
    times, which could lead to the creation of invalid SCM objects.
    
    * libguile/chars.h (SCM_MAKE_CHAR): Reimplement.
---
 libguile/chars.h | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/libguile/chars.h b/libguile/chars.h
index 488dd25..505a7f9 100644
--- a/libguile/chars.h
+++ b/libguile/chars.h
@@ -3,7 +3,8 @@
 #ifndef SCM_CHARS_H
 #define SCM_CHARS_H
 
-/* Copyright (C) 1995,1996,2000,2001,2004, 2006, 2008, 2009 Free Software 
Foundation, Inc.
+/* Copyright (C) 1995, 1996, 2000, 2001, 2004, 2006, 2008, 2009, 2019
+ *   Free Software Foundation, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public License
@@ -36,16 +37,18 @@ typedef scm_t_int32 scm_t_wchar;
 #define SCM_CHARP(x) (SCM_ITAG8(x) == scm_tc8_char)
 #define SCM_CHAR(x) ((scm_t_wchar)SCM_ITAG8_DATA(x))
 
-/* SCM_MAKE_CHAR maps signed chars (-128 to 127) and unsigned chars (0
-   to 255) to Latin-1 codepoints (0 to 255) while allowing higher
+/* SCM_MAKE_CHAR maps signed chars (-128 to 127) and unsigned chars
+   (0 to 255) to Latin-1 codepoints (0 to 255) while allowing higher
    codepoints (256 to 1114111) to pass through unchanged.
 
-   This macro evaluates x twice, which may lead to side effects if not
-   used properly. */
-#define SCM_MAKE_CHAR(x)                                                \
-  ((x) <= 1                                                             \
-   ? SCM_MAKE_ITAG8 ((scm_t_bits) (unsigned char) (x), scm_tc8_char)    \
-   : SCM_MAKE_ITAG8 ((scm_t_bits) (x), scm_tc8_char))
+   To avoid evaluating X more than once, we use an arithmetic trick: we
+   compute (X mod 2^N) mod (2^N - 256), which is equal to the required
+   mapping in the range -256 .. (2^N - 257).  Here, N is the number of
+   bits in scm_t_bits.  Note that (scm_t_bits) (x) implicitly computes
+   (X mod 2^N), and (scm_t_bits) -256 equals (2^N - 256).  GCC is able
+   to optimize away these operations in practice.  */
+#define SCM_MAKE_CHAR(x) \
+  (SCM_MAKE_ITAG8 ((scm_t_bits) (x) % (scm_t_bits) -256, scm_tc8_char))
 
 #define SCM_CODEPOINT_DOTTED_CIRCLE (0x25cc)
 #define SCM_CODEPOINT_SURROGATE_START (0xd800)