Subject: Re: bug#47408: Etags support for Mercury [v0.5]

From: Eli Zaretskii <[email-masked]>
Date: Thu, 10 Jun 2021 20:20:45 +0300

> Cc: [email-masked]
> From: fabrice nicol <[email-masked]>
> Date: Thu, 10 Jun 2021 18:52:49 +0200
> 
> your latest fix for Mercury 'etags' support has introduced a regression 
> for existentially quantified predicates.

Is it a "regression" in the sense that "M-." no longer finds the
definitions?

> These predicates have the following (somewhat simplified) syntax (in 
> extended regexp  form, \s for white space):
> 
> :-[:blank:]+some[:blank:]*\[[:blank:]*T(,[:blank:]*[:upper:]{1})*[:blank:]*\][:blank:]+pred[:blank:]+([:lower:]+([:alnum:]|[:punct:])*)+[:blank:]*\([^()]+\)([:blank:]|[:lower:])*\.
> 
> Example:
> 
> :- some [T] pred unravel_univ(univ::in, T::out) is det.
> 
> Your fix incorrectly outputs such quantified predicates. For example on 
> tagging univ.m (attached), your commit version yields:
> 
> 
> :- some [T] pred unravel_univ(^?[T] pred unravel_univ^A141,4333
> 
> 
> whilst my original code yields the correct tag:
> 
> 
> :- some [T] pred unravel_univ(^?141,4333

Why do you think the current result is incorrect, while the previous
result was correct?

> In other words, 'pred unravel_univ' is wrongly repeated in your latest 
> commit.

It isn't "wrongly repeated".  TAGS files support 2 different ways of
specifying a tag: implicitly named or explicitly named.  The "repeated
name" form is the latter; it should be used whenever the NAME argument
passed to make_tag includes characters that etags.el doesn't expect to
find in an identifier; see the function notinname and the comments
before make_tag.

It should have been the job of mercury_pr to find the identifier
itself within the line whose pointer it accepts as S, and pass only
that to make_tag as NAME/NAMELEN arguments.  I made a step in that
direction, but it turns out I didn't go far enough.  Feel free to
propose improvements to the code I installed so as to identify the
name of the identifier and nothing else, as other callers of make_tag
do.

> The issue seems to be located at the patch line below:
> 
> 
> + char *name = skip_non_spaces (s + len0);

The only problem with the above line is that it assumes there's only
one non-space "word" before the identifier proper, whereas the example
you show makes it clear there could be more than one.  Which means the
code might need to repeatedly skip these non-identifier words until we
exhaust them all.  I will look into fixing that (but I really prefer
that you do it in my stead, as I don't know enough about the Mercury's
syntax).

But other than that, the changes I installed are IMO a step in the
right direction: your original code incorrectly passed to make_tag the
same arguments as both NAME and LINESTART, and passed zero as NAMELEN,
which was the immediate reason why ctags didn't output anything for
Mercury sources.  Please compare the way you called make_tag with how
the rest of the code calls that function.

> Pending a more accurate 'fix for the fix', it would probably be wiser to 
> revert to original code, as it - at least - gives a correct output for 
> 'etags' invocation.

The original code was incorrect, so it doesn't sound right to me to
revert to it.  I will work on fixing the cases you described (unless
you beat me to it).

Thanks for turning my attention to this issue.