ocaml / tuareg

Emacs OCaml mode
GNU General Public License v3.0
360 stars 79 forks source link

Bad interplay with tuareg--pattern-pre-form-let and ggtags-fontify-code #223

Closed nverno closed 4 years ago

nverno commented 4 years ago

tuareg--pattern-pre-form-let attempts to add a text property to a point that is potentially out-of-bounds here

This point can be off buffer in cases like ggtags-fontify-code , eg.

(with-temp-buffer
        (insert code)
        (funcall mode)
        (font-lock-ensure) ;; tuareg--pattern-matcher-limit is at eob
        (buffer-string))
monnier commented 4 years ago

tuareg--pattern-pre-form-let attempts to add a text property to a point that is potentially out-of-bounds

Hmm... I don't see how that can happen here (nitpick: the line you point to "removes" a text property, fundamentally, even though technically it does use put-text-property).

This point can be off buffer in cases like ggtags-fontify-code , eg.

(with-temp-buffer
        (insert code)
        (funcall mode)
        (font-lock-ensure) ;; tuareg--pattern-matcher-limit is at eob
        (buffer-string))

I don't see how/why this code makes tuareg--pattern-matcher-limit be out of bounds. I'd tend to assume that the problem will only show up for some particular values of code above.

Do you have some more precise recipe, or a backtrace (hopefully together with the content of the tuareg-mode buffer at that moment)?

monnier commented 4 years ago

Do you have some more precise recipe, or a backtrace (hopefully together with the content of the tuareg-mode buffer at that moment)?

Or, based on your patch in #224, could you try the alternative patch below, see if it fixes your problem as well?

diff --git a/tuareg.el b/tuareg.el
index ca5b538..760319d 100644
--- a/tuareg.el
+++ b/tuareg.el
@@ -1211,7 +1209,8 @@ This based on the fontification and is faster than calling `syntax-ppss'."
       (unless tuareg--pattern-matcher-limit
         (setq tuareg--pattern-matcher-limit (point)))
       ;; Remove any possible highlithing on "="
-      (put-text-property (point) (1+ (point)) 'face nil)
+      (unless (eobp)
+        (put-text-property (point) (1+ (point)) 'face nil))
       ;; move the point back for the sub-matcher
       (goto-char opoint))
     (put-text-property (point) tuareg--pattern-matcher-limit
nverno commented 4 years ago

Without the patch I pushed, or your simpler one, I see the following quite often

Debugger entered--Lisp error: (args-out-of-range 51 52)
  put-text-property(51 52 face nil)
  (let* ((opoint (point)) (limit (+ opoint 800)) pos) (setq tuareg--pattern-matcher-limit nil) (while (and (setq pos (re-search-forward "[=({:]" limit t)) (progn (backward-char) (cond ((or (char-equal 40 ...) (char-equal 123 ...)) (if (ignore-errors ...) t (goto-char ...) nil)) ((char-equal 58 (char-after)) (skip-chars-backward "a-zA-Z0-9_'") (if (not ...) (setq tuareg--pattern-matcher-limit ...)) (goto-char pos) t) (t nil))))) (setq tuareg--pattern-matcher-type-limit (1+ (point))) (unless tuareg--pattern-matcher-limit (setq tuareg--pattern-matcher-limit (point))) (put-text-property (point) (1+ (point)) 'face nil) (goto-char opoint))
  (if (or (tuareg--font-lock-in-string-or-comment) (looking-at "[ \11\n]*open\\_>") (looking-at "[ \11\n]*exception\\_>")) (progn (setq tuareg--pattern-matcher-limit (point)) (setq tuareg--pattern-matcher-type-limit (point))) (let* ((opoint (point)) (limit (+ opoint 800)) pos) (setq tuareg--pattern-matcher-limit nil) (while (and (setq pos (re-search-forward "[=({:]" limit t)) (progn (backward-char) (cond ((or ... ...) (if ... t ... nil)) ((char-equal 58 ...) (skip-chars-backward "a-zA-Z0-9_'") (if ... ...) (goto-char pos) t) (t nil))))) (setq tuareg--pattern-matcher-type-limit (1+ (point))) (unless tuareg--pattern-matcher-limit (setq tuareg--pattern-matcher-limit (point))) (put-text-property (point) (1+ (point)) 'face nil) (goto-char opoint)) (put-text-property (point) tuareg--pattern-matcher-limit 'font-lock-multiline t) tuareg--pattern-matcher-limit)
  tuareg--pattern-pre-form-let()
  eval((tuareg--pattern-pre-form-let))
  font-lock-fontify-keywords-region(1 51 nil)
  font-lock-default-fontify-region(1 51 nil)
  font-lock-fontify-region(1 51)
  #f(compiled-function (beg end) #<bytecode 0x1fdcd571782d>)(1 51)
  font-lock-ensure()
  ggtags-fontify-code("let fold_fdecl (f_param : 'a -> uid * Ll.ty -> 'a)")

As for a precise recipe, I haven't tried to reproduce from emacs -Q, but I believe simply enabling ggtags-mode in a tuareg buffer would reproduce it.

nverno commented 4 years ago

Here is a recipe -- I see the above error simply evaluating

(let ((code "let fold_fdecl (f_param : 'a -> uid * Ll.ty -> 'a)"))
  (with-temp-buffer
    (insert code)
    (funcall 'tuareg-mode)
    (font-lock-ensure) ;; tuareg--pattern-matcher-limit is at eob
    (buffer-string)))
monnier commented 4 years ago

Without the patch I pushed, or your simpler one, I see the following quite often

Thanks, as well as for the good recipe.

I installed my patch, which I believe should fix this problem.

Chris00 commented 4 years ago

Thanks for reporting/working on this.