Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

imenu crash when a comment contains 'in' #7844

Closed
vicuna opened this issue Aug 31, 2018 · 7 comments
Closed

imenu crash when a comment contains 'in' #7844

vicuna opened this issue Aug 31, 2018 · 7 comments
Assignees

Comments

@vicuna
Copy link

vicuna commented Aug 31, 2018

Original bug ID: 7844
Reporter: wilfred
Assigned to: @gasche
Status: resolved (set by @gasche on 2018-09-13T12:57:59Z)
Resolution: fixed
Priority: normal
Severity: minor
Fixed in version: 4.08.0+dev/beta1/beta2
Category: emacs mode
Has duplicate: #7689
Monitored by: wyuenho @nojb @gasche

Bug description

imenu in Emacs crashes when a comment contains the word 'in'. This also affects tuareg-mode.

Steps to reproduce

Create an ocaml buffer with the following contents:

?(**

  • in
    *)

And run M-x imenu.

Traceback produced:

Debugger entered--Lisp error: (wrong-type-argument integer-or-marker-p nil)
buffer-substring-no-properties(nil nil)
(if string (substring string begin end) (buffer-substring-no-properties begin end))
(let* ((data (match-data)) (begin (nth (* 2 num) data)) (end (nth (1+ (* 2 num)) data))) (if string (substring string begin end) (buffer-substring-no-properties begin end)))
caml-match-string(5)
(cons (caml-match-string 5) (point))
(setq index (cons (caml-match-string 5) (point)))
(while (caml-prev-index-position-function) (setq index (cons (caml-match-string 5) (point))) "Macro to display a progress message.\nRELPOS is the relative position to display.\nIf RELPOS is nil, then the relative position in the buffer\nis calculated.\nPREVPOS is the variable in which we store the last position displayed." (setq all-alist (cons index all-alist)) (cond ((looking-at "[ \011]*and") (setq and-alist (cons index and-alist))) ((looking-at "[ \011]*let") (setq value-alist (cons index (append and-alist value-alist))) (setq and-alist nil)) ((looking-at "[ \011]*type") (setq type-alist (cons index (append and-alist type-alist))) (setq and-alist nil)) ((looking-at "[ \011]*class") (setq class-alist (cons index (append and-alist class-alist))) (setq and-alist nil)) ((looking-at "[ \011]val") (setq value-alist (cons index value-alist))) ((looking-at "[ \011]\(module\|functor\)") (setq module-alist (cons index module-alist))) ((looking-at "[ \011]*method") (setq method-alist (cons index method-alist)))))
(let (value-alist type-alist class-alist method-alist module-alist and-alist all-alist menu-alist (prev-pos (point-max)) index) (goto-char prev-pos) "Macro to display a progress message.\nRELPOS is the relative position to display.\nIf RELPOS is nil, then the relative position in the buffer\nis calculated.\nPREVPOS is the variable in which we store the last position displayed." (while (caml-prev-index-position-function) (setq index (cons (caml-match-string 5) (point))) "Macro to display a progress message.\nRELPOS is the relative position to display.\nIf RELPOS is nil, then the relative position in the buffer\nis calculated.\nPREVPOS is the variable in which we store the last position displayed." (setq all-alist (cons index all-alist)) (cond ((looking-at "[ \011]*and") (setq and-alist (cons index and-alist))) ((looking-at "[ \011]*let") (setq value-alist (cons index (append and-alist value-alist))) (setq and-alist nil)) ((looking-at "[ \011]*type") (setq type-alist (cons index (append and-alist type-alist))) (setq and-alist nil)) ((looking-at "[ \011]*class") (setq class-alist (cons index (append and-alist class-alist))) (setq and-alist nil)) ((looking-at "[ \011]val") (setq value-alist (cons index value-alist))) ((looking-at "[ \011]\(module\|functor\)") (setq module-alist (cons index module-alist))) ((looking-at "[ \011]*method") (setq method-alist (cons index method-alist))))) (mapc (function (lambda (pair) (if (symbol-value (cdr pair)) (setq menu-alist (cons (cons (car pair) (sort (symbol-value (cdr pair)) 'imenu--sort-by-name)) menu-alist))))) '(("Values" . value-alist) ("Types" . type-alist) ("Modules" . module-alist) ("Methods" . method-alist) ("Classes" . class-alist))) (if all-alist (setq menu-alist (cons (cons "Index" all-alist) menu-alist))) "Macro to display a progress message.\nRELPOS is the relative position to display.\nIf RELPOS is nil, then the relative position in the buffer\nis calculated.\nPREVPOS is the variable in which we store the last position displayed." menu-alist)
tuareg-imenu-create-index()
imenu--make-index-alist()
imenu-choose-buffer-index()
byte-code("\300 C\207" [imenu-choose-buffer-index] 1)
call-interactively(imenu record nil)
command-execute(imenu record)

Note that tuareg-imenu-create-index is an alias for caml-create-index-function.

@vicuna
Copy link
Author

vicuna commented Sep 9, 2018

Comment author: wyuenho

I just ran into this problem too. It's quite annoying.

@vicuna
Copy link
Author

vicuna commented Sep 9, 2018

Comment author: @gasche

Would one of you be willing to debug the issue and propose a fix to the elisp code?

@vicuna
Copy link
Author

vicuna commented Sep 10, 2018

Comment author: wyuenho

Further debugging shows that it's not that there's an "in" in a comment that causes this issue, but that the search doesn't stop when the beginning of the buffer is reached. So the code the is looking at the last regular expression match while expecting data from the current match, in which there isn't any.

@vicuna
Copy link
Author

vicuna commented Sep 11, 2018

Comment author: wilfred

I think the problem is this:

caml-create-index-function calls caml-prev-index-position-function, which is intended to search backwards until it finds the previous definition suitable for imenu.

However, because caml-in-comment-p incorrectly returns nil, we think we've found a match. The bad match data is just a side effect of caml-prev-index-position-function returning a number when it's inside the comment.

@vicuna
Copy link
Author

vicuna commented Sep 11, 2018

Comment author: wilfred

I've opened #2040

@vicuna
Copy link
Author

vicuna commented Sep 11, 2018

Comment author: wyuenho

Ah ha! I'll test it out and report back on Github.

BTW, This bug seems to be a duplicate of #7689.

@vicuna
Copy link
Author

vicuna commented Sep 13, 2018

Comment author: @gasche

This should now be fixed by Wilfred's pull request -- to be part of 4.08.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants