From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Fabrice Nicol Newsgroups: gmane.emacs.bugs Subject: bug#47408: [PATCH] Etags support for Mercury -- fix explicit tags for existentially-quantified procedures Date: Thu, 17 Jun 2021 13:19:26 +0200 Message-ID: References: <46bb9128-8bf5-e24c-2172-1cbb4202ee1d@gmail.com> <83lf7c5t6z.fsf@gnu.org> <46a10b9f-91d7-63c4-1513-0af4743e0940@gmail.com> <83eed45omq.fsf@gnu.org> <83y2b8zrxa.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="00000000000004cc4005c4f4622c" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="28854"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 47408@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Thu Jun 17 13:20:14 2021 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1ltq4D-0007NJ-7p for geb-bug-gnu-emacs@m.gmane-mx.org; Thu, 17 Jun 2021 13:20:13 +0200 Original-Received: from localhost ([::1]:38442 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ltq4C-0006ua-9P for geb-bug-gnu-emacs@m.gmane-mx.org; Thu, 17 Jun 2021 07:20:12 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:48164) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ltq42-0006rz-Au for bug-gnu-emacs@gnu.org; Thu, 17 Jun 2021 07:20:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:42338) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1ltq42-0001IG-2g for bug-gnu-emacs@gnu.org; Thu, 17 Jun 2021 07:20:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1ltq41-0003jR-V2 for bug-gnu-emacs@gnu.org; Thu, 17 Jun 2021 07:20:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Fabrice Nicol Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Thu, 17 Jun 2021 11:20:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 47408 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 47408-submit@debbugs.gnu.org id=B47408.162392878714320 (code B ref 47408); Thu, 17 Jun 2021 11:20:01 +0000 Original-Received: (at 47408) by debbugs.gnu.org; 17 Jun 2021 11:19:47 +0000 Original-Received: from localhost ([127.0.0.1]:53884 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ltq3m-0003it-GV for submit@debbugs.gnu.org; Thu, 17 Jun 2021 07:19:46 -0400 Original-Received: from mail-ej1-f43.google.com ([209.85.218.43]:41953) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ltq3k-0003ie-VH for 47408@debbugs.gnu.org; Thu, 17 Jun 2021 07:19:45 -0400 Original-Received: by mail-ej1-f43.google.com with SMTP id ho18so9230665ejc.8 for <47408@debbugs.gnu.org>; Thu, 17 Jun 2021 04:19:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=eHmwasZ8lvljyXMRgWKzzEQXTcxRTfskWLJvgnt33jk=; b=PeVXon1KXzo8QdIh7aOFgJcKlZpqlSjOfgzTduGYCvlStUkOjVyOK/n7uE7OcRbuPQ KDIHgkU7EvK7kWVyGIz274RSsnlVb4H+6JiIHkGqTw8I5I/S4OWdp5rz8a5BNtdcbksJ 8NMtcLc/8FAibGupBFU9XR3iSpSsDi++W+x4TjbPzeMyREl2WYoHUKhEthKslVTpLqPB EqrgE5mGn50D5N8Ygp79x/BlQ+8kgvbpuPlKakrFXGiyHgwdvpoLyu5dUJnUG3WR6dFP CFjito7wpM8pexfKJ4K0HtRAclyJhM7RSYMAz/txEJgCoZ9wFgu7+qKsgaTEa5GLUXlx DPrw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=eHmwasZ8lvljyXMRgWKzzEQXTcxRTfskWLJvgnt33jk=; b=VgtOM8W5dHU21T3glLRBOeipQ8ad0jCeoztd5Q+M+em9tBuqNqJ8/KJ4VHM93hFeVP fTh/AUA/oMx1njSFObLVlAJTbsrCNi/ey1aqkCGm48vEPDtZa7gh4F1sRae+GYA/3uHB uCimExetzFDRvhX+wXftm1ivsExT4Ca69tNP6kDVyLkMwJ9dtCRThvS0caE2+Iic0SzD bgb6kResTAINFGp4PPiYS4IetFC5NQW21AUSPZp/z9TuFGCiOBzQcmPH/WvT2Lmljrte MKHbBqS0JXd7opJ7+HeJIRnqw43AmEg42a/nzLSErOJHF5eT5ukbD1UpuzqlZnimjXl0 xjKw== X-Gm-Message-State: AOAM533r1KDNdCXCpZD7tI2Fy9ssUeBdHkpySSvkPyiQoXCiHvED03l9 ZPxCqda5MEuTLSj+67bY74hdFqy9iByT2ptgZIM= X-Google-Smtp-Source: ABdhPJzZVflfIa+MDHzA7p6yDgoSWl5G1KMSfK4zfeEXepBOwtx0Vz+VAee0TTEQnExDZWwvISp7sBK6wvK0MN2B6b4= X-Received: by 2002:a17:906:430f:: with SMTP id j15mr4576449ejm.445.1623928779097; Thu, 17 Jun 2021 04:19:39 -0700 (PDT) In-Reply-To: <83y2b8zrxa.fsf@gnu.org> X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:208661 Archived-At: --00000000000004cc4005c4f4622c Content-Type: text/plain; charset="UTF-8" Hi Eli, Thanks. I confirm that this works, but I have 2 follow-up issues with > this patch: > > 1. It adds tags for some identifiers that AFAUI are actually > keywords, and shouldn't be in the TAGS tables. Examples: > "interface" (e.g., on line 146 of accumulator.m) and > "implementation" (e.g., on line 166). I guess this is unintended? > If so, how to fix it? > This is intended. I commented this in the commit message (one-word declarations). I confirm that ':- implementation' and ':-interface' are *formally* declarations in Mercury as all others are. They were not included in the previous version bercause of an incomplete switch. It is quite useful from a practical point of view to add them to the tag base. When interfaces are big (they can reach a couple of thousand lines in real-world programming) it is sometimes useful to have a bookmark to jump to the start of the implementation section and back. I used to create an ad-hoc emacs bookmark for this. Tagging removes the need for this. Simply strike M-. at a blank line an select the interface/implementation tag. In the C family this is the same as striking M-. on an header include declaration and jumping to the header file and back. Some IDEs use F4 for this. Think of Mercury interfaces as C headers. > > > 2. It always produces "explicitly named" tags, which I think is > unnecessary. AFAICT, this is related to the following snippet from > mercury_pr: > > > + /* Left-trim type definitions. */ > > + > > + while (pos > namelength + offset > > + && c_isspace (s[pos - namelength - offset])) > > + --offset; > > + > > + /* There is no need to correct namelength or call notinname. */ > > + s[pos - offset - 1] = '\0'; > > + > > + make_tag (s + pos - namelength - offset, namelength, true, s, > pos, lineno, linecharno); > > + return pos; > > I don't understand why you need to overwrite s[pos - offset -1] > with the null byte: the same effect could be obtained by adjusting > the POS argument passed to make_tag. Also, you in effect chop off > the last character of NAME, but don't adjust NAMELENGTH > accordingly. These factors together cause make_tag to decide that > an explicitly-named tag is in order, because name[namelength-1] is > a null byte, which is rejected as being "not-a-name" character. > > To fix this second issue, I propose the change below, which should > be applied on top of your patches: > > diff --git a/lib-src/etags.c b/lib-src/etags.c > index 370e825..2b0288e 100644 > --- a/lib-src/etags.c > +++ b/lib-src/etags.c > @@ -6585,10 +6585,8 @@ mercury_pr (char *s, char *last, ptrdiff_t lastlen) > && c_isspace (s[pos - namelength - offset])) > --offset; > > - /* There is no need to correct namelength or call notinname. */ > - s[pos - offset - 1] = '\0'; > - > - make_tag (s + pos - namelength - offset, namelength, true, s, pos, > lineno, linecharno); > + make_tag (s + pos - namelength - offset, namelength - 1, true, > + s, pos - offset - 1, lineno, linecharno); > return pos; > } > > I've verified that etags after this change still produces the correct > TAGS file, including for the file univ.m you sent up-thread. > > Do you agree with the changes I propose? If not, could you please > explain what I miss here? > OK, this is another way of achieving an equivalent result. Please leave me until tomorrow to perform more tests so that I can formally confirm that this is fine. Best Fabrice > --00000000000004cc4005c4f4622c Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Eli,


Thanks.=C2=A0 I confirm that this works, but I have 2 follow-up issues with=
this patch:

1. It adds tags for some identifiers that AFAUI are actually
=C2=A0 =C2=A0keywords, and shouldn't be in the TAGS tables.=C2=A0 Examp= les:
=C2=A0 =C2=A0"interface" (e.g., on line 146 of accumulator.m) and=
=C2=A0 =C2=A0"implementation" (e.g., on line 166).=C2=A0 I guess = this is unintended?
=C2=A0 =C2=A0If so, how to fix it?

This is intended. I commented this in the= commit message (one-word declarations).=C2=A0
I con= firm that=C2=A0
':- implementation' and '= ;:-interface' are formally declarations in Mercury as all others= are.=C2=A0
They were not included in the previous v= ersion bercause of an incomplete switch.
It is quite= useful from a practical point of view to add them to the tag base. When in= terfaces are big (they can reach a couple of thousand lines in real-world p= rogramming) it is sometimes useful to have a bookmark to jump to the start = of the implementation section and back. I used to create an ad-hoc emacs bo= okmark for this. Tagging removes the need for this. Simply strike M-. at a = blank line an select the interface/implementation tag.
In the C family this is the same as striking M-. on an header include de= claration and jumping to the header file and back. Some IDEs use F4 for thi= s. Think of Mercury interfaces as C headers.





2. It always produces "explicitly named" tags, which I think is =C2=A0 =C2=A0unnecessary.=C2=A0 AFAICT, this is related to the following sn= ippet from
=C2=A0 =C2=A0mercury_pr:

> +=C2=A0 =C2=A0 =C2=A0 /* Left-trim type definitions.=C2=A0 */
> +
> +=C2=A0 =C2=A0 =C2=A0 while (pos > namelength + offset
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 && c_isspace (s[pos - name= length - offset]))
> +=C2=A0 =C2=A0 =C2=A0--offset;
> +
> +=C2=A0 =C2=A0 =C2=A0 /* There is no need to correct namelength or cal= l notinname.=C2=A0 */
> +=C2=A0 =C2=A0 =C2=A0 s[pos - offset - 1] =3D '\0';
> +
> +=C2=A0 =C2=A0 =C2=A0 make_tag (s + pos - namelength - offset, namelen= gth, true, s, pos, lineno, linecharno);
> +=C2=A0 =C2=A0 =C2=A0 return pos;

=C2=A0 =C2=A0I don't understand why you need to overwrite s[pos - offse= t -1]
=C2=A0 =C2=A0with the null byte: the same effect could be obtained by adjus= ting
=C2=A0 =C2=A0the POS argument passed to make_tag.=C2=A0 Also, you in effect= chop off
=C2=A0 =C2=A0the last character of NAME, but don't adjust NAMELENGTH =C2=A0 =C2=A0accordingly.=C2=A0 These factors together cause make_tag to de= cide that
=C2=A0 =C2=A0an explicitly-named tag is in order, because name[namelength-1= ] is
=C2=A0 =C2=A0a null byte, which is rejected as being "not-a-name"= character.

=C2=A0 =C2=A0To fix this second issue, I propose the change below, which sh= ould
=C2=A0 =C2=A0be applied on top of your patches:

diff --git a/lib-src/etags.c b/lib-src/etags.c
index 370e825..2b0288e 100644
--- a/lib-src/etags.c
+++ b/lib-src/etags.c
@@ -6585,10 +6585,8 @@ mercury_pr (char *s, char *last, ptrdiff_t lastlen)<= br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0&& c_isspace (s[pos= - namelength - offset]))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 --offset;

-=C2=A0 =C2=A0 =C2=A0 /* There is no need to correct namelength or call not= inname.=C2=A0 */
-=C2=A0 =C2=A0 =C2=A0 s[pos - offset - 1] =3D '\0';
-
-=C2=A0 =C2=A0 =C2=A0 make_tag (s + pos - namelength - offset, namelength, = true, s, pos, lineno, linecharno);
+=C2=A0 =C2=A0 =C2=A0 make_tag (s + pos - namelength - offset, namelength -= 1, true,
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0s, pos - offset - 1= , lineno, linecharno);
=C2=A0 =C2=A0 =C2=A0 =C2=A0return pos;
=C2=A0 =C2=A0 =C2=A0}

I've verified that etags after this change still produces the correct TAGS file, including for the file univ.m you sent up-thread.

Do you agree with the changes I propose?=C2=A0 If not, could you please
explain what I miss here?
OK, this is another way of achieving an equivalent= result. Please leave me until tomorrow to perform more tests so that I can= formally confirm that this is fine.
Best
Fabrice
--00000000000004cc4005c4f4622c--