Re: Excessive use of `eassert`

[Stefan Monnier]

BTW, gcc refuses to compile the current code on my i386 machine.
It gives me loads of things like:

    lisp.h:620:28: error: expected expression before ‘{’ token
      620 | # define LISP_INITIALLY(w) {w}
          |                            ^
    lisp.h:941:3: note: in expansion of macro ‘LISP_INITIALLY’
      941 |   LISP_INITIALLY ((Lisp_Word) ((uintptr_t) (ptr) + LISP_WORD_TAG 
(tag)))
          |   ^~~~~~~~~~~~~~
    lisp.h:415:3: note: in expansion of macro ‘TAG_PTR’
      415 |   TAG_PTR (Lisp_Symbol, (index) * sizeof *lispsym)
          |   ^~~~~~~
    lisp.h:482:37: note: in expansion of macro ‘lisp_h_builtin_lisp_symbol’
      482 | # define builtin_lisp_symbol(index) lisp_h_builtin_lisp_symbol 
(index)
          |                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~
    ./globals.h:7597:14: note: in expansion of macro ‘builtin_lisp_symbol’
     7597 | #define Qnil builtin_lisp_symbol (0)
          |              ^~~~~~~~~~~~~~~~~~~
    dispnew.c:6110:39: note: in expansion of macro ‘Qnil’
     6110 |       file = Fexpand_file_name (file, Qnil);
          |                                       ^~~~

How 'bout the patch below.  It gives the expected optimization under
`-O2` and `-Og` and avoids duplicating the logic.  It doesn't give the
expected optimization under `-O0` because `-O0` doesn't do any inlining,
but we've lived with that for a while so maybe that's OK (admittedly,
I'm biased since I almost never use `-O0`).


        Stefan


diff --git a/src/lisp.h b/src/lisp.h
index 54d2f4d3dd1..74e4a78ae26 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -409,11 +409,7 @@ #define lisp_h_FIXNUMP(x) \
        & ((1 << INTTYPEBITS) - 1)))
 #define lisp_h_FLOATP(x) TAGGEDP (x, Lisp_Float)
 #define lisp_h_NILP(x)  BASE_EQ (x, Qnil)
-/* Equivalent to "make_lisp_symbol (&lispsym[INDEX])",
-   and typically faster when compiling without optimization.  */
-#define lisp_h_builtin_lisp_symbol(index) \
-  TAG_PTR (Lisp_Symbol, (index) * sizeof *lispsym)
-#define lisp_h_SET_SYMBOL_VAL(sym, v) \
+#define lisp_h_SET_SYMBOL_VAL(sym, v)                 \
    (eassert ((sym)->u.s.redirect == SYMBOL_PLAINVAL), \
     (sym)->u.s.val.value = (v))
 #define lisp_h_SYMBOL_CONSTANT_P(sym) \
@@ -479,7 +475,6 @@ #define lisp_h_XHASH(a) XUFIXNUM_RAW (a)
 # define FLOATP(x) lisp_h_FLOATP (x)
 # define FIXNUMP(x) lisp_h_FIXNUMP (x)
 # define NILP(x) lisp_h_NILP (x)
-# define builtin_lisp_symbol(index) lisp_h_builtin_lisp_symbol (index)
 # define SET_SYMBOL_VAL(sym, v) lisp_h_SET_SYMBOL_VAL (sym, v)
 # define SYMBOL_CONSTANT_P(sym) lisp_h_SYMBOL_CONSTANT_P (sym)
 # define SYMBOL_TRAPPED_WRITE_P(sym) lisp_h_SYMBOL_TRAPPED_WRITE_P (sym)
@@ -1169,13 +1164,24 @@ XSYMBOL (Lisp_Object a)
   return XBARE_SYMBOL (a);
 }
 
+/* Apparently the `eassert` in `make_lisp_symbol` makes it non-inlinable
+   for GCC under -Og and even -O2, so use a non-checking version
+   for builtin symbols, so "apparent constants" like `Qnil` are indeed
+   compiled as constants.  */
 INLINE Lisp_Object
-make_lisp_symbol (struct Lisp_Symbol *sym)
+make__lisp_symbol (struct Lisp_Symbol *sym)
 {
   /* GCC 7 x86-64 generates faster code if lispsym is
      cast to char * rather than to intptr_t.  */
   char *symoffset = (char *) ((char *) sym - (char *) lispsym);
   Lisp_Object a = TAG_PTR (Lisp_Symbol, symoffset);
+  return a;
+}
+
+INLINE Lisp_Object
+make_lisp_symbol (struct Lisp_Symbol *sym)
+{
+  Lisp_Object a = make__lisp_symbol (sym);
   eassert (XBARE_SYMBOL (a) == sym);
   return a;
 }
@@ -1183,7 +1189,7 @@ make_lisp_symbol (struct Lisp_Symbol *sym)
 INLINE Lisp_Object
 (builtin_lisp_symbol) (int index)
 {
-  return lisp_h_builtin_lisp_symbol (index);
+  return make__lisp_symbol (&lispsym[index]);
 }
 
 INLINE bool