[COMMIT] Reduce calls to marker_position(), bytecode comparisons, #'max, #'min

APPROVE COMMIT NOTE: This patch has been committed # HG changeset patch # User Aidan Kehoe <kehoea(a)parhasard.net&gt; # Date 1534371432 -3600 # Wed Aug 15 23:17:12 2018 +0100 # Node ID 51111ef13381b362dba0f4463ddae7257871e24f # Parent 99135ed6ffc1217d9082691df80fb3a3471785ce Reduce calls to marker_position(), bytecode comparisons, #'max, #'min src/ChangeLog addition: 2018-08-15 Aidan Kehoe <kehoea(a)parhasard.net&gt; Calls to marker_position() are O(N), reduce them in bytecode. * buffer.h: * bytecode.c: * bytecode.c (bytecode_gtr): * bytecode.c (bytecode_lss): * bytecode.c (bytecode_leq): * bytecode.c (bytecode_geq): * bytecode.c (bytecode_arithop): * data.c: * data.c (Fmax): * data.c (Fmin): * insdel.c (init_buffer_text): * number.h: * number.h (promote_args_lazy): 1. In bytecode and in interpreted code, if #'max, #'min are supplied marker arguments, return a marker result if they can be compared without converting to fixnums. 2. In bytecode, when doing arithmetic comparisons between markers and fixnums, uses the fact the the byte marker position is always the inclusive upper bound on the character marker position, and that (byte_marker_position(OBJ) / MAX_ICHAR_LEN) is the inclusive lower bound, to convert O(N) operations to O(1) for a decent proportion of comparisons like (< MARKER (point-max)). 3. Buffer format is actually fixed at compile time, make decisions regarding that easier for the compiler. tests/ChangeLog addition: 2018-08-15 Aidan Kehoe <kehoea(a)parhasard.net&gt; * automated/lisp-tests.el: 1. #'max, #'min can now return markers, accept that. 2. Gross checks of comparison of fixnums and markers for correctness, now we have made some bytecode changes to help with that. man/ChangeLog addition: 2018-08-15 Aidan Kehoe <kehoea(a)parhasard.net&gt; * lispref/markers.texi (Information from Markers): Document an important algorithmic consideration with #'marker-position. diff -r 99135ed6ffc1 -r 51111ef13381 man/ChangeLog --- a/man/ChangeLog Tue Aug 14 12:17:35 2018 +0100 +++ b/man/ChangeLog Wed Aug 15 23:17:12 2018 +0100 ＠＠ -1,3 +1,9 ＠＠ +2018-08-15 Aidan Kehoe <kehoea(a)parhasard.net&gt; + + * lispref/markers.texi (Information from Markers): + Document an important algorithmic consideration with + #'marker-position. + 2017-11-23 Aidan Kehoe <kehoea(a)parhasard.net&gt; * internals/internals.texi (A Summary of the Various XEmacs Modules): diff -r 99135ed6ffc1 -r 51111ef13381 man/lispref/markers.texi --- a/man/lispref/markers.texi Tue Aug 14 12:17:35 2018 +0100 +++ b/man/lispref/markers.texi Wed Aug 15 23:17:12 2018 +0100 ＠＠ -348,7 +348,8 ＠＠ ＠defun marker-position marker This function returns the position that ＠var{marker} points to, or -＠code{nil} if it points nowhere. +＠code{nil} if it points nowhere. This operation is O(N) where ＠var{N} is +the number of characters in the buffer; avoid it if you can. ＠end defun ＠defun marker-buffer marker diff -r 99135ed6ffc1 -r 51111ef13381 src/ChangeLog --- a/src/ChangeLog Tue Aug 14 12:17:35 2018 +0100 +++ b/src/ChangeLog Wed Aug 15 23:17:12 2018 +0100 ＠＠ -1,3 +1,31 ＠＠ +2018-08-15 Aidan Kehoe <kehoea(a)parhasard.net&gt; + + Calls to marker_position() are O(N), reduce them in bytecode. + * buffer.h: + * bytecode.c: + * bytecode.c (bytecode_gtr): + * bytecode.c (bytecode_lss): + * bytecode.c (bytecode_leq): + * bytecode.c (bytecode_geq): + * bytecode.c (bytecode_arithop): + * data.c: + * data.c (Fmax): + * data.c (Fmin): + * insdel.c (init_buffer_text): + * number.h: + * number.h (promote_args_lazy): + 1. In bytecode and in interpreted code, if #'max, #'min are + supplied marker arguments, return a marker result if they can be + compared without converting to fixnums. + 2. In bytecode, when doing arithmetic comparisons between markers + and fixnums, uses the fact the the byte marker position is always + the inclusive upper bound on the character marker position, and + that (byte_marker_position(OBJ) / MAX_ICHAR_LEN) is the inclusive + lower bound, to convert O(N) operations to O(1) for a decent + proportion of comparisons like (< MARKER (point-max)). + 3. Buffer format is actually fixed at compile time, make decisions + regarding that easier for the compiler. + 2018-08-12 Aidan Kehoe <kehoea(a)parhasard.net&gt; * fontcolor-x.c (x_font_instance_properties): diff -r 99135ed6ffc1 -r 51111ef13381 src/buffer.h --- a/src/buffer.h Tue Aug 14 12:17:35 2018 +0100 +++ b/src/buffer.h Wed Aug 15 23:17:12 2018 +0100 ＠＠ -345,9 +345,9 ＠＠ #define BYTE_BUF_PT(buf) ((buf)->pt + 0) #define BUF_PT(buf) ((buf)->bufpt + 0) -/* Internal format of buffer. */ +/* Internal format of buffer. Always FORMAT_DEFAULT currently. */ #ifdef MULE -#define BUF_FORMAT(buf) ((buf)->text->format) +#define BUF_FORMAT(buf) FORMAT_DEFAULT /* ((buf)->text->format) */ #else #define BUF_FORMAT(buf) FORMAT_DEFAULT #endif diff -r 99135ed6ffc1 -r 51111ef13381 src/bytecode.c --- a/src/bytecode.c Tue Aug 14 12:17:35 2018 +0100 +++ b/src/bytecode.c Wed Aug 15 23:17:12 2018 +0100 ＠＠ -382,6 +382,176 ＠＠ #endif /* WITH_NUMBER_TYPES */ } +#ifdef MULE +#define MAX_ICHAR_LEN_FORMAT(fmt) (ichar_len_fmt (CHAR_CODE_LIMIT - 1, fmt)) +#else +#define MAX_ICHAR_LEN_FORMAT(buf) MAX_ICHAR_LEN +#endif + +/* Given POS, the Bytebpos corresponding to MARKER, return the smallest + Charbpos it can represent, which represents the situation where all the + characters are of length MAX_ICHAR_LEN. */ +#define LOWER_CHAR_BOUND(marker, pos) \ + (pos / MAX_ICHAR_LEN_FORMAT (BUF_FORMAT (XMARKER (marker)->buf))) + +static Boolint +bytecode_gtr (Lisp_Object obj1, Lisp_Object obj2) +{ + if (MARKERP (obj1) && FIXNUMP (obj2)) + { + const Bytebpos pos1 = byte_marker_position (obj1); + const Charbpos cpos1min = LOWER_CHAR_BOUND (obj1, pos1); + + /* (POS1 / MAX_ICHAR_LEN) is a lower bound on OBJ1's character length, + we may be able to give a result without an O(N) operation. */ + if (cpos1min > XREALFIXNUM (obj2)) + { + return 1; + } + /* Similarly, POS1 is an upper limit on OBJ1's character offset, and if + it is less than or equal to OBJ2, OBJ1's character offset cannot be + greater than OBJ2. */ + if (pos1 <= XREALFIXNUM (obj2)) + { + return 0; + } + } + else if (FIXNUMP (obj1) && MARKERP (obj2)) + { + const Bytebpos pos2 = byte_marker_position (obj2); + const Charbpos cpos2min = LOWER_CHAR_BOUND (obj2, pos2); + + /* The byte marker position is an upper limit on the character marker + position, we may be able to compare without O(N) lossage. */ + if (XREALFIXNUM (obj1) > pos2) + { + return 1; + } + if (XREALFIXNUM (obj2) <= cpos2min) + { + return 0; + } + } + + /* It is common to compare a marker to the result of (point) or (point-max), + which are both fixnums; the above optimizations have some value given + that. In a big buffer, e.g a two-thousand message VM buffer, an O(N) + operation becoming an O(1) operation is a significant improvement! A + similar optimization would be possible for markers in different buffers, + but that is done sufficiently rarely that I don't feel it's worth it. */ + + /* FALLTHROUGH */ + return bytecode_arithcompare (obj1, obj2) > 0; +} + +static Boolint +bytecode_lss (Lisp_Object obj1, Lisp_Object obj2) +{ + /* See bytecode_gtr() for the reasoning for the following. */ + if (MARKERP (obj1) && FIXNUMP (obj2)) + { + const Bytebpos pos1 = byte_marker_position (obj1); + const Charbpos cpos1min = LOWER_CHAR_BOUND (obj1, pos1); + + if (pos1 < XFIXNUM (obj2)) + { + return 1; + } + if (cpos1min >= XFIXNUM (obj2)) + { + return 0; + } + } + else if (FIXNUMP (obj1) && MARKERP (obj2)) + { + const Bytebpos pos2 = byte_marker_position (obj2); + const Charbpos cpos2min = LOWER_CHAR_BOUND (obj2, pos2); + + if (XFIXNUM (obj1) < cpos2min) + { + return 1; + } + if (XFIXNUM (obj1) >= pos2) + { + return 0; + } + } + /* FALLTHROUGH */ + return bytecode_arithcompare (obj1, obj2) < 0; +} + +static Boolint +bytecode_leq (Lisp_Object obj1, Lisp_Object obj2) +{ + /* See bytecode_gtr() for the reasoning for the following. */ + if (MARKERP (obj1) && FIXNUMP (obj2)) + { + const Bytebpos pos1 = byte_marker_position (obj1); + const Charbpos cpos1min = LOWER_CHAR_BOUND (obj1, pos1); + + if (pos1 <= XREALFIXNUM (obj2)) + { + return 1; + } + if (cpos1min > XREALFIXNUM (obj2)) + { + return 0; + } + } + else if (FIXNUMP (obj1) && MARKERP (obj2)) + { + const Bytebpos pos2 = byte_marker_position (obj2); + const Charbpos cpos2min = LOWER_CHAR_BOUND (obj2, pos2); + + if (XREALFIXNUM (obj1) <= cpos2min) + { + return 1; + } + if (XREALFIXNUM (obj1) > pos2) + { + return 0; + } + } + + return bytecode_arithcompare (obj1, obj2) <= 0; +} + +static Boolint +bytecode_geq (Lisp_Object obj1, Lisp_Object obj2) +{ + /* See bytecode_gtr() for the reasoning for the following. */ + if (MARKERP (obj1) && FIXNUMP (obj2)) + { + const Bytebpos pos1 = byte_marker_position (obj1); + const Charbpos cpos1min = LOWER_CHAR_BOUND (obj1, pos1); + + if (cpos1min >= XREALFIXNUM (obj2)) + { + return 1; + } + if (pos1 < XREALFIXNUM (obj2)) + { + return 0; + } + } + else if (FIXNUMP (obj1) && MARKERP (obj2)) + { + const Bytebpos pos2 = byte_marker_position (obj2); + const Charbpos cpos2min = LOWER_CHAR_BOUND (obj2, pos2); + + if (XREALFIXNUM (obj1) >= pos2) + { + return 1; + } + if (XREALFIXNUM (obj1) < cpos2min) + { + return 0; + } + } + + return bytecode_arithcompare (obj1, obj2) >= 0; +} + static Lisp_Object bytecode_arithop (Lisp_Object obj1, Lisp_Object obj2, Opcode opcode) { ＠＠ -393,15 +563,11 ＠＠ switch (opcode) { case Bmax: - return make_fixnum (marker_position - ((byte_marker_position (obj1) - < byte_marker_position (obj2)) ? - obj2 : obj1)); + return byte_marker_position (obj1) < byte_marker_position (obj2) + ? obj2 : obj1; case Bmin: - return make_fixnum (marker_position - ((byte_marker_position (obj1) - > byte_marker_position (obj2)) ? - obj2 : obj1)); + return byte_marker_position (obj1) > byte_marker_position (obj2) + ? obj2 : obj1; default: obj1 = make_fixnum (marker_position (obj1)); obj2 = make_fixnum (marker_position (obj2)); ＠＠ -555,7 +721,26 ＠＠ if (FIXNUMP (obj1)) ival1 = XFIXNUM (obj1); else if (CHARP (obj1)) ival1 = XCHAR (obj1); - else if (MARKERP (obj1)) ival1 = marker_position (obj1); + else if (MARKERP (obj1)) + { + if (MARKERP (obj2) + && (XMARKER (obj1)->buffer == XMARKER (obj2)->buffer)) + { + if (opcode == Bmax) + { + return byte_marker_position (obj1) < byte_marker_position (obj2) + ? obj2 : obj1; + } + else if (opcode == Bmin) + { + return byte_marker_position (obj1) > byte_marker_position (obj2) + ? obj2 : obj1; + } + /* Otherwise, convert to a fixnum in the normal way. */ + } + ival1 = marker_position (obj1); + } + else if (FLOATP (obj1)) ival1 = 0, float_p = 1; else { ＠＠ -1249,28 +1434,28 ＠＠ case Bgtr: { Lisp_Object arg = POP; - TOP_LVALUE = bytecode_arithcompare (TOP, arg) > 0 ? Qt : Qnil; + TOP_LVALUE = bytecode_gtr (TOP, arg) ? Qt : Qnil; break; } case Blss: { Lisp_Object arg = POP; - TOP_LVALUE = bytecode_arithcompare (TOP, arg) < 0 ? Qt : Qnil; + TOP_LVALUE = bytecode_lss (TOP, arg) ? Qt : Qnil; break; } case Bleq: { Lisp_Object arg = POP; - TOP_LVALUE = bytecode_arithcompare (TOP, arg) <= 0 ? Qt : Qnil; + TOP_LVALUE = bytecode_leq (TOP, arg) ? Qt : Qnil; break; } case Bgeq: { Lisp_Object arg = POP; - TOP_LVALUE = bytecode_arithcompare (TOP, arg) >= 0 ? Qt : Qnil; + TOP_LVALUE = bytecode_geq (TOP, arg) ? Qt : Qnil; break; } diff -r 99135ed6ffc1 -r 51111ef13381 src/data.c --- a/src/data.c Tue Aug 14 12:17:35 2018 +0100 +++ b/src/data.c Wed Aug 15 23:17:12 2018 +0100 ＠＠ -2453,8 +2453,10 ＠＠ DEFUN ("max", Fmax, 1, MANY, 0, /* Return largest of all the arguments. All arguments must be real numbers, characters or markers. -The value is always a number; markers and characters are converted -to numbers. + +Markers in distinct buffers will be converted to fixnums, but markers in the +same buffer will be compared without conversion, and may be returned as the +value. Characters are always converted to fixnums, and are never returned. arguments: (FIRST &rest ARGS) */ ＠＠ -2467,35 +2469,38 ＠＠ args[0] = wrong_type_argument (Qnumber_char_or_marker_p, args[0]); if (CHARP (args[0])) args[0] = make_fixnum (XCHAR (args[0])); - else if (MARKERP (args[0])) - args[0] = make_fixnum (marker_position (args[0])); for (i = 1; i < nargs; i++) { - switch (promote_args (args + maxindex, args + i)) + switch (promote_args_lazy (args + maxindex, args + i)) { - case FIXNUM_T: + case LAZY_MARKER_T: + if (byte_marker_position (args[maxindex]) + < byte_marker_position (args[i])) + maxindex = i; + break; + case LAZY_FIXNUM_T: if (XREALFIXNUM (args[maxindex]) < XREALFIXNUM (args[i])) maxindex = i; break; #ifdef HAVE_BIGNUM - case BIGNUM_T: + case LAZY_BIGNUM_T: if (bignum_lt (XBIGNUM_DATA (args[maxindex]), XBIGNUM_DATA (args[i]))) maxindex = i; break; #endif #ifdef HAVE_RATIO - case RATIO_T: + case LAZY_RATIO_T: if (ratio_lt (XRATIO_DATA (args[maxindex]), XRATIO_DATA (args[i]))) maxindex = i; break; #endif - case FLOAT_T: + case LAZY_FLOAT_T: if (XFLOAT_DATA (args[maxindex]) < XFLOAT_DATA (args[i])) maxindex = i; break; #ifdef HAVE_BIGFLOAT - case BIGFLOAT_T: + case LAZY_BIGFLOAT_T: if (bigfloat_lt (XBIGFLOAT_DATA (args[maxindex]), XBIGFLOAT_DATA (args[i]))) maxindex = i; ＠＠ -2505,52 +2510,85 ＠＠ } return args[maxindex]; #else /* !WITH_NUMBER_TYPES */ - EMACS_INT imax; - double dmax; - Lisp_Object *args_end = args + nargs; - int_or_double iod; - - number_char_or_marker_to_int_or_double (*args++, &iod); - if (iod.int_p) - imax = iod.c.ival; - else + REGISTER int i, maxindex = 0; + Lisp_Object max_so_far; + + while (!(CHARP (args[0]) || MARKERP (args[0]) || REALP (args[0]))) + args[0] = wrong_type_argument (Qnumber_char_or_marker_p, args[0]); + if (CHARP (args[0])) + args[0] = make_fixnum (XCHAR (args[0])); + max_so_far = args[0]; + for (i = 1; i < nargs; i++) { - dmax = iod.c.dval; - goto max_floats; - } - - while (args < args_end) - { - number_char_or_marker_to_int_or_double (*args++, &iod); - if (iod.int_p) - { - if (imax < iod.c.ival) imax = iod.c.ival; - } + EMACS_INT ival1, ival2; + int float_p; + Lisp_Object obj2 = args[i]; + + retry: + float_p = 0; + + if (FIXNUMP (max_so_far)) ival1 = XFIXNUM (max_so_far); + else if (CHARP (max_so_far)) ival1 = XCHAR (max_so_far); + else if (MARKERP (max_so_far)) + { + if (MARKERP (obj2) + && (XMARKER (max_so_far)->buffer == XMARKER (obj2)->buffer)) + { + if (byte_marker_position (max_so_far) + < byte_marker_position (obj2)) + { + max_so_far = obj2; + } + continue; + } + /* Otherwise, convert to a fixnum in the normal way. */ + ival1 = marker_position (max_so_far); + } + else if (FLOATP (max_so_far)) ival1 = 0, float_p = 1; else - { - dmax = (double) imax; - if (dmax < iod.c.dval) dmax = iod.c.dval; - goto max_floats; + { + max_so_far = wrong_type_argument (Qnumber_char_or_marker_p, + max_so_far); + goto retry; + } + + if (FIXNUMP (obj2)) ival2 = XFIXNUM (obj2); + else if (CHARP (obj2)) ival2 = XCHAR (obj2); + else if (MARKERP (obj2)) ival2 = marker_position (obj2); + else if (FLOATP (obj2)) ival2 = 0, float_p = 1; + else + { + obj2 = wrong_type_argument (Qnumber_char_or_marker_p, obj2); + goto retry; + } + + if (!float_p) + { + if (ival1 < ival2) max_so_far = make_fixnum (ival2); + } + else + { + double dval1 = FLOATP (max_so_far) ? XFLOAT_DATA (max_so_far) + : (double) ival1; + double dval2 = FLOATP (obj2) ? XFLOAT_DATA (obj2) : (double) ival2; + if (dval1 < dval2) + { + max_so_far = FLOATP (obj2) ? obj2 : make_float (dval2); + } } } - return make_fixnum (imax); - - max_floats: - while (args < args_end) - { - double dval = number_char_or_marker_to_double (*args++); - if (dmax < dval) dmax = dval; - } - return make_float (dmax); + return max_so_far; #endif /* WITH_NUMBER_TYPES */ } DEFUN ("min", Fmin, 1, MANY, 0, /* Return smallest of all the arguments. All arguments must be numbers, characters or markers. -The value is always a number; markers and characters are converted -to numbers. + +Markers in distinct buffers will be converted to fixnums, but markers in the +same buffer will be compared without conversion, and may be returned as the +value. Characters are always converted to fixnums, and are never returned. arguments: (FIRST &rest ARGS) */ ＠＠ -2563,36 +2601,39 ＠＠ args[0] = wrong_type_argument (Qnumber_char_or_marker_p, args[0]); if (CHARP (args[0])) args[0] = make_fixnum (XCHAR (args[0])); - else if (MARKERP (args[0])) - args[0] = make_fixnum (marker_position (args[0])); for (i = 1; i < nargs; i++) { - switch (promote_args (args + minindex, args + i)) + switch (promote_args_lazy (args + minindex, args + i)) { - case FIXNUM_T: + case LAZY_MARKER_T: + if (byte_marker_position (args[minindex]) + > byte_marker_position (args[i])) + minindex = i; + break; + case LAZY_FIXNUM_T: if (XREALFIXNUM (args[minindex]) > XREALFIXNUM (args[i])) minindex = i; break; #ifdef HAVE_BIGNUM - case BIGNUM_T: + case LAZY_BIGNUM_T: if (bignum_gt (XBIGNUM_DATA (args[minindex]), XBIGNUM_DATA (args[i]))) minindex = i; break; #endif #ifdef HAVE_RATIO - case RATIO_T: + case LAZY_RATIO_T: if (ratio_gt (XRATIO_DATA (args[minindex]), XRATIO_DATA (args[i]))) minindex = i; break; #endif - case FLOAT_T: + case LAZY_FLOAT_T: if (XFLOAT_DATA (args[minindex]) > XFLOAT_DATA (args[i])) minindex = i; break; #ifdef HAVE_BIGFLOAT - case BIGFLOAT_T: + case LAZY_BIGFLOAT_T: if (bigfloat_gt (XBIGFLOAT_DATA (args[minindex]), XBIGFLOAT_DATA (args[i]))) minindex = i; ＠＠ -2602,44 +2643,75 ＠＠ } return args[minindex]; #else /* !WITH_NUMBER_TYPES */ - EMACS_INT imin; - double dmin; - Lisp_Object *args_end = args + nargs; - int_or_double iod; - - number_char_or_marker_to_int_or_double (*args++, &iod); - if (iod.int_p) - imin = iod.c.ival; - else + REGISTER int i; + Lisp_Object min_so_far; + + while (!(CHARP (args[0]) || MARKERP (args[0]) || REALP (args[0]))) + args[0] = wrong_type_argument (Qnumber_char_or_marker_p, args[0]); + if (CHARP (args[0])) + args[0] = make_fixnum (XCHAR (args[0])); + min_so_far = args[0]; + for (i = 1; i < nargs; i++) { - dmin = iod.c.dval; - goto min_floats; - } - - while (args < args_end) - { - number_char_or_marker_to_int_or_double (*args++, &iod); - if (iod.int_p) - { - if (imin > iod.c.ival) imin = iod.c.ival; - } + EMACS_INT ival1, ival2; + int float_p; + Lisp_Object obj2 = args[i]; + + retry: + float_p = 0; + + if (FIXNUMP (min_so_far)) ival1 = XFIXNUM (min_so_far); + else if (CHARP (min_so_far)) ival1 = XCHAR (min_so_far); + else if (MARKERP (min_so_far)) + { + if (MARKERP (obj2) + && (XMARKER (min_so_far)->buffer == XMARKER (obj2)->buffer)) + { + if (byte_marker_position (min_so_far) + > byte_marker_position (obj2)) + { + min_so_far = obj2; + } + continue; + } + /* Otherwise, convert to a fixnum in the normal way. */ + ival1 = marker_position (min_so_far); + } + else if (FLOATP (min_so_far)) ival1 = 0, float_p = 1; else - { - dmin = (double) imin; - if (dmin > iod.c.dval) dmin = iod.c.dval; - goto min_floats; + { + min_so_far = wrong_type_argument (Qnumber_char_or_marker_p, + min_so_far); + goto retry; + } + + if (FIXNUMP (obj2)) ival2 = XFIXNUM (obj2); + else if (CHARP (obj2)) ival2 = XCHAR (obj2); + else if (MARKERP (obj2)) ival2 = marker_position (obj2); + else if (FLOATP (obj2)) ival2 = 0, float_p = 1; + else + { + obj2 = wrong_type_argument (Qnumber_char_or_marker_p, obj2); + goto retry; + } + + if (!float_p) + { + if (ival1 > ival2) min_so_far = make_fixnum (ival2); + } + else + { + double dval1 = FLOATP (min_so_far) ? XFLOAT_DATA (min_so_far) + : (double) ival1; + double dval2 = FLOATP (obj2) ? XFLOAT_DATA (obj2) : (double) ival2; + if (dval1 > dval2) + { + min_so_far = FLOATP (obj2) ? obj2 : make_float (dval2); + } } } - return make_fixnum (imin); - - min_floats: - while (args < args_end) - { - double dval = number_char_or_marker_to_double (*args++); - if (dmin > dval) dmin = dval; - } - return make_float (dmin); + return min_so_far; #endif /* WITH_NUMBER_TYPES */ } diff -r 99135ed6ffc1 -r 51111ef13381 src/insdel.c --- a/src/insdel.c Tue Aug 14 12:17:35 2018 +0100 +++ b/src/insdel.c Wed Aug 15 23:17:12 2018 +0100 ＠＠ -1801,7 +1801,7 ＠＠ b->text->cached_bytepos = 1; /* &&#### Set to FORMAT_8_BIT_FIXED when that code is working */ - BUF_FORMAT (b) = FORMAT_DEFAULT; + /* BUF_FORMAT (b) = FORMAT_DEFAULT; */ #endif /* MULE */ b->text->line_number_cache = Qnil; diff -r 99135ed6ffc1 -r 51111ef13381 src/number.h --- a/src/number.h Tue Aug 14 12:17:35 2018 +0100 +++ b/src/number.h Wed Aug 15 23:17:12 2018 +0100 ＠＠ -454,14 +454,12 ＠＠ /* promote_args() *always* converts a marker argument to a fixnum. Unfortunately, for a marker with byte position N, getting the (character) - marker position is O(N). Getting the character position isn't necessary - for bytecode_arithcompare() if two markers being compared are in the same - buffer, comparing the byte position is enough. + marker position is O(N). Getting the character position isn't always + necessary for bytecode_arithcompare(), if they are in the same buffer, + comparing the byte position is enough. Similarly, min and max don't necessarily need to have their arguments - converted from markers, though we have always promised up to this point - that the result is a fixnum rather than a marker, and that's what we're - continuing to do. */ + converted from markers. */ DECLARE_INLINE_HEADER ( enum lazy_number_type diff -r 99135ed6ffc1 -r 51111ef13381 tests/ChangeLog --- a/tests/ChangeLog Tue Aug 14 12:17:35 2018 +0100 +++ b/tests/ChangeLog Wed Aug 15 23:17:12 2018 +0100 ＠＠ -1,3 +1,11 ＠＠ +2018-08-15 Aidan Kehoe <kehoea(a)parhasard.net&gt; + + * automated/lisp-tests.el: + 1. #'max, #'min can now return markers, accept that. + 2. Gross checks of comparison of fixnums and markers for + correctness, now we have made some bytecode changes to help with + that. + 2018-07-05 Aidan Kehoe <kehoea(a)parhasard.net&gt; * automated/lisp-tests.el (featurep): diff -r 99135ed6ffc1 -r 51111ef13381 tests/automated/lisp-tests.el --- a/tests/automated/lisp-tests.el Tue Aug 14 12:17:35 2018 +0100 +++ b/tests/automated/lisp-tests.el Wed Aug 15 23:17:12 2018 +0100 ＠＠ -3308,13 +3308,30 ＠＠ ,(concat "checking #'< correct with long arguments list, " context)) (map-plist #'(lambda (object1 object2) - (Assert (> object1 object2) + (Assert (and (> object1 object2) + (not (> object2 object1))) ,(concat "checking markers correctly ordered, >, " context)) - (Assert (< object2 object1) + (Assert (and (< object2 object1) + (not (< object1 object2))) ,(concat "checking markers correctly ordered, <, " + context)) + (Assert (and (>= object1 object2) + (not (>= object2 object1))) + ,(concat + "checking markers correctly ordered, >=, " + context)) + (Assert (and (<= object2 object1) + (not (<= object1 object2))) + ,(concat + "checking markers correctly ordered, <=, " + context)) + (Assert (and (<= object2 object1) + (not (<= object1 object2))) + ,(concat + "checking markers correctly ordered, <=, " context))) markers) ;; OK, so up to this point there has been no need for byte-char ＠＠ -3328,18 +3345,43 ＠＠ (= (min object1 object2) object2) ,(concat "checking min, correct, two markers, " context)) - ;; It is probably reasonable to change this design - ;; decision. + ;; XEmacs; changed design decision, don't convert + ;; markers to fixnums unless absolutely necessary, + ;; that is an O(N) operation. (Assert - (fixnump (max object1 object2)) + (markerp (max object1 object2)) + ,(concat + "checking no unnecessary fixnum conversion" + ", max, " context)) + (Assert + (markerp (min object1 object2)) ,(concat - "checking fixnum conversion as documented, max, " - context)) - (Assert - (fixnump (min object1 object2)) - ,(concat - "checking fixnum conversion as documented, min, " - context))) + "checking no unnecessary fixnum conversion" + ", min, " context)) + (Assert (and (> object1 (marker-position object2)) + (not (> (marker-position object2) + object1))) + ,(concat + "checking #'> correct, marker, fixnum " + context)) + (Assert (and (< object2 (marker-position object1)) + (not (< (marker-position object1) + object2))) + ,(concat + "checking #'< correct, marker, fixnum " + context)) + (Assert (and (>= object1 (marker-position object2)) + (not (>= (marker-position object2) + object1))) + ,(concat + "checking #'>= correct, marker, fixnum " + context)) + (Assert (and (<= object2 (marker-position object1)) + (not (<= (marker-position object1) + object2))) + ,(concat + "checking #'<= correct, marker, fixnum " + context))) markers)))) (with-temp-buffer (loop for ii from 0 to 100 -- ‘As I sat looking up at the Guinness ad, I could never figure out / How your man stayed up on the surfboard after forty pints of stout’ (C. Moore)