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

ABCL: Missing source position from string buffer evaluation #753

Closed
Enerccio opened this issue Feb 4, 2023 · 4 comments
Closed

ABCL: Missing source position from string buffer evaluation #753

Enerccio opened this issue Feb 4, 2023 · 4 comments

Comments

@Enerccio
Copy link

Enerccio commented Feb 4, 2023

Why is buffer evaluation of function returning function name instead of position when there is position?

???

https://github.com/slime/slime/blob/master/swank/abcl.lisp#L1053

@stassats
Copy link
Member

stassats commented Feb 4, 2023

Don't ask questions, there's nobody to answer them. Pull requests would go further.

@easye
Copy link
Contributor

easye commented Feb 4, 2023

Why is buffer evaluation of function returning function name instead of position when there is position?

Probably left-over from a misunderstood merge with @alanruttenberg's changes.

Can you describe a simple recipe for me to get to such a failure in a SLIME session with ABCL? I can probably fix things from there, or at least understand the problem a little better.

@Enerccio
Copy link
Author

Enerccio commented Feb 4, 2023

Why is buffer evaluation of function returning function name instead of position when there is position?

Probably left-over from a misunderstood merge with @alanruttenberg's changes.

Can you describe a simple recipe for me to get to such a failure in a SLIME session with ABCL? I can probably fix things from there, or at least understand the problem a little better.

Not sure how to do it from slime since I am using swank directly.

I send it message:

(:emacs-rex (swank:load-file "/home/pvan/test/x.cl") ":CL-USER" T 2)

which contains:

(defun buble (a b c)
  (+ a b c))

(defun buble2 (a b c &optional opt xasd &rest boxagur)
  (+ a b c))

(defun mobydick (a b c)
  (+ x y z))

(defun asadasda ()
  (mobydick 1 2 3))

And then when I need to query for information later I get no position despite pos being correct.

‌‌(swank::find-definition-for-thing 'mobydick)
 
(:LOCATION (:FILE \"/home/pvan/test/x.cl\") (:FUNCTION-NAME \"MOBYDICK\") (:ALIGN T))

I fixed it myself in a an override function

(defun slime-location-from-source-annotation (sym it)
  (destructuring-bind (what path pos) it

    (let* ((isfunction
            ;; all of these are (defxxx forms, which is what :function locations look for in slime
            (and (consp what) (member (car what)
                                      '(:function :generic-function :macro :class :compiler-macro
                                        :type :constant :variable :package :structure :condition))))
           (ismethod (and (consp what) (eq (car what) :method)))
           (<position> (cond (isfunction (list :function-name (princ-to-string (second what)) :position (1+ (or pos 0))))
                                             (ismethod (stringify-method-specs what) :position (1+ (or pos 0)))
                                             (t (list :position (1+ (or pos 0))))))
           (path2 (if (eq path :top-level)
                      ;; this is bogus - figure out some way to guess which is the repl associated with :toplevel
                      ;; or get rid of this
                      "emacs-buffer:*slime-repl*"
                      (maybe-redirect-to-jar path))))
      (when (atom what)
        (setq what (list what sym)))
      (list (definition-specifier what)
            (if (ext:pathname-jar-p (pathname path2))
                `(:location
                  (:zip ,@(split-string (subseq path2 (length "jar:file:")) "!/"))
                  ;; pos never seems right. Use function name.
                  ,<position>
                  (:align t))
                ;; conspire with swank-compile-string to keep the
                ;; buffer name in a pathname whose device is
                ;; "emacs-buffer".
                  (if (eql 0 (search "emacs-buffer:" path2))
                      `(:location
                        (:buffer ,(subseq path2  (load-time-value (length "emacs-buffer:"))))
                        ,<position>
                        (:align t))
                      `(:location
                        (:file ,path2)
                        ,<position>
                        (:align t))))))))

And I get correct result:

‌‌(swank::find-definition-for-thing 'mobydick)
 
(:LOCATION (:FILE \"/home/pvan/test/x.cl\") (:FUNCTION-NAME \"MOBYDICK\" :POSITION 104) (:ALIGN T))

easye added a commit to easye/slime that referenced this issue Feb 5, 2023
Merge slime-location-from-source-annotation patch from
<https://github.com/Enerccio>.

Addresses <slime#753>
easye added a commit to easye/slime that referenced this issue Feb 5, 2023
Merge re-worked slime-location-from-source-annotation patch from
<https://github.com/Enerccio>.

slime.el documents the grammar for location to always be a four item
list:

      <location> ::= (:location <buffer> <position> <hints>)
                   | (:error <message>)

      <buffer>   ::= (:file <filename>)
                   | (:buffer <buffername>)
                   | (:buffer-and-file <buffername> <filename>)
                   | (:source-form <string>)
                   | (:zip <file> <entry>)

      <position> ::= (:position <fixnum>) ; 1 based (for files)
                   | (:offset <start> <offset>) ; start+offset (for C-c C-c)
                   | (:line <line> [<column>])
                   | (:function-name <string>)
                   | (:source-path <list> <start-position>)
                   | (:method <name string> <specializers>
                   . <qualifiers>)

Addresses <slime#753>.
easye added a commit to easye/slime that referenced this issue Feb 5, 2023
Implement FIND-SOURCE-LOCATION.

Merged re-worked slime-location-from-source-annotation patch from
<https://github.com/Enerccio>.

slime.el documents the grammar for location to always be a four item
list:

      <location> ::= (:location <buffer> <position> <hints>)
                   | (:error <message>)

      <buffer>   ::= (:file <filename>)
                   | (:buffer <buffername>)
                   | (:buffer-and-file <buffername> <filename>)
                   | (:source-form <string>)
                   | (:zip <file> <entry>)

      <position> ::= (:position <fixnum>) ; 1 based (for files)
                   | (:offset <start> <offset>) ; start+offset (for C-c C-c)
                   | (:line <line> [<column>])
                   | (:function-name <string>)
                   | (:source-path <list> <start-position>)
                   | (:method <name string> <specializers> . <qualifiers>)

Addresses <slime#753>.
easye added a commit that referenced this issue Feb 5, 2023
Implement FIND-SOURCE-LOCATION.

Merged re-worked slime-location-from-source-annotation patch from
<https://github.com/Enerccio>.

slime.el documents the grammar for location to always be a four item
list:

      <location> ::= (:location <buffer> <position> <hints>)
                   | (:error <message>)

      <buffer>   ::= (:file <filename>)
                   | (:buffer <buffername>)
                   | (:buffer-and-file <buffername> <filename>)
                   | (:source-form <string>)
                   | (:zip <file> <entry>)

      <position> ::= (:position <fixnum>) ; 1 based (for files)
                   | (:offset <start> <offset>) ; start+offset (for C-c C-c)
                   | (:line <line> [<column>])
                   | (:function-name <string>)
                   | (:source-path <list> <start-position>)
                   | (:method <name string> <specializers> . <qualifiers>)

Addresses <#753>.
@easye
Copy link
Contributor

easye commented Feb 5, 2023

@Enerccio your problems should be addressed with #755.

According to the grammar documented in https://github.com/slime/slime/blob/master/slime.el#L3349, the position should be specified by a single clause. We now privilege the numerical position if it is available.

In addition, the SLIME interface for FIND-SOURCE-LOCATION wasn't implemented for ABCL.

@easye easye closed this as completed Feb 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants