From dfed333b312d06b3416ebfadff544eae38313391 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Wed, 22 May 2019 13:25:47 -0700 Subject: [PATCH] Remove fixnum restriction on some display vars This is a minor patch to remove some fixnum restrictions. Many more such patches are needed, but one thing at a time. * doc/emacs/custom.texi (Examining): Update fill-column example. * src/buffer.c (fill-column, left-margin, tab-width) (buffer-saved-size, left-margin-width, right-margin-width) (left-fringe-width, right-fringe-width, scroll-bar-width) (scroll-bar-height, buffer-display-count): Allow any integer; do not restrict to fixnums. * src/character.h (SANE_TAB_WIDTH): Do not assume tab_width is a nonnegative fixnum. (sanitize_tab_width): Take a Lisp_Object integer, not an EMACS_INT. Only use changed. * src/data.c (store_symval_forwarding): Remove unnecessary SYMBOLP since the predicate (e.g., Qintegerp) is always a symbol (leave the test in as an eassert). Avoid assignments inside if-conditions. * src/fileio.c (Fdo_auto_save): Do not assume buffer-saved-size is a fixnum. Avoid undefined behavior on EMACS_INT overflow by multiplying a fixnum by at most 4, not by at most 13. * src/window.c (set_window_buffer): When buffer-display-count is too large for a fixnum, make it a bignum. * src/xdisp.c (FILL_COLUMN_INDICATOR_NEEDED): Remove macro, ... (fill_column_indicator_column): ... replacing with this new function. All uses changed. The function is a bit pickier, to prevent problems with non-character fixnums and columns out of range for int, and to remove the assumption that integers are in fixnum range. (append_space_for_newline, extend_face_to_end_of_line): Avoid undefined behavior with signed integer overflow. Simplify. --- doc/emacs/custom.texi | 1 + src/buffer.c | 22 +++---- src/character.h | 9 +-- src/data.c | 23 +++---- src/fileio.c | 9 ++- src/lisp.h | 2 +- src/window.c | 4 +- src/xdisp.c | 141 +++++++++++++++++++----------------------- 8 files changed, 101 insertions(+), 110 deletions(-) diff --git a/doc/emacs/custom.texi b/doc/emacs/custom.texi index 8a8ac5d0464..bdd6decb6b5 100644 --- a/doc/emacs/custom.texi +++ b/doc/emacs/custom.texi @@ -801,6 +801,7 @@ Its value is 70 Automatically becomes buffer-local when set. This variable is safe as a file local variable if its value satisfies the predicate ‘integerp’. + Probably introduced at or before Emacs version 18. Documentation: Column beyond which automatic line-wrapping should happen. diff --git a/src/buffer.c b/src/buffer.c index 3b5078a175b..209e29f0f19 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -5603,17 +5603,17 @@ Use the command `abbrev-mode' to change this variable. */); doc: /* Non-nil if searches and matches should ignore case. */); DEFVAR_PER_BUFFER ("fill-column", &BVAR (current_buffer, fill_column), - Qfixnump, + Qintegerp, doc: /* Column beyond which automatic line-wrapping should happen. Interactively, you can set the buffer local value using \\[set-fill-column]. */); DEFVAR_PER_BUFFER ("left-margin", &BVAR (current_buffer, left_margin), - Qfixnump, + Qintegerp, doc: /* Column for the default `indent-line-function' to indent to. Linefeed indents to this column in Fundamental mode. */); DEFVAR_PER_BUFFER ("tab-width", &BVAR (current_buffer, tab_width), - Qfixnump, + Qintegerp, doc: /* Distance between tab stops (for display of tab characters), in columns. NOTE: This controls the display width of a TAB character, and not the size of an indentation step. @@ -5784,7 +5784,7 @@ If it is nil, that means don't auto-save this buffer. */); Backing up is done before the first time the file is saved. */); DEFVAR_PER_BUFFER ("buffer-saved-size", &BVAR (current_buffer, save_length), - Qfixnump, + Qintegerp, doc: /* Length of current buffer when last read in, saved or auto-saved. 0 initially. -1 means auto-saving turned off until next real save. @@ -5858,7 +5858,7 @@ In addition, a char-table has six extra slots to control the display of: See also the functions `display-table-slot' and `set-display-table-slot'. */); DEFVAR_PER_BUFFER ("left-margin-width", &BVAR (current_buffer, left_margin_cols), - Qfixnump, + Qintegerp, doc: /* Width in columns of left marginal area for display of a buffer. A value of nil means no marginal area. @@ -5866,7 +5866,7 @@ Setting this variable does not take effect until a new buffer is displayed in a window. To make the change take effect, call `set-window-buffer'. */); DEFVAR_PER_BUFFER ("right-margin-width", &BVAR (current_buffer, right_margin_cols), - Qfixnump, + Qintegerp, doc: /* Width in columns of right marginal area for display of a buffer. A value of nil means no marginal area. @@ -5874,7 +5874,7 @@ Setting this variable does not take effect until a new buffer is displayed in a window. To make the change take effect, call `set-window-buffer'. */); DEFVAR_PER_BUFFER ("left-fringe-width", &BVAR (current_buffer, left_fringe_width), - Qfixnump, + Qintegerp, doc: /* Width of this buffer's left fringe (in pixels). A value of 0 means no left fringe is shown in this buffer's window. A value of nil means to use the left fringe width from the window's frame. @@ -5883,7 +5883,7 @@ Setting this variable does not take effect until a new buffer is displayed in a window. To make the change take effect, call `set-window-buffer'. */); DEFVAR_PER_BUFFER ("right-fringe-width", &BVAR (current_buffer, right_fringe_width), - Qfixnump, + Qintegerp, doc: /* Width of this buffer's right fringe (in pixels). A value of 0 means no right fringe is shown in this buffer's window. A value of nil means to use the right fringe width from the window's frame. @@ -5900,12 +5900,12 @@ Setting this variable does not take effect until a new buffer is displayed in a window. To make the change take effect, call `set-window-buffer'. */); DEFVAR_PER_BUFFER ("scroll-bar-width", &BVAR (current_buffer, scroll_bar_width), - Qfixnump, + Qintegerp, doc: /* Width of this buffer's vertical scroll bars in pixels. A value of nil means to use the scroll bar width from the window's frame. */); DEFVAR_PER_BUFFER ("scroll-bar-height", &BVAR (current_buffer, scroll_bar_height), - Qfixnump, + Qintegerp, doc: /* Height of this buffer's horizontal scroll bars in pixels. A value of nil means to use the scroll bar height from the window's frame. */); @@ -6175,7 +6175,7 @@ Setting this variable is very fast, much faster than scanning all the text in the buffer looking for properties to change. */); DEFVAR_PER_BUFFER ("buffer-display-count", - &BVAR (current_buffer, display_count), Qfixnump, + &BVAR (current_buffer, display_count), Qintegerp, doc: /* A number incremented each time this buffer is displayed in a window. The function `set-window-buffer' increments it. */); diff --git a/src/character.h b/src/character.h index 5dff85aed47..cc57a2a7d5c 100644 --- a/src/character.h +++ b/src/character.h @@ -558,12 +558,13 @@ enum /* Return a non-outlandish value for the tab width. */ -#define SANE_TAB_WIDTH(buf) \ - sanitize_tab_width (XFIXNAT (BVAR (buf, tab_width))) +#define SANE_TAB_WIDTH(buf) sanitize_tab_width (BVAR (buf, tab_width)) + INLINE int -sanitize_tab_width (EMACS_INT width) +sanitize_tab_width (Lisp_Object width) { - return 0 < width && width <= 1000 ? width : 8; + return (FIXNUMP (width) && 0 < XFIXNUM (width) && XFIXNUM (width) <= 1000 + ? XFIXNUM (width) : 8); } /* Return the width of ASCII character C. The width is measured by diff --git a/src/data.c b/src/data.c index 476d28eadbc..c1699aeae73 100644 --- a/src/data.c +++ b/src/data.c @@ -1122,20 +1122,21 @@ store_symval_forwarding (lispfwd valcontents, Lisp_Object newval, int offset = XBUFFER_OBJFWD (valcontents)->offset; Lisp_Object predicate = XBUFFER_OBJFWD (valcontents)->predicate; - if (!NILP (newval)) + if (!NILP (newval) && !NILP (predicate)) { - if (SYMBOLP (predicate)) + eassert (SYMBOLP (predicate)); + Lisp_Object choiceprop = Fget (predicate, Qchoice); + if (!NILP (choiceprop)) { - Lisp_Object prop; - - if ((prop = Fget (predicate, Qchoice), !NILP (prop))) - { - if (NILP (Fmemq (newval, prop))) - wrong_choice (prop, newval); - } - else if ((prop = Fget (predicate, Qrange), !NILP (prop))) + if (NILP (Fmemq (newval, choiceprop))) + wrong_choice (choiceprop, newval); + } + else + { + Lisp_Object rangeprop = Fget (predicate, Qrange); + if (CONSP (rangeprop)) { - Lisp_Object min = XCAR (prop), max = XCDR (prop); + Lisp_Object min = XCAR (rangeprop), max = XCDR (rangeprop); if (! NUMBERP (newval) || NILP (CALLN (Fleq, min, newval, max))) wrong_range (min, max, newval); diff --git a/src/fileio.c b/src/fileio.c index 4ee125d7de2..9e9779967dd 100644 --- a/src/fileio.c +++ b/src/fileio.c @@ -5802,6 +5802,7 @@ A non-nil CURRENT-ONLY argument means save only current buffer. */) && BUF_SAVE_MODIFF (b) < BUF_MODIFF (b) && BUF_AUTOSAVE_MODIFF (b) < BUF_MODIFF (b) /* -1 means we've turned off autosaving for a while--see below. */ + && FIXNUMP (BVAR (b, save_length)) && XFIXNUM (BVAR (b, save_length)) >= 0 && (do_handled_files || NILP (Ffind_file_name_handler (BVAR (b, auto_save_file_name), @@ -5815,13 +5816,17 @@ A non-nil CURRENT-ONLY argument means save only current buffer. */) && before_time.tv_sec - b->auto_save_failure_time < 1200) continue; + enum { growth_factor = 4 }; + verify (BUF_BYTES_MAX <= EMACS_INT_MAX / growth_factor); + set_buffer_internal (b); if (NILP (Vauto_save_include_big_deletions) - && (XFIXNAT (BVAR (b, save_length)) * 10 - > (BUF_Z (b) - BUF_BEG (b)) * 13) + && FIXNUMP (BVAR (b, save_length)) /* A short file is likely to change a large fraction; spare the user annoying messages. */ && XFIXNAT (BVAR (b, save_length)) > 5000 + && (growth_factor * (BUF_Z (b) - BUF_BEG (b)) + < (growth_factor - 1) * XFIXNAT (BVAR (b, save_length))) /* These messages are frequent and annoying for `*mail*'. */ && !NILP (BVAR (b, filename)) && NILP (no_message)) diff --git a/src/lisp.h b/src/lisp.h index 876b757bf3f..6db90596899 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -2679,7 +2679,7 @@ struct Lisp_Buffer_Objfwd { enum Lisp_Fwd_Type type; /* = Lisp_Fwd_Buffer_Obj */ int offset; - /* One of Qnil, Qfixnump, Qsymbolp, Qstringp, Qfloatp or Qnumberp. */ + /* One of Qnil, Qintegerp, Qsymbolp, Qstringp, Qfloatp or Qnumberp. */ Lisp_Object predicate; }; diff --git a/src/window.c b/src/window.c index ca7cf7a4a61..deeb4f63fe0 100644 --- a/src/window.c +++ b/src/window.c @@ -3947,8 +3947,8 @@ set_window_buffer (Lisp_Object window, Lisp_Object buffer, b->display_error_modiff = 0; /* Update time stamps of buffer display. */ - if (FIXNUMP (BVAR (b, display_count))) - bset_display_count (b, make_fixnum (XFIXNUM (BVAR (b, display_count)) + 1)); + if (INTEGERP (BVAR (b, display_count))) + bset_display_count (b, Fadd1 (BVAR (b, display_count))); bset_display_time (b, Fcurrent_time ()); w->window_end_pos = 0; diff --git a/src/xdisp.c b/src/xdisp.c index 9eed74cb98a..5f438152341 100644 --- a/src/xdisp.c +++ b/src/xdisp.c @@ -376,17 +376,26 @@ static Lisp_Object list_of_error; || it->s[IT_BYTEPOS (*it)] == '\t')) \ || (IT_BYTEPOS (*it) < ZV_BYTE \ && (*BYTE_POS_ADDR (IT_BYTEPOS (*it)) == ' ' \ - || *BYTE_POS_ADDR (IT_BYTEPOS (*it)) == '\t')))) \ - -/* Test all the conditions needed to print the fill column indicator. */ -#define FILL_COLUMN_INDICATOR_NEEDED(it) \ - Vdisplay_fill_column_indicator \ - && (it->continuation_lines_width == 0) \ - && (!NILP (Vdisplay_fill_column_indicator_column)) \ - && FIXNATP (Vdisplay_fill_column_indicator_character) \ - && ((EQ (Vdisplay_fill_column_indicator_column, Qt) \ - && FIXNATP (BVAR (current_buffer, fill_column))) \ - || (FIXNATP (Vdisplay_fill_column_indicator_column))) + || *BYTE_POS_ADDR (IT_BYTEPOS (*it)) == '\t')))) + +/* If all the conditions needed to print the fill column indicator are + met, return the (nonnegative) column number, else return a negative + value. */ +static int +fill_column_indicator_column (struct it *it) +{ + if (Vdisplay_fill_column_indicator + && it->continuation_lines_width == 0 + && CHARACTERP (Vdisplay_fill_column_indicator_character)) + { + Lisp_Object col = (EQ (Vdisplay_fill_column_indicator_column, Qt) + ? BVAR (current_buffer, fill_column) + : Vdisplay_fill_column_indicator_column); + if (RANGED_FIXNUMP (0, col, INT_MAX)) + return XFIXNUM (col); + } + return -1; +} /* True means print newline to stdout before next mini-buffer message. */ @@ -20160,18 +20169,11 @@ append_space_for_newline (struct it *it, bool default_face_p) /* Corner case for when display-fill-column-indicator-mode is active and the extra character should be added in the same place than the line. */ - if ((it->w->pseudo_window_p == 0) - && FILL_COLUMN_INDICATOR_NEEDED(it)) + int indicator_column = (it->w->pseudo_window_p == 0 + ? fill_column_indicator_column (it) + : -1); + if (0 <= indicator_column) { - int fill_column_indicator_column = -1; - - if (EQ (Vdisplay_fill_column_indicator_column, Qt)) - fill_column_indicator_column = - XFIXNAT (BVAR (current_buffer, fill_column)); - else - fill_column_indicator_column = - XFIXNAT (Vdisplay_fill_column_indicator_column); - struct font *font = default_face->font ? default_face->font : FRAME_FONT (it->f); @@ -20179,18 +20181,19 @@ append_space_for_newline (struct it *it, bool default_face_p) font->average_width ? font->average_width : font->space_width; - const int column_x = - char_width * fill_column_indicator_column + - it->lnum_pixel_width; - - if (it->current_x == column_x) + int column_x; + if (!INT_MULTIPLY_WRAPV (indicator_column, char_width, + &column_x) + && !INT_ADD_WRAPV (it->lnum_pixel_width, column_x, + &column_x) + && it->current_x == column_x) { it->c = it->char_to_display = XFIXNAT (Vdisplay_fill_column_indicator_character); it->face_id = merge_faces (it->w, Qfill_column_indicator, 0, saved_face_id); - face = FACE_FROM_ID(it->f, it->face_id); + face = FACE_FROM_ID (it->f, it->face_id); goto produce_glyphs; } } @@ -20422,30 +20425,22 @@ extend_face_to_end_of_line (struct it *it) /* Display fill column indicator if not in modeline or toolbar and display fill column indicator mode is active. */ - if ((it->w->pseudo_window_p == 0) - && FILL_COLUMN_INDICATOR_NEEDED(it)) + int indicator_column = (it->w->pseudo_window_p == 0 + ? fill_column_indicator_column (it) + : -1); + if (0 <= indicator_column) { - int fill_column_indicator_column = -1; - - if (EQ (Vdisplay_fill_column_indicator_column, Qt)) - fill_column_indicator_column = - XFIXNAT (BVAR (current_buffer, fill_column)); - else - fill_column_indicator_column = - XFIXNAT (Vdisplay_fill_column_indicator_column); - struct font *font = default_face->font ? default_face->font : FRAME_FONT (f); const int char_width = font->average_width ? font->average_width : font->space_width; - const int column_x = - char_width * fill_column_indicator_column + - it->lnum_pixel_width; - - if ((it->current_x <= column_x) - && (column_x <= it->last_visible_x)) + int column_x; + if (!INT_MULTIPLY_WRAPV (indicator_column, char_width, &column_x) + && !INT_ADD_WRAPV (it->lnum_pixel_width, column_x, &column_x) + && it->current_x <= column_x + && column_x <= it->last_visible_x) { const char saved_char = it->char_to_display; const struct text_pos saved_pos = it->position; @@ -20625,45 +20620,33 @@ extend_face_to_end_of_line (struct it *it) it->face_id = face->id; /* Display fill-column indicator if needed. */ - if (FILL_COLUMN_INDICATOR_NEEDED(it)) + int indicator_column = fill_column_indicator_column (it); + if (0 <= indicator_column + && INT_ADD_WRAPV (it->lnum_pixel_width, indicator_column, + &indicator_column)) + indicator_column = -1; + do { - int fill_column_indicator_column = -1; + int saved_face_id; + bool indicate = it->current_x == indicator_column; + if (indicate) + { + saved_face_id = it->face_id; + it->face_id = + merge_faces (it->w, Qfill_column_indicator, 0, saved_face_id); + it->c = it->char_to_display = + XFIXNAT (Vdisplay_fill_column_indicator_character); + } - /* Vdisplay_fill_column_indicator_column accepts the special - value t to use the default fill-column variable. The - conditions are all defined in the macro - FILL_COLUMN_INDICATOR_NEEDED. */ - if (EQ (Vdisplay_fill_column_indicator_column, Qt)) - fill_column_indicator_column = - XFIXNAT (BVAR (current_buffer, fill_column)) + it->lnum_pixel_width; - else - fill_column_indicator_column = - XFIXNAT (Vdisplay_fill_column_indicator_column) + it->lnum_pixel_width; + PRODUCE_GLYPHS (it); - do + if (indicate) { - if (it->current_x == fill_column_indicator_column) - { - const int saved_face_id = it->face_id; - it->face_id = - merge_faces (it->w, Qfill_column_indicator, 0, saved_face_id); - it->c = it->char_to_display = - XFIXNAT (Vdisplay_fill_column_indicator_character); - PRODUCE_GLYPHS (it); - it->face_id = saved_face_id; - it->c = it->char_to_display = ' '; - } - else - PRODUCE_GLYPHS (it); - } while (it->current_x <= it->last_visible_x); + it->face_id = saved_face_id; + it->c = it->char_to_display = ' '; + } } - else - { - do - { - PRODUCE_GLYPHS (it); - } while (it->current_x <= it->last_visible_x); - } + while (it->current_x <= it->last_visible_x); if (WINDOW_RIGHT_MARGIN_WIDTH (it->w) > 0 && (it->glyph_row->used[RIGHT_MARGIN_AREA] -- 2.39.2