[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: inlinable functions instead of macros
From: |
Paul Eggert |
Subject: |
Re: inlinable functions instead of macros |
Date: |
Fri, 24 Aug 2012 14:37:21 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux i686; rv:14.0) Gecko/20120714 Thunderbird/14.0 |
On 08/22/2012 07:31 AM, Stefan Monnier wrote:
> It was working simply and effectively
It's simple to put easserts into macros, but as you know this has
problems that make the macros hard to use. They cannot be invoked
from GDB, which complicates debugging. More important, callers
sometimes cannot specify arguments with side effects, and this
restriction makes callers error-prone. We've tried to address some of
the side-effect issues with this hack:
/* The IDX==IDX tries to detect when the macro argument is side-effecting.
*/
#define ASET(ARRAY, IDX, VAL) \
(eassert ((IDX) == (IDX)), \
eassert ((IDX) >= 0 && (IDX) < ASIZE (ARRAY)), \
XVECTOR (ARRAY)->contents[IDX] = (VAL))
but even this hack doesn't catch the side-effect problem reliably,
and anyway it's better to not introduce further problems like this.
Here's how I'd like to address this issue:
1. Revert the setters and getters (e.g., bset_filename, BVAR)
and go back to ordinary field accesses, as described in
<http://bugs.gnu.org/12215#61>. This will simplify the problem
by reducing the number of functions/macros affected.
We're still working on the details here but it's clear that
most of these functions will go.
2. Revert the easserts recently added to Emacs that are contributing
to this problem. We went without these easserts for years, and
there's no particular reason to add them now if they cause problems.
3. Assuming (1) and (2), only two problematic easserts remain: in
blv_found and set_blv_found. Address them with the simple solution
that Sam Steingold proposed in
<http://lists.gnu.org/archive/html/emacs-devel/2012-08/msg00641.html>,
combined with an externally-visible wrapper for GDB's sake.
4. After 24.3, look into fixing the side-effect problem for ASET and
other macros too, as described in Bug#11935. That's a bigger
project, but it will have other benefits too.
Here's a patch to implement (2) and (3); it assumes the patch
mentioned in (1). This patch makes Emacs a tiny bit smaller with
the default build, so it should be OK on efficiency grounds.
=== modified file 'src/ChangeLog'
--- src/ChangeLog 2012-08-24 04:43:52 +0000
+++ src/ChangeLog 2012-08-24 21:24:45 +0000
@@ -1,5 +1,17 @@
2012-08-24 Paul Eggert <address@hidden>
+ Report better line numbers when assertions fail (Bug#12215).
+ * alloc.c, lisp.h (LISP_INLINE_EXTERNALLY_VISIBLE): New macro.
+ * buffer.h (buffer_intervals, set_buffer_intervals):
+ * lisp.h (gc_aset, vcopy): Remove recently-introduced easserts, as
+ they're not that useful in practice and they emit bogus line numbers.
+ * lisp.h (eassert, blv_found, set_blv_found):
+ Reimplement in terms of eassert_f, blv_found_f, set_blv_found_f.
+ (eassert_f, blv_found_f, set_blv_found_f): New macros or functions,
+ containing the guts of the old, but which let the caller specify
+ file and line number.
+ (blv_found, set_blv_found): Also define as a function, for GDB.
+
Remove setters like bset_keymap and getters like BVAR (Bug#12215).
They make the code harder to read, and for now they don't
bring anything to the table. Open-code them instead.
=== modified file 'src/alloc.c'
--- src/alloc.c 2012-08-23 06:49:09 +0000
+++ src/alloc.c 2012-08-24 21:24:45 +0000
@@ -21,6 +21,7 @@
#include <config.h>
#define LISP_INLINE EXTERN_INLINE
+#define LISP_INLINE_EXTERNALLY_VISIBLE EXTERN_INLINE EXTERNALLY_VISIBLE
#include <stdio.h>
#include <limits.h> /* For CHAR_BIT. */
=== modified file 'src/buffer.h'
--- src/buffer.h 2012-08-24 04:43:52 +0000
+++ src/buffer.h 2012-08-24 21:24:45 +0000
@@ -948,7 +948,6 @@
BUFFER_INLINE INTERVAL
buffer_intervals (struct buffer *b)
{
- eassert (b->text != NULL);
return b->text->intervals;
}
@@ -957,7 +956,6 @@
BUFFER_INLINE void
set_buffer_intervals (struct buffer *b, INTERVAL i)
{
- eassert (b->text != NULL);
b->text->intervals = i;
}
=== modified file 'src/lisp.h'
--- src/lisp.h 2012-08-24 04:43:52 +0000
+++ src/lisp.h 2012-08-24 21:24:45 +0000
@@ -32,6 +32,7 @@
INLINE_HEADER_BEGIN
#ifndef LISP_INLINE
# define LISP_INLINE INLINE
+# define LISP_INLINE_EXTERNALLY_VISIBLE INLINE
#endif
/* The ubiquitous max and min macros. */
@@ -110,8 +111,13 @@
/* Define an Emacs version of 'assert (COND)', since some
system-defined 'assert's are flaky. COND should be free of side
effects; it may or may not be evaluated. */
+
+#define eassert(cond) eassert_f (cond, __FILE__, __LINE__)
+
#ifndef ENABLE_CHECKING
-# define eassert(X) ((void) (0 && (X))) /* Check that X compiles. */
+ /* Do nothing, but check that args compile. */
+# define eassert_f(cond, file, line) \
+ ((void) (0 && (cond) && (file)[(line) >> 31]))
#else /* ENABLE_CHECKING */
extern _Noreturn void die (const char *, const char *, int);
@@ -126,10 +132,10 @@
testing that STRINGP (x) is true, making the test redundant. */
extern bool suppress_checking EXTERNALLY_VISIBLE;
-# define eassert(cond) \
+# define eassert_f(cond, file, line) \
((cond) || suppress_checking \
? (void) 0 \
- : die ("assertion failed: " # cond, __FILE__, __LINE__))
+ : die ("assertion failed: " # cond, file, line))
#endif /* ENABLE_CHECKING */
/* Use the configure flag --enable-check-lisp-object-type to make
@@ -2335,7 +2341,6 @@
{
/* Like ASET, but also can be used in the garbage collector:
sweep_weak_table calls set_hash_key etc. while the table is marked. */
- eassert (0 <= idx && idx < (ASIZE (array) & ~ARRAY_MARK_FLAG));
XVECTOR (array)->contents[idx] = val;
}
@@ -2344,7 +2349,6 @@
LISP_INLINE void
vcopy (Lisp_Object v, ptrdiff_t offset, Lisp_Object *args, ptrdiff_t count)
{
- eassert (0 <= offset && 0 <= count && offset + count <= ASIZE (v));
memcpy (XVECTOR (v)->contents + offset, args, count * sizeof *args);
}
@@ -2383,18 +2387,33 @@
/* Buffer-local (also frame-local) variable access functions. */
LISP_INLINE int
-blv_found (struct Lisp_Buffer_Local_Value *blv)
+blv_found_f (struct Lisp_Buffer_Local_Value *blv, char const *file, int line)
{
- eassert (blv->found == !EQ (blv->defcell, blv->valcell));
+ eassert_f (blv->found == !EQ (blv->defcell, blv->valcell), file, line);
return blv->found;
}
+#define blv_found(blv) \
+ blv_found_f (blv, __FILE__, __LINE__)
+LISP_INLINE_EXTERNALLY_VISIBLE int
+(blv_found) (struct Lisp_Buffer_Local_Value *blv)
+{
+ return blv_found (blv);
+}
LISP_INLINE void
-set_blv_found (struct Lisp_Buffer_Local_Value *blv, int found)
+set_blv_found_f (struct Lisp_Buffer_Local_Value *blv, int found,
+ char const *file, int line)
{
- eassert (found == !EQ (blv->defcell, blv->valcell));
+ eassert_f (found == !EQ (blv->defcell, blv->valcell), file, line);
blv->found = found;
}
+#define set_blv_found(blv, found) \
+ set_blv_found_f (blv, found, __FILE__, __LINE__)
+LISP_INLINE_EXTERNALLY_VISIBLE void
+(set_blv_found) (struct Lisp_Buffer_Local_Value *blv, int found)
+{
+ set_blv_found (blv, found);
+}
LISP_INLINE Lisp_Object
blv_value (struct Lisp_Buffer_Local_Value *blv)
- Re: C backtraces for Emacs, (continued)
- Re: C backtraces for Emacs, Paul Eggert, 2012/08/24
- Re: C backtraces for Emacs, Paul Eggert, 2012/08/25
- Re: C backtraces for Emacs, Eli Zaretskii, 2012/08/25
- Re: inlinable functions instead of macros, Paul Eggert, 2012/08/22
- Re: inlinable functions instead of macros, Andreas Schwab, 2012/08/22
- Re: inlinable functions instead of macros, Stefan Monnier, 2012/08/22
- Re: inlinable functions instead of macros,
Paul Eggert <=
- Re: inlinable functions instead of macros, Tom Tromey, 2012/08/24
- Re: inlinable functions instead of macros, Paul Eggert, 2012/08/24
- Re: inlinable functions instead of macros, Eli Zaretskii, 2012/08/25
- Re: inlinable functions instead of macros, Tom Tromey, 2012/08/25
- Re: inlinable functions instead of macros, Paul Eggert, 2012/08/26
- Re: inlinable functions instead of macros, Stefan Monnier, 2012/08/24
- Re: inlinable functions instead of macros, Paul Eggert, 2012/08/26
- Re: inlinable functions instead of macros, Eli Zaretskii, 2012/08/22
Re: inlinable functions instead of macros, Richard Stallman, 2012/08/18