Subject: [PATCH 2/2] Add compile-time check that BVAR is used correctly

From: Spencer Baugh <[email-masked]>
Date: Sun, 9 May 2021 13:09:33 -0400

BVAR should only be used for permanent-local buffer variables, so
we'll make a list of such variables and check that the field passed to
BVAR is in that list.  The added offsetof call will fail at compile
time if the passed field is not present in bvar_permanent_locals,
while not incurring any overhead at runtime.

We could add a similar check for BVAR_OR_DEFAULT, but it's not as
important since BVAR_OR_DEFAULT will work fine for any Lisp field.

* src/buffer.h (BVAR): Add compile-time check that the passed field is
permanently buffer-local.
(BVAR_OR_DEFAULT): Stop using BVAR, since this can be called on fields
which aren't permanently buffer-local.
(BVAR_DEFAULT): Add.
* src/buffer.c (init_buffer_once):
* src/category.h (Vstandard_category_table):
* src/syntax.h (Vstandard_syntax_table): Use BVAR_DEFAULT instead of
BVAR to access buffer_defaults fields that aren't permanently
buffer-local.
---
 src/buffer.c   | 14 +++++++-------
 src/buffer.h   | 28 +++++++++++++++++++++++-----
 src/category.h |  2 +-
 src/syntax.h   |  2 +-
 4 files changed, 32 insertions(+), 14 deletions(-)

diff --git a/src/buffer.c b/src/buffer.c
index c0d72ef5c5..1ee804bd9d 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -5112,10 +5112,10 @@ init_buffer_once (void)
   bset_abbrev_table (&buffer_defaults, Qnil);
   bset_display_table (&buffer_defaults, Qnil);
   /* Later further initialized by init_{syntax,category}_once.  */
-  BVAR (&buffer_defaults, syntax_table) = Qnil;
-  BVAR (&buffer_defaults, category_table) = Qnil;
+  BVAR_DEFAULT (syntax_table) = Qnil;
+  BVAR_DEFAULT (category_table) = Qnil;
 
-  XSETFASTINT (BVAR (&buffer_defaults, tab_width), 8);
+  XSETFASTINT (BVAR_DEFAULT (tab_width), 8);
   bset_truncate_lines (&buffer_defaults, Qnil);
   bset_word_wrap (&buffer_defaults, Qnil);
   bset_ctl_arrow (&buffer_defaults, Qt);
@@ -5128,11 +5128,11 @@ init_buffer_once (void)
   bset_cursor_in_non_selected_windows (&buffer_defaults, Qt);
 
   bset_buffer_file_coding_system (&buffer_defaults, Qnil);
-  XSETFASTINT (BVAR (&buffer_defaults, fill_column), 70);
-  XSETFASTINT (BVAR (&buffer_defaults, left_margin), 0);
+  XSETFASTINT (BVAR_DEFAULT (fill_column), 70);
+  XSETFASTINT (BVAR_DEFAULT (left_margin), 0);
   bset_cache_long_scans (&buffer_defaults, Qt);
-  XSETFASTINT (BVAR (&buffer_defaults, left_margin_cols), 0);
-  XSETFASTINT (BVAR (&buffer_defaults, right_margin_cols), 0);
+  XSETFASTINT (BVAR_DEFAULT (left_margin_cols), 0);
+  XSETFASTINT (BVAR_DEFAULT (right_margin_cols), 0);
   bset_left_fringe_width (&buffer_defaults, Qnil);
   bset_right_fringe_width (&buffer_defaults, Qnil);
   bset_fringes_outside_margins (&buffer_defaults, Qnil);
diff --git a/src/buffer.h b/src/buffer.h
index f78046a9a8..3e8198bb70 100644
--- a/src/buffer.h
+++ b/src/buffer.h
@@ -280,13 +280,31 @@ struct buffer_text
     bool_bf redisplay : 1;
   };
 
-/* Most code should use this macro to access Lisp fields in struct buffer.  */
+/* This structure is used purely as a list of field names that are
+   permitted for use with BVAR; it's never actually instantiated.  */
+struct bvar_permanent_locals {
+  char name, filename, directory, backed_up, save_length, auto_save_file_name,
+    read_only, mark, local_var_alist, major_mode, local_minor_modes, mode_name,
+    keymap, downcase_table, upcase_table, case_canon_table, case_eqv_table,
+    mark_active, enable_multibyte_characters, file_format,
+    auto_save_file_format, width_table, pt_marker, begv_marker, zv_marker,
+    point_before_scroll, file_truename, invisibility_spec, last_selected_window,
+    display_count, display_time, undo_list;
+};
+
+/* Most code should use BVAR or BVAR_OR_DEFAULT to access Lisp fields
+   in struct buffer.  BVAR should be used for fields that are
+   permanently buffer-local, and BVAR_OR_DEFAULT should be used for
+   fields that may not have a buffer-local binding and should use the
+   default in that case.  */
+#define BVAR(buf, field) \
+  *((void) offsetof (struct bvar_permanent_locals, field), &(buf)->field ## _)
 
-#define BVAR(buf, field) ((buf)->field ## _)
+#define BVAR_OR_DEFAULT(buf, field) (EQ ((buf)->field ## _, Qunbound)	\
+				     ? BVAR_DEFAULT(field)		\
+				     : (buf)->field ## _)
 
-#define BVAR_OR_DEFAULT(buf, field) (EQ (BVAR ((buf), field), Qunbound) \
-				     ? BVAR (&buffer_defaults, field) \
-				     : BVAR ((buf), field))
+#define BVAR_DEFAULT(field) buffer_defaults.field ## _
 
 /* Max number of builtin per-buffer variables.  */
 enum { MAX_PER_BUFFER_VARS = 50 };
diff --git a/src/category.h b/src/category.h
index cc32990478..85e872a940 100644
--- a/src/category.h
+++ b/src/category.h
@@ -94,7 +94,7 @@ CHAR_HAS_CATEGORY (int ch, int category)
 
 /* The standard category table is stored where it will automatically
    be used in all new buffers.  */
-#define Vstandard_category_table BVAR (&buffer_defaults, category_table)
+#define Vstandard_category_table BVAR_DEFAULT (category_table)
 
 /* Return the doc string of CATEGORY in category table TABLE.  */
 #define CATEGORY_DOCSTRING(table, category)				\
diff --git a/src/syntax.h b/src/syntax.h
index 187946c899..c85743808c 100644
--- a/src/syntax.h
+++ b/src/syntax.h
@@ -31,7 +31,7 @@ extern void update_syntax_table_forward (ptrdiff_t, bool, Lisp_Object);
 
 /* The standard syntax table is stored where it will automatically
    be used in all new buffers.  */
-#define Vstandard_syntax_table BVAR (&buffer_defaults, syntax_table)
+#define Vstandard_syntax_table BVAR_DEFAULT (syntax_table)
 
 /* A syntax table is a chartable whose elements are cons cells
    (CODE+FLAGS . MATCHING-CHAR).  MATCHING-CHAR can be nil if the char
-- 
2.31.1