GNU bug report logs - #31350
27.0; `pcase' message: "Redundant pcase pattern"

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 31350 in the body.
You can then email your comments to 31350 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 27.0; `pcase' message: "Redundant pcase pattern"
Date: Wed, 2 May 2018 17:48:35 -0700 (PDT)

I mistakenly had not removed an old version of a pcase clause that I
changed, so one of the two was redundant.  Compiling showed the
redundancy message in *Messages*.

Compiling also showed a message in *Compile Log* saying that a lexical
variable, which occurred only in the second of the clauses with
redundant patterns, was unused.

But the interpreted code worked as I wanted.  Apparently it was the
first of the two clauses (not the second, which had the "unused" lexical
variable) that was ignored when interpreting.  That lexical variable was
used when interpreting.

Can the messaging be improved, to tell you what the effect will be of
the redundant clauses when using the byte-compiled code, versus what it
might be for the interpreted code?  Can the compiler tell a user, for
example, which of a set of clauses with redundancy will (or will not) be
used?

The current messaging seems a bit confusing/incomplete.  It pointed to
the second of the two clauses, which was OK, and which had the "unused"
lexical variable (which was in fact used, at least when interpreting),
saying that it was redundant.

Can the messaging at least tell you (1) all of a set of clauses that are
mutually redundant and (2) which one of them will actually be used by
the compiled code, the others presumably having been pruned?

These are the two clauses in question:

 (`,a `(not,a))
 ((and a (guard (not recursivep))) `(not ,a))

Variable RECURSIVEP is an argument passed to the function that invokes
the `pcase'.  The second clause is the good one.  It is the one that was
flagged as redundant and having an unused lexical variable.  Is it the
clause that gets pruned when compiling?  When interpreting it seems like
the first of the two clauses is not used.

In GNU Emacs 27.0.50 (build 3, x86_64-w64-mingw32)
 of 2018-03-21
Repository revision: e70d0c9e66d7a8609450b2889869d16aeb0363b5
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --without-dbus --host=x86_64-w64-mingw32
 --without-compress-install -C 'CFLAGS=-O2 -static -g3''

Message #8 received at 31350 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 31350 <at> debbugs.gnu.org
Subject: Re: bug#31350: 27.0; `pcase' message: "Redundant pcase pattern"
Date: Thu, 03 May 2018 03:16:25 +0200

Drew Adams <drew.adams <at> oracle.com> writes:

> But the interpreted code worked as I wanted.  Apparently it was the
> first of the two clauses (not the second, which had the "unused" lexical
> variable) that was ignored when interpreting.  That lexical variable was
> used when interpreting.

Are you sure?  I get

(macroexpand-1
 '(pcase foo
    (`,a `(not ,a))
    ((and a (guard (not recursivep))) `(not ,a))))

==>
 (let ((a foo))
  `(not ,a))

and when running the pcase form, the lexical binding of RECURSIVEP is
ignored for me, e.g.

(let ((recursivep t))
  (pcase 1
    (`,a `(not ,a))
    ((and a (guard (not recursivep))) `(not ,a))))
==> (not 1)   (instead of nil)


Michael.

Message #11 received at 31350 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 31350 <at> debbugs.gnu.org
Subject: RE: bug#31350: 27.0; `pcase' message: "Redundant pcase pattern"
Date: Thu, 3 May 2018 11:18:45 -0700 (PDT)

> > But the interpreted code worked as I wanted.  Apparently it was the
> > first of the two clauses (not the second, which had the "unused"
> > lexical variable) that was ignored when interpreting.  That lexical 
> > variable was used when interpreting.
> 
> Are you sure?

No, not at this point.

> I get
> (macroexpand-1
>  '(pcase foo
>     (`,a `(not ,a))
>     ((and a (guard (not recursivep))) `(not ,a))))
> ==>
>  (let ((a foo))
>   `(not ,a))
> 
> and when running the pcase form, the lexical binding of RECURSIVEP is
> ignored for me, e.g.
> 
> (let ((recursivep t))
>   (pcase 1
>     (`,a `(not ,a))
>     ((and a (guard (not recursivep))) `(not ,a))))
> ==> (not 1)   (instead of nil)

OK, thanks.  The rest of the bug report stands, though.

Message #14 received at 31350 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 31350 <at> debbugs.gnu.org
Subject: Re: bug#31350: 27.0; `pcase' message: "Redundant pcase pattern"
Date: Thu, 20 Aug 2020 17:55:02 +0200

Drew Adams <drew.adams <at> oracle.com> writes:

> I mistakenly had not removed an old version of a pcase clause that I
> changed, so one of the two was redundant.  Compiling showed the
> redundancy message in *Messages*.

[...]

> Can the messaging at least tell you (1) all of a set of clauses that are
> mutually redundant and (2) which one of them will actually be used by
> the compiled code, the others presumably having been pruned?
>
> These are the two clauses in question:
>
>  (`,a `(not,a))
>  ((and a (guard (not recursivep))) `(not ,a))

Can you post a complete test case that demonstrates the problem?

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

Message #19 received at 31350 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 31350 <at> debbugs.gnu.org, Drew Adams <drew.adams <at> oracle.com>
Subject: Re: bug#31350: 27.0; `pcase' message: "Redundant pcase pattern"
Date: Sun, 04 Oct 2020 00:52:03 +0200

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> > These are the two clauses in question:
> >
> >  (`,a `(not,a))
> >  ((and a (guard (not recursivep))) `(not ,a))
>
> Can you post a complete test case that demonstrates the problem?

Well, when we compile something like

(defun test (foo recursivep)
  (pcase foo
    (a `(not ,a))
    ((and a (guard recursivep))
     `(not ,a))))

the message Drew means says

  Redundant pcase pattern: (and a (guard recursivep))

I'm happy with that, I think it tells anything I need to know.  I'm not
happy about the fact that I missed the `message' because it's only a
message and I only see it when I look into the *Messages* buffer because
it's overwritten very soon in the echo area.

The compiler log only shows

Compiling file /home/micha/today/pctest.el at Sun Oct  4 00:44:51 2020
pctest.el:46:1: Warning: Unused lexical argument `recursivep'

which is confusing if you missed the message about the redundant
pattern.

Michael.

Message #22 received at 31350 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Michael Heerdegen <michael_heerdegen <at> web.de>, Lars Ingebrigtsen
 <larsi <at> gnus.org>
Cc: 31350 <at> debbugs.gnu.org
Subject: RE: bug#31350: 27.0; `pcase' message: "Redundant pcase pattern"
Date: Sat, 3 Oct 2020 15:58:34 -0700 (PDT)

> Well, when we compile something like
> 
> (defun test (foo recursivep)
>   (pcase foo
>     (a `(not ,a))
>     ((and a (guard recursivep))
>      `(not ,a))))
> 
> the message Drew means says
> 
>   Redundant pcase pattern: (and a (guard recursivep))
> 
> I'm happy with that, I think it tells anything I need to know.  I'm not
> happy about the fact that I missed the `message' because it's only a
> message and I only see it when I look into the *Messages* buffer because
> it's overwritten very soon in the echo area.
> 
> The compiler log only shows
> 
> Compiling file /home/micha/today/pctest.el at Sun Oct  4 00:44:51 2020
> pctest.el:46:1: Warning: Unused lexical argument `recursivep'
> 
> which is confusing if you missed the message about the redundant
> pattern.

The ephemeral (so hidden) message is another problem.

I'll repeat the request, which is what I think a user
would like to know:

  Can the messaging at least tell you:
  (1) all of a set of clauses that are mutually redundant and
  (2) which one of them will actually be used by the compiled
      code, the others presumably having been pruned?

IOW, what's the actual effect, for users?  How does pcase
deal with the redundancy?  Can that at least be documented
somewhere (maybe it is already)?

Message #25 received at 31350 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 31350 <at> debbugs.gnu.org
Subject: Re: bug#31350: 27.0; `pcase' message: "Redundant pcase pattern"
Date: Sun, 04 Oct 2020 03:37:38 +0200

Drew Adams <drew.adams <at> oracle.com> writes:

> I'll repeat the request, which is what I think a user
> would like to know:
>
>   Can the messaging at least tell you:
>   (1) all of a set of clauses that are mutually redundant and
>   (2) which one of them will actually be used by the compiled
>       code, the others presumably having been pruned?
>
> IOW, what's the actual effect, for users?  How does pcase
> deal with the redundancy?  Can that at least be documented
> somewhere (maybe it is already)?

I think you make an error in reasoning: we don't have mutual redundancy
here.  A case can be redundant when it will never match because whenever
would match, a previous case in the cases list always matches, so it is
effectively shadowed.  But this is not symmetric (cases are always tried
from first to last (!), and e.g. a `_' catchall pattern in one case will
make all following cases redundant, but not the other way round).

I think the message is more like a warning that a case can never match,
and in all cases where this happened to me, as also in your case, the
reason was a very obvious editing mistake.  I don't think there is much
to say here, Emacs just tells you: this case here will never be used,
look what you have done wrong.  There is a problem with your code
whenever you see that message.  And there is nothing to say about the
semantics as well.

BTW, I think the implementation only covers the most obvious and
simplistic cases, like those involving catchall patterns or duplicated
patterns.


Michael.

Message #28 received at 31350 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 31350 <at> debbugs.gnu.org
Subject: RE: bug#31350: 27.0; `pcase' message: "Redundant pcase pattern"
Date: Sat, 3 Oct 2020 19:06:46 -0700 (PDT)

> > I'll repeat the request, which is what I think a user
> > would like to know:
> >
> >   Can the messaging at least tell you:
> >   (1) all of a set of clauses that are mutually redundant and
> >   (2) which one of them will actually be used by the compiled
> >       code, the others presumably having been pruned?
> >
> > IOW, what's the actual effect, for users?  How does pcase
> > deal with the redundancy?  Can that at least be documented
> > somewhere (maybe it is already)?
> 
> I think you make an error in reasoning: we don't have mutual redundancy
> here.  A case can be redundant when it will never match because whenever
> would match, a previous case in the cases list always matches, so it is
> effectively shadowed.  But this is not symmetric (cases are always tried
> from first to last (!), and e.g. a `_' catchall pattern in one case will
> make all following cases redundant, but not the other way round).
> 
> I think the message is more like a warning that a case can never match,
> and in all cases where this happened to me, as also in your case, the
> reason was a very obvious editing mistake.  I don't think there is much
> to say here, Emacs just tells you: this case here will never be used,
> look what you have done wrong.  There is a problem with your code
> whenever you see that message.  And there is nothing to say about the
> semantics as well.
> 
> BTW, I think the implementation only covers the most obvious and
> simplistic cases, like those involving catchall patterns or duplicated
> patterns.

If what you say is the case then the help should tell
users that.  To me it's not obvious.

And even if a user (somehow) understands that later
cases are made redundant by earlier ones, how to tell
which earlier ones are implicated?

Message #31 received at 31350 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 31350 <at> debbugs.gnu.org, Drew Adams <drew.adams <at> oracle.com>
Subject: Re: bug#31350: 27.0; `pcase' message: "Redundant pcase pattern"
Date: Sun, 04 Oct 2020 16:03:00 +0200

Michael Heerdegen <michael_heerdegen <at> web.de> writes:

> I think the message is more like a warning that a case can never match,
> and in all cases where this happened to me, as also in your case, the
> reason was a very obvious editing mistake.  I don't think there is much
> to say here, Emacs just tells you: this case here will never be used,
> look what you have done wrong.

Perhaps the warning should be more explicit?  "Redundant pcase pattern"
is obvious if you already know what it means, but if you don't, it's
not.  :-)  So something like "pcase pattern shadowed by previous pcase
pattern" or something along those lines?

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

Message #34 received at 31350 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>, Michael Heerdegen
 <michael_heerdegen <at> web.de>
Cc: 31350 <at> debbugs.gnu.org
Subject: RE: bug#31350: 27.0; `pcase' message: "Redundant pcase pattern"
Date: Sun, 4 Oct 2020 10:45:54 -0700 (PDT)

> Perhaps the warning should be more explicit?  "Redundant pcase pattern"
> is obvious if you already know what it means, but if you don't, it's
> not.  :-)  So something like "pcase pattern shadowed by previous pcase
> pattern" or something along those lines?

That's a start.  But as I said:

  And even if a user (somehow) understands that later
  cases are made redundant by earlier ones, how to tell
  which earlier ones are implicated?

Just saying that some previous pcase pattern makes
this one redundant doesn't tell you which previous
pattern that does that.  It should.

Message #37 received at 31350 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 31350 <at> debbugs.gnu.org, Drew Adams <drew.adams <at> oracle.com>
Subject: Re: bug#31350: 27.0; `pcase' message: "Redundant pcase pattern"
Date: Mon, 05 Oct 2020 01:54:50 +0200

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Perhaps the warning should be more explicit?  "Redundant pcase pattern"
> is obvious if you already know what it means, but if you don't, it's
> not.  :-)  So something like "pcase pattern shadowed by previous pcase
> pattern" or something along those lines?

Yeah, a better wording would be nice.

Michael.

Message #40 received at 31350 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 31350 <at> debbugs.gnu.org
Subject: Re: bug#31350: 27.0; `pcase' message: "Redundant pcase pattern"
Date: Mon, 05 Oct 2020 01:55:51 +0200

Drew Adams <drew.adams <at> oracle.com> writes:

> Just saying that some previous pcase pattern makes
> this one redundant doesn't tell you which previous
> pattern that does that.  It should.

Note that the message is probably just a byproduct produced by the
current implementation, it's nice to have, but it's limited and might
vanish in the future.  I don't know if the the code knows which of the
branches is the shadowing one.  If I have the time I might have a look.

Michael.

Message #43 received at 31350 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 31350 <at> debbugs.gnu.org, Drew Adams <drew.adams <at> oracle.com>
Subject: Re: bug#31350: 27.0; `pcase' message: "Redundant pcase pattern"
Date: Sat, 12 Dec 2020 14:24:27 +0100

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Perhaps the warning should be more explicit?  "Redundant pcase pattern"
> is obvious if you already know what it means, but if you don't, it's
> not.  :-)  So something like "pcase pattern shadowed by previous pcase
> pattern" or something along those lines?

I've now done this in Emacs 28, and I'm closing this bug report.  There
was some talk about changing this warning in other ways, and if that's
something people want to pursue, opening a new bug report for that would
be fine.

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

Message #50 received at 31350 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>, Michael Heerdegen
 <michael_heerdegen <at> web.de>
Cc: 31350 <at> debbugs.gnu.org
Subject: RE: bug#31350: 27.0; `pcase' message: "Redundant pcase pattern"
Date: Sat, 12 Dec 2020 09:00:34 -0800 (PST)

> I've now done this in Emacs 28, and I'm closing this bug report.  There
> was some talk about changing this warning in other ways, and if that's
> something people want to pursue, opening a new bug report for that would
> be fine.

I can't tell just what was fixed and what might not
have been, and the thread is too long to dig into now.

If something was reported that hasn't yet been fixed,
and you're aware of that, it would be great if you
filed the bug report for that.

It's good to keep a record of reports of outstanding
problems, as well as to keep a record of suggested
improvements.

Thx.

Message #53 received at 31350 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: michael_heerdegen <at> web.de, larsi <at> gnus.org, 31350 <at> debbugs.gnu.org
Subject: Re: bug#31350: 27.0; `pcase' message: "Redundant pcase pattern"
Date: Sat, 12 Dec 2020 21:01:01 +0200

> Date: Sat, 12 Dec 2020 09:00:34 -0800 (PST)
> From: Drew Adams <drew.adams <at> oracle.com>
> Cc: 31350 <at> debbugs.gnu.org
> 
> > I've now done this in Emacs 28, and I'm closing this bug report.  There
> > was some talk about changing this warning in other ways, and if that's
> > something people want to pursue, opening a new bug report for that would
> > be fine.
> 
> I can't tell just what was fixed and what might not
> have been

You certainly can, if you look at the changes committed by Lars.  I've
shown the URL for doing that several times in the past.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.