APPROVE COMMIT NOTE: This patch has been committed # HG changeset patch # User Aidan Kehoe <kehoea(a)parhasard.net&gt; # Date 1538350975 -3600 # Mon Oct 01 00:42:55 2018 +0100 # Node ID 9a9e9993b4125f9223cf1189c5cd4791075ea1e3 # Parent 45b565af61281f58888966bc576f3d0f5c0a50b0 Advise against #'marker-position in its docstring; follow this advice, lisp/ src/ChangeLog addition: 2018-10-01 Aidan Kehoe <kehoea(a)parhasard.net&gt; * marker.c (Fmarker_position): Advise not to use this function in its docstring; give an example of a restricted use case where it is reasonable. lisp/ChangeLog addition: 2018-10-01 Aidan Kehoe <kehoea(a)parhasard.net&gt; * code-files.el (insert-file-contents): * cus-edit.el (custom-redraw): * font-lock.el (font-lock-fontify-syntactically-region): * help.el (Help-princ-face): * help.el (Help-prin1-face): * help.el (help-symbol-regexp): * indent.el (increase-left-margin): * indent.el (decrease-left-margin): * info.el (Info-find-file-node): * info.el (Info-cease-edit): * mouse.el (default-mouse-track-deal-with-down-event): * select.el (select-convert-to-lineno): * select.el (select-convert-to-sourceloc): * simple.el (kill-region): * simple.el (pop-global-mark): * simple.el (signal-error-on-buffer-boundary): * wid-edit.el (widget-setup): * window-xemacs.el (restore-saved-window-parameters): Don't call #'marker-position in the core Lisp code, as we have just advised in its docstring. man/ChangeLog addition: 2018-10-01 Aidan Kehoe <kehoea(a)parhasard.net&gt; * lispref/compile.texi (Different Behavior): Mention the unfortunate algorithmic complexity of #'marker-position when cautioning about the behaviour of (+ MARKER 0). diff -r 45b565af6128 -r 9a9e9993b412 lisp/ChangeLog --- a/lisp/ChangeLog Fri Sep 28 11:00:54 2018 +0100 +++ b/lisp/ChangeLog Mon Oct 01 00:42:55 2018 +0100 ＠＠ -1,3 +1,26 ＠＠ +2018-10-01 Aidan Kehoe <kehoea(a)parhasard.net&gt; + + * code-files.el (insert-file-contents): + * cus-edit.el (custom-redraw): + * font-lock.el (font-lock-fontify-syntactically-region): + * help.el (Help-princ-face): + * help.el (Help-prin1-face): + * help.el (help-symbol-regexp): + * indent.el (increase-left-margin): + * indent.el (decrease-left-margin): + * info.el (Info-find-file-node): + * info.el (Info-cease-edit): + * mouse.el (default-mouse-track-deal-with-down-event): + * select.el (select-convert-to-lineno): + * select.el (select-convert-to-sourceloc): + * simple.el (kill-region): + * simple.el (pop-global-mark): + * simple.el (signal-error-on-buffer-boundary): + * wid-edit.el (widget-setup): + * window-xemacs.el (restore-saved-window-parameters): + Don't call #'marker-position in the core Lisp code, as we have + just advised in its docstring. + 2018-09-28 Aidan Kehoe <kehoea(a)parhasard.net&gt; * byte-optimize.el (byte-optimize-car): diff -r 45b565af6128 -r 9a9e9993b412 lisp/code-files.el --- a/lisp/code-files.el Fri Sep 28 11:00:54 2018 +0100 +++ b/lisp/code-files.el Mon Oct 01 00:42:55 2018 +0100 ＠＠ -536,12 +536,12 ＠＠ ;; convention of post-read-conversion. We try to ;; support the old way. #### Should we kill this? (funcall func (point) (marker-position endmark)) - (funcall func (- (marker-position endmark) (point)))))) + (funcall func (- endmark (point)))))) (if visit (progn (set-buffer-auto-saved) (set-buffer-modified-p nil))))) - (setcar (cdr return-val) (- (marker-position endmark) (point)))) + (setcar (cdr return-val) (- endmark (point)))) ;; now finally set the buffer's `buffer-file-coding-system' ... (if (run-hook-with-args-until-success 'insert-file-contents-post-hook filename visit return-val) diff -r 45b565af6128 -r 9a9e9993b412 lisp/cus-edit.el --- a/lisp/cus-edit.el Fri Sep 28 11:00:54 2018 +0100 +++ b/lisp/cus-edit.el Mon Oct 01 00:42:55 2018 +0100 ＠＠ -1646,8 +1646,10 ＠＠ (let ((line (count-lines (point-min) (point))) (column (current-column)) (pos (point)) - (from (marker-position (widget-get widget :from))) - (to (marker-position (widget-get widget :to)))) + (from (widget-get widget :from)) + (to (widget-get widget :to))) + (check-type from marker) + (check-type to marker) (save-excursion (widget-value-set widget (widget-value widget)) (custom-redraw-magic widget)) diff -r 45b565af6128 -r 9a9e9993b412 lisp/font-lock.el --- a/lisp/font-lock.el Fri Sep 28 11:00:54 2018 +0100 +++ b/lisp/font-lock.el Mon Oct 01 00:42:55 2018 +0100 ＠＠ -1640,18 +1640,18 ＠＠ (goto-char start) (let ((lisp-like (font-lock-lisp-like major-mode)) - (cache (marker-position font-lock-cache-position)) + (cache font-lock-cache-position) state string beg depth) ;; ;; Find the state at the `beginning-of-line' before `start'. - (if (eq start cache) + (if (= start cache) ;; Use the cache for the state of `start'. (setq state font-lock-cache-state) ;; Find the state of `start'. (if (null font-lock-beginning-of-syntax-function) ;; Use the state at the previous cache position, if any, or ;; otherwise calculate from `point-min'. - (if (or (null cache) (< start cache)) + (if (or (null (marker-buffer cache)) (< start cache)) (setq state (parse-partial-sexp (point-min) start)) (setq state (parse-partial-sexp cache start nil nil font-lock-cache-state))) diff -r 45b565af6128 -r 9a9e9993b412 lisp/help.el --- a/lisp/help.el Fri Sep 28 11:00:54 2018 +0100 +++ b/lisp/help.el Mon Oct 01 00:42:55 2018 +0100 ＠＠ -1314,11 +1314,10 ＠＠ (put-nonduplicable-text-property opoint (point standard-output) 'face face standard-output))) ((markerp standard-output) - (let ((buf (marker-buffer standard-output)) - (pos (marker-position standard-output))) + (let ((pos (copy-marker standard-output))) (princ object) (put-nonduplicable-text-property - pos (marker-position standard-output) 'face face buf))) + pos standard-output 'face face (marker-buffer standard-output)))) (t (princ object)))) ;; replacement for `prin1' that puts the text in the specified face, ＠＠ -1330,11 +1329,10 ＠＠ (put-nonduplicable-text-property opoint (point standard-output) 'face face standard-output))) ((markerp standard-output) - (let ((buf (marker-buffer standard-output)) - (pos (marker-position standard-output))) + (let ((pos (copy-marker standard-output))) (prin1 object) (put-nonduplicable-text-property - pos (marker-position standard-output) 'face face buf))) + pos standard-output 'face face (marker-buffer standard-output)))) (t (prin1 object)))) (defvar help-symbol-regexp diff -r 45b565af6128 -r 9a9e9993b412 lisp/indent.el --- a/lisp/indent.el Fri Sep 28 11:00:54 2018 +0100 +++ b/lisp/indent.el Mon Oct 01 00:42:55 2018 +0100 ＠＠ -213,12 +213,12 ＠＠ (if (bolp) (setq from (point))) (goto-char to) (setq to (point-marker))) - (alter-text-property from (marker-position to) 'left-margin ; XEmacs + (alter-text-property from to 'left-margin ; XEmacs (lambda (v) (max (- left-margin) (+ inc (or v 0))))) - (indent-rigidly from (marker-position to) inc) ; XEmacs + (indent-rigidly from to inc) ; XEmacs (if auto-fill-function (save-excursion - (fill-region from (marker-position to) nil t t))) ; XEmacs + (fill-region from to nil t t))) ; XEmacs (move-marker to nil)) (defun decrease-left-margin (from to inc) diff -r 45b565af6128 -r 9a9e9993b412 lisp/info.el --- a/lisp/info.el Fri Sep 28 11:00:54 2018 +0100 +++ b/lisp/info.el Mon Oct 01 00:42:55 2018 +0100 ＠＠ -781,7 +781,7 ＠＠ ;; First get advice from tag table if file has one. ;; Also, if this is an indirect info file, ;; read the proper subfile into this buffer. - (if (marker-position Info-tag-table-marker) + (if (marker-buffer Info-tag-table-marker) (let (foun found-mode (m Info-tag-table-marker)) (save-excursion (set-buffer (marker-buffer Info-tag-table-marker)) ＠＠ -3119,7 +3119,7 ＠＠ (setq buffer-read-only t) ;; Make mode line update. (set-buffer-modified-p (buffer-modified-p)) - (and (marker-position Info-tag-table-marker) + (and (marker-buffer Info-tag-table-marker) (buffer-modified-p) (message "Tags may have changed. Use Info-tagify if necessary")))  diff -r 45b565af6128 -r 9a9e9993b412 lisp/mouse.el --- a/lisp/mouse.el Fri Sep 28 11:00:54 2018 +0100 +++ b/lisp/mouse.el Mon Oct 01 00:42:55 2018 +0100 ＠＠ -1205,7 +1205,7 ＠＠ (markerp default-mouse-track-previous-point) (eq (current-buffer) (marker-buffer default-mouse-track-previous-point))) - (marker-position default-mouse-track-previous-point) + (copy-marker default-mouse-track-previous-point) (point))) (default-mouse-track-set-point event default-mouse-track-window) (if (not adjust) diff -r 45b565af6128 -r 9a9e9993b412 lisp/select.el --- a/lisp/select.el Fri Sep 28 11:00:54 2018 +0100 +++ b/lisp/select.el Mon Oct 01 00:42:55 2018 +0100 ＠＠ -597,8 +597,8 ＠＠ ((and (consp value) (markerp (car value)) (markerp (cdr value))) - (setq a (marker-position (car value)) - b (marker-position (cdr value)) + (setq a (car value) + b (cdr value) buf (marker-buffer (car value))))) (save-excursion (set-buffer buf) ＠＠ -649,8 +649,8 ＠＠ ((and (consp value) (markerp (car value)) (markerp (cdr value))) - (setq a (marker-position (car value)) - b (marker-position (cdr value)) + (setq a (car value) + b (cdr value) buf (or (marker-buffer (car value)) (error "selection is in a killed buffer")) file-name (buffer-file-name buf)))) diff -r 45b565af6128 -r 9a9e9993b412 lisp/simple.el --- a/lisp/simple.el Fri Sep 28 11:00:54 2018 +0100 +++ b/lisp/simple.el Mon Oct 01 00:42:55 2018 +0100 ＠＠ -1465,8 +1465,6 ＠＠ ; (if region-hack (zmacs-deactivate-region))))) ;; start and end can be markers but the rest of this function is ;; written as if they are only integers - (if (markerp start) (setq start (marker-position start))) - (if (markerp end) (setq end (marker-position end))) (or (and start end) (if zmacs-regions ;; rewritten for I18N3 snarfing (error "The region is not active now") (error "The mark is not set now"))) ＠＠ -2018,15 +2016,14 ＠＠ (or global-mark-ring (error "No global mark set")) (let* ((marker (car global-mark-ring)) - (buffer (marker-buffer marker)) - (position (marker-position marker))) + (buffer (marker-buffer marker))) (setq global-mark-ring (nconc (cdr global-mark-ring) (list (car global-mark-ring)))) (set-buffer buffer) - (or (and (>= position (point-min)) - (<= position (point-max))) + (or (and (>= marker (point-min)) + (<= marker (point-max))) (widen)) - (goto-char position) + (goto-char marker) (switch-to-buffer buffer)))  diff -r 45b565af6128 -r 9a9e9993b412 lisp/wid-edit.el --- a/lisp/wid-edit.el Fri Sep 28 11:00:54 2018 +0100 +++ b/lisp/wid-edit.el Mon Oct 01 00:42:55 2018 +0100 ＠＠ -1553,8 +1553,7 ＠＠ widget-field-list (cons field widget-field-list)) (let ((from (car (widget-get field :field-extent))) (to (cdr (widget-get field :field-extent)))) - (widget-specify-field field - (marker-position from) (marker-position to)) + (widget-specify-field field from to) (set-marker from nil) (set-marker to nil)) ;; If the field is placed within the inactive zone, deactivate it. diff -r 45b565af6128 -r 9a9e9993b412 lisp/window-xemacs.el --- a/lisp/window-xemacs.el Fri Sep 28 11:00:54 2018 +0100 +++ b/lisp/window-xemacs.el Mon Oct 01 00:42:55 2018 +0100 ＠＠ -522,13 +522,13 ＠＠ (set-window-buffer window (saved-window-buffer saved-window)) (set-window-start window - (marker-position (saved-window-start-marker saved-window)) + (saved-window-start-marker saved-window) t) (if (markerp (saved-window-point-marker saved-window)) (set-window-point window - (marker-position (saved-window-point-marker saved-window)))) + (saved-window-point-marker saved-window))) (set-marker (mark-marker t buffer) - (marker-position (saved-window-mark-marker saved-window)) + (saved-window-mark-marker saved-window) buffer) (if (not (eq buffer (window-configuration-current-buffer configuration))) (goto-char (window-point window) buffer))))) diff -r 45b565af6128 -r 9a9e9993b412 man/ChangeLog --- a/man/ChangeLog Fri Sep 28 11:00:54 2018 +0100 +++ b/man/ChangeLog Mon Oct 01 00:42:55 2018 +0100 ＠＠ -1,3 +1,10 ＠＠ +2018-10-01 Aidan Kehoe <kehoea(a)parhasard.net&gt; + + * lispref/compile.texi (Different Behavior): + Mention the unfortunate algorithmic complexity of + #'marker-position when cautioning about the behaviour of (+ MARKER + 0). + 2018-09-02 Aidan Kehoe <kehoea(a)parhasard.net&gt; * cl.texi (Time of Evaluation): Use the new-style diff -r 45b565af6128 -r 9a9e9993b412 man/lispref/compile.texi --- a/man/lispref/compile.texi Fri Sep 28 11:00:54 2018 +0100 +++ b/man/lispref/compile.texi Mon Oct 01 00:42:55 2018 +0100 ＠＠ -1097,8 +1097,11 ＠＠ If you're trying to use ＠samp{(+ ＠var{object} 0)} to convert ＠var{object} to integer, consider using an explicit conversion function, which is clearer and guaranteed to work. -Instead of ＠samp{(+ ＠var{marker} 0)}, use ＠samp{(marker-position ＠var{marker})}. Instead of ＠samp{(+ ＠var{char} 0)}, use ＠samp{(char-int ＠var{char})}. +Instead of ＠samp{(+ ＠var{marker} 0)}, you can write +＠samp{(marker-position ＠var{marker})}, but this has its own efficiency +problems. You many not need to convert from a marker at all, since most +of the primitives convert automatically; test your code! ＠end itemize For maximal equivalence between interpreted and compiled code, the diff -r 45b565af6128 -r 9a9e9993b412 src/ChangeLog --- a/src/ChangeLog Fri Sep 28 11:00:54 2018 +0100 +++ b/src/ChangeLog Mon Oct 01 00:42:55 2018 +0100 ＠＠ -1,3 +1,9 ＠＠ +2018-10-01 Aidan Kehoe <kehoea(a)parhasard.net&gt; + + * marker.c (Fmarker_position): + Advise not to use this function in its docstring; give an example + of a restricted use case where it is reasonable. + 2018-09-17 Aidan Kehoe <kehoea(a)parhasard.net&gt; * buffer.c (Fline_number): diff -r 45b565af6128 -r 9a9e9993b412 src/marker.c --- a/src/marker.c Fri Sep 28 11:00:54 2018 +0100 +++ b/src/marker.c Mon Oct 01 00:42:55 2018 +0100 ＠＠ -136,8 +136,22 ＠＠ } DEFUN ("marker-position", Fmarker_position, 1, 1, 0, /* -Return the position MARKER points at, as a character number. +Return the buffer position to which MARKER points, as a fixnum. + Return `nil' if marker doesn't point anywhere. + +Usually there is no need to call this function; if you are using +marker positions in arithmetic or comparing them, markers are +converted automatically as needed. Markers are stored internally as +byte, not character positions, so in the worst case scenario +`marker-position' is O(N) on the (possibly large) position in the +buffer, and much of the time the C code can avoid this entirely given +its knowledge of the relationship between byte and character +positions. + +One of the few use cases for this function is when saving a buffer +offset to an external file, when the integer value does need to be +generated. */ (marker)) { -- ‘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)