bug#24960: 25.1; etags incorrectly indexes quoted Erlang atoms

bug#24960: 25.1; etags incorrectly indexes quoted Erlang atoms

[David Hull]

[-- Attachment #1.1: Type: text/plain, Size: 6019 bytes --]

There is an error in how etags treats quoted function and record names
in Erlang sources.  etags treats the quotes a part of the name, which is
incorrect and prevents xref-find-definitions from working.

In Erlang, the names of functions and records are "atoms".  Here's a
description of an atom, from "Erlang Handbook"
(http://opensource.erlang-solutions.com/erlang-handbook/):

> An atom is a symbolic name, also known as a literal. Atoms begin with
> a lower-case letter, and may contain alphanumeric characters,
> underscores (_) or at-signs (@). Alternatively atoms can be specified
> by enclosing them in single quotes ('), necessary when they start with
> an uppercase character or contain characters other than underscores
> and at-signs.

Note that the quotes around quoted atoms are not part of the name.  That
is, the following two strings represent the same atom: xyzzy 'xyzzy'
Because etags currently treats the quotes as part of the name, an
attempt to look up the name using the unquoted form will fail.  I have
created a patch to fix this bug.

Attached is the patch (etags.patch), an example Erlang source file that
demonstrates the bug (test.erl), a TAGS file generated from the current
etags program (TAGS), and a TAGS file generated from the patched etags
program (TAGS.new).
-- 
David Hull

In GNU Emacs 25.1.1 (x86_64-apple-darwin13.4.0, NS appkit-1265.21 Version
10.9.5 (Build 13F1911))
 of 2016-09-20 built on builder10-9.porkrind.org
Windowing system distributor 'Apple', version 10.3.1504
Configured using:
 'configure --with-ns '--enable-locallisppath=/Library/Application
 Support/Emacs/${version}/site-lisp:/Library/Application
 Support/Emacs/site-lisp' --with-modules'

Configured features:
NOTIFY ACL GNUTLS LIBXML2 ZLIB TOOLKIT_SCROLL_BARS NS MODULES

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: C

Minor modes in effect:
  diff-auto-refine-mode: t
  magit-auto-revert-mode: t
  global-git-commit-mode: t
  dired-omit-mode: t
  async-bytecomp-package-mode: t
  show-paren-mode: t
  global-anzu-mode: t
  anzu-mode: t
  icomplete-mode: t
  dtrt-indent-mode: t
  shell-dirtrack-mode: t
  which-function-mode: t
  override-global-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t
  abbrev-mode: t

Recent messages:
Tramp: Process has finished.
Tramp: Waiting for prompts from remote shell...done
Copying /var/folders/y9/k3f9vqgn73gf64x8ktywlkd10r0222/T/tramp.28509UNF.c
to
/scp:cos6:/home/david.hull/rpmbuild/BUILD/emacs-25.1/lib-src/etags.c...done
Wrote /scp:cos6:/home/david.hull/rpmbuild/BUILD/emacs-25.1/lib-src/etags.c
Loading dired-x...done
Omitting...
(Nothing to omit)
Turning on magit-auto-revert-mode...done
Configuring package magit...done
next-line: End of buffer [2 times]
Quit [4 times]

Load-path shadows:
/Users/david.hull/.emacs.d/elpa/python-mode-20161116.218/python-mode hides
/Users/david.hull/emacs/python-mode
/Users/david.hull/.emacs.d/elpa/tabulated-list-20120406.1351/tabulated-list
hides
/Users/david.hull/Applications/Emacs.app/Contents/Resources/lisp/emacs-lisp/tabulated-list
/Users/david.hull/.emacs.d/elpa/seq-20151121.1017/seq hides
/Users/david.hull/Applications/Emacs.app/Contents/Resources/lisp/emacs-lisp/seq

Features:
(shadow sort mail-extr emacsbug sendmail magit-blame magit-stash
magit-bisect magit-remote magit-commit magit-sequence magit magit-apply
magit-wip magit-log magit-diff smerge-mode diff-mode magit-core
magit-autorevert autorevert filenotify magit-process magit-popup
magit-mode magit-git crm magit-section magit-utils git-commit log-edit
message dired-x dired rfc822 mml mml-sec epg mm-decode mm-bodies
mm-encode mail-parse rfc2231 rfc2047 rfc2045 ietf-drums mailabbrev
mail-utils gmm-utils mailheader pcvs-util with-editor async-bytecomp
async server cc-mode cc-fonts cc-guess cc-menus cc-cmds cc-styles
cc-align cc-engine cc-vars cc-defs tramp-cache markdown-mode noutline
outline pulse jka-compr find-func erldoc debug misearch multi-isearch
add-log ws-butler erlang derived tempo cl paren compile python tramp-sh
anzu thingatpt etags-table etags xref project edmacro kmacro icomplete
dtrt-indent tramp tramp-compat tramp-loaddefs trampver shell pcomplete
comint ansi-color ring format-spec which-func imenu delight advice
use-package diminish bind-key easy-mmode exec-path-from-shell finder-inf
gh-common gh-profile url-parse auth-source gnus-util mm-util help-fns
mail-prsvr password-cache url-vars rx s marshal eieio-compat cl-seq ht
json map dash eieio eieio-core cl-macs thrift-autoloads info package
epg-config seq byte-opt gv bytecomp byte-compile cl-extra help-mode
easymenu cconv cl-loaddefs pcase cl-lib time-date mule-util tooltip
eldoc electric uniquify ediff-hook vc-hooks lisp-float-type mwheel
ns-win ucs-normalize term/common-win tool-bar dnd fontset image
regexp-opt fringe tabulated-list newcomment elisp-mode lisp-mode
prog-mode register page menu-bar rfn-eshadow timer select scroll-bar
mouse jit-lock font-lock syntax facemenu font-core frame cl-generic cham
georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese charscript case-table epa-hook
jka-cmpr-hook help simple abbrev minibuffer cl-preloaded nadvice
loaddefs button faces cus-face macroexp files text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote kqueue cocoa ns multi-tty
make-network-process emacs)

Memory information:
((conses 16 429292 44685)
  (symbols 48 36785 1)
  (miscs 40 793 1229)
  (strings 32 76992 21185)
  (string-bytes 1 2419332)
  (vectors 16 62070)
  (vector-slots 8 1710176 392883)
  (floats 8 578 784)
  (intervals 56 8956 290)
  (buffers 976 41))

[-- Attachment #1.2: Type: text/html, Size: 6768 bytes --]

[-- Attachment #2: etags.patch --]
[-- Type: application/octet-stream, Size: 2854 bytes --]

--- emacs-25.1/lib-src/etags.c.orig	2016-11-18 00:50:45.010888216 +0000
+++ emacs-25.1/lib-src/etags.c	2016-11-18 01:18:25.379498904 +0000
@@ -5803,7 +5803,7 @@ prolog_atom (char *s, size_t pos)
  * Assumes that Erlang functions start at column 0.
  * Original code by Anders Lindgren (1996)
  */
-static int erlang_func (char *, char *);
+static int erlang_func (char *, char *, int *);
 static void erlang_attribute (char *);
 static int erlang_atom (char *);
 
@@ -5813,6 +5813,7 @@ Erlang_functions (FILE *inf)
   char *cp, *last;
   int len;
   int allocated;
+  int name_offset = 0;
 
   allocated = 0;
   len = 0;
@@ -5837,7 +5838,7 @@ Erlang_functions (FILE *inf)
 	      last = NULL;
 	    }
 	}
-      else if ((len = erlang_func (cp, last)) > 0)
+      else if ((len = erlang_func (cp, last, &name_offset)) > 0)
 	{
 	  /*
 	   * Function.  Store the function name so that we only
@@ -5848,7 +5849,7 @@ Erlang_functions (FILE *inf)
 	  else if (len + 1 > allocated)
 	    xrnew (last, len + 1, char);
 	  allocated = len + 1;
-	  memcpy (last, cp, len);
+	  memcpy (last, cp + name_offset, len);
 	  last[len] = '\0';
 	}
     }
@@ -5867,12 +5868,13 @@ Erlang_functions (FILE *inf)
  * was found.
  */
 static int
-erlang_func (char *s, char *last)
+erlang_func (char *s, char *last, int *name_offset)
 
                 		/* Name of last clause. */
 {
   int pos;
   int len;
+  char *name = s;
 
   pos = erlang_atom (s);
   if (pos < 1)
@@ -5881,13 +5883,23 @@ erlang_func (char *s, char *last)
   len = pos;
   pos = skip_spaces (s + pos) - s;
 
+  /* If the name is quoted, the quotes are not part of the name. */
+  if (len > 2 && name[0] == '\'' && name[len - 1] == '\'')
+    {
+      *name_offset = 1;
+      name++;
+      len -= 2;
+    }
+  else
+    *name_offset = 0;
+
   /* Save only the first clause. */
   if (s[pos++] == '('
       && (last == NULL
 	  || len != (int)strlen (last)
-	  || !strneq (s, last, len)))
+	  || !strneq (name, last, len)))
 	{
-	  make_tag (s, len, true, s, pos, lineno, linecharno);
+	  make_tag (name, len, true, s, pos, lineno, linecharno);
 	  return len;
 	}
 
@@ -5908,13 +5920,23 @@ erlang_func (char *s, char *last)
 erlang_attribute (char *s)
 {
   char *cp = s;
+  int pos;
+  int len;
 
   if ((LOOKING_AT (cp, "-define") || LOOKING_AT (cp, "-record"))
       && *cp++ == '(')
     {
-      int len = erlang_atom (skip_spaces (cp));
+      cp = skip_spaces (cp);
+      len = erlang_atom (cp);
+      pos = cp + len - s;
       if (len > 0)
-	make_tag (cp, len, true, s, cp + len - s, lineno, linecharno);
+        /* If the name is quoted, the quotes are not part of the name. */
+        if (len > 2 && cp[0] == '\'' && cp[len - 1] == '\'')
+          {
+            cp++;
+            len -= 2;
+          }
+	make_tag (cp, len, true, s, pos, lineno, linecharno);
     }
   return;
 }

[-- Attachment #3: ChangeLog --]
[-- Type: application/octet-stream, Size: 147 bytes --]

2016-11-17  David Hull <david.hull@openx.com>

	* etags.c (erlang_func, erlang_attribute): Fix tagging of quoted
	Erlang function and macro names.

[-- Attachment #4: test.erl --]
[-- Type: application/octet-stream, Size: 221 bytes --]

-module(test).
-export([ f/0, g/1 ]).

-record(testone, {a, b, c}).
-record('testtwo', {d}).

f () ->
  #'testone'{a = g(true), b = banana, c = 10}.

'g' (true) ->
  #testtwo{d = <<"dog">>};
g (false) ->
  'Hello World'.

[-- Attachment #5: TAGS --]
[-- Type: application/octet-stream, Size: 99 bytes --]

[-- Attachment #6: TAGS.new --]
[-- Type: application/octet-stream, Size: 92 bytes --]

bug#24960: 25.1; etags incorrectly indexes quoted Erlang atoms

[Eli Zaretskii]

> From: David Hull <david.hull@openx.com>
> Date: Thu, 17 Nov 2016 17:52:43 -0800
> 
> Attached is the patch (etags.patch), an example Erlang source file that
> demonstrates the bug (test.erl), a TAGS file generated from the current
> etags program (TAGS), and a TAGS file generated from the patched etags
> program (TAGS.new).

Thanks.

The patch you sent is too large to be accepted without the legal
paperwork.  Would you agree to assign to the FSF copyright for your
changes?  If so, I will send you the legal form for that, and then
your contribution can be accepted after the paperwork is completed.

bug#24960: 25.1; etags incorrectly indexes quoted Erlang atoms

[David Hull]

[-- Attachment #1: Type: text/plain, Size: 438 bytes --]

On Fri, Nov 18, 2016 at 12:27 AM, Eli Zaretskii <eliz@gnu.org> wrote:

> The patch you sent is too large to be accepted without the legal
> paperwork.  Would you agree to assign to the FSF copyright for your
> changes?  If so, I will send you the legal form for that, and then
> your contribution can be accepted after the paperwork is completed.
>

Yes, I would be happy to assign the copyright for my changes to the FSF.
-- 
David Hull

[-- Attachment #2: Type: text/html, Size: 909 bytes --]

bug#24960: 25.1; etags incorrectly indexes quoted Erlang atoms

[Eli Zaretskii]

> From: David Hull <david.hull@openx.com>
> Date: Fri, 18 Nov 2016 08:33:59 -0800
> Cc: 24960@debbugs.gnu.org
> 
> On Fri, Nov 18, 2016 at 12:27 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>  The patch you sent is too large to be accepted without the legal
>  paperwork. Would you agree to assign to the FSF copyright for your
>  changes? If so, I will send you the legal form for that, and then
>  your contribution can be accepted after the paperwork is completed.
> 
> Yes, I would be happy to assign the copyright for my changes to the FSF.

Thanks, I sent the form off-list.

bug#24960: 25.1; etags incorrectly indexes quoted Erlang atoms

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: David Hull <david.hull@openx.com>
>> Date: Fri, 18 Nov 2016 08:33:59 -0800
>> Cc: 24960@debbugs.gnu.org
>> 
>> On Fri, Nov 18, 2016 at 12:27 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> 
>>  The patch you sent is too large to be accepted without the legal
>>  paperwork. Would you agree to assign to the FSF copyright for your
>>  changes? If so, I will send you the legal form for that, and then
>>  your contribution can be accepted after the paperwork is completed.
>> 
>> Yes, I would be happy to assign the copyright for my changes to the FSF.
>
> Thanks, I sent the form off-list.

It looks like the copyright assignment went through, but the patch
wasn't applied.

I had a look at the patch, but I know nothing about either Erlang or
etags.c, so I can't really say whether it's reasonable or not.  It looks
nice, though.  :-)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#24960: 25.1; etags incorrectly indexes quoted Erlang atoms

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: David Hull <david.hull@openx.com>,  24960@debbugs.gnu.org
> Date: Tue, 25 Jun 2019 01:39:10 +0200
> 
> It looks like the copyright assignment went through, but the patch
> wasn't applied.
> 
> I had a look at the patch, but I know nothing about either Erlang or
> etags.c, so I can't really say whether it's reasonable or not.  It looks
> nice, though.  :-)

I think it should go in.

bug#24960: 25.1; etags incorrectly indexes quoted Erlang atoms

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Lars Ingebrigtsen <larsi@gnus.org>
>> Cc: David Hull <david.hull@openx.com>,  24960@debbugs.gnu.org
>> Date: Tue, 25 Jun 2019 01:39:10 +0200
>> 
>> It looks like the copyright assignment went through, but the patch
>> wasn't applied.
>> 
>> I had a look at the patch, but I know nothing about either Erlang or
>> etags.c, so I can't really say whether it's reasonable or not.  It looks
>> nice, though.  :-)
>
> I think it should go in.

There was one problem in the patch:

       if (len > 0)
-	make_tag (cp, len, true, s, cp + len - s, lineno, linecharno);
+        /* If the name is quoted, the quotes are not part of the name. */
+        if (len > 2 && cp[0] == '\'' && cp[len - 1] == '\'')
+          {
+            cp++;
+            len -= 2;
+          }
+	make_tag (cp, len, true, s, pos, lineno, linecharno);

I think what was meant was that the make_tag should be in that "if"
branch, so I slapped a brace pair around the new lines.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#24960: 25.1; etags incorrectly indexes quoted Erlang atoms

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: david.hull@openx.com,  24960@debbugs.gnu.org
> Date: Tue, 25 Jun 2019 19:13:55 +0200
> 
>        if (len > 0)
> -	make_tag (cp, len, true, s, cp + len - s, lineno, linecharno);
> +        /* If the name is quoted, the quotes are not part of the name. */
> +        if (len > 2 && cp[0] == '\'' && cp[len - 1] == '\'')
> +          {
> +            cp++;
> +            len -= 2;
> +          }
> +	make_tag (cp, len, true, s, pos, lineno, linecharno);
> 
> I think what was meant was that the make_tag should be in that "if"
> branch, so I slapped a brace pair around the new lines.

Yes, thanks.

bug#24960: 25.1; etags incorrectly indexes quoted Erlang atoms

[David Hull]

[-- Attachment #1: Type: text/plain, Size: 872 bytes --]

Yes, I agree that my patch should have had the braces you're suggesting.
Thanks for the fix.

On Tue, Jun 25, 2019 at 10:26 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Lars Ingebrigtsen <larsi@gnus.org>
> > Cc: david.hull@openx.com,  24960@debbugs.gnu.org
> > Date: Tue, 25 Jun 2019 19:13:55 +0200
> >
> >        if (len > 0)
> > -     make_tag (cp, len, true, s, cp + len - s, lineno, linecharno);
> > +        /* If the name is quoted, the quotes are not part of the name.
> */
> > +        if (len > 2 && cp[0] == '\'' && cp[len - 1] == '\'')
> > +          {
> > +            cp++;
> > +            len -= 2;
> > +          }
> > +     make_tag (cp, len, true, s, pos, lineno, linecharno);
> >
> > I think what was meant was that the make_tag should be in that "if"
> > branch, so I slapped a brace pair around the new lines.
>
> Yes, thanks.
>


-- 
David Hull

[-- Attachment #2: Type: text/html, Size: 1626 bytes --]