GNU bug report logs - #19208
replace-match unhelpful error message

Previous Next

Full log

View this message in rfc822 format

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: phillip.lord <at> newcastle.ac.uk (Phillip Lord)
Cc: 19208 <at> debbugs.gnu.org
Subject: bug#19208: replace-match unhelpful error message
Date: Sat, 03 Aug 2019 16:01:15 +0200

phillip.lord <at> newcastle.ac.uk (Phillip Lord) writes:

> (progn
>   (goto-char (point-min))
>   (re-search-forward "This")
>   (replace-match "That" nil nil nil 1))
>
> This causes the following error.
>
> progn: Args out of range: -1, -1
>
> The error is caused because I have asked for a subexp that doesn't exist in
> the regexp. A programmer error for sure, but the Args out of range error seems
> rather unfriendly to me, especially as I have no idea where the -1, or
> -1 has come from.
>
> "Attempt to replace regexp subexpression that doesn't exist", for
> example, would be nicer.

Yup.

This is the code that signals the error:

      if (search_regs.start[sub] < BEGV
	  || search_regs.start[sub] > search_regs.end[sub]
	  || search_regs.end[sub] > ZV)
	args_out_of_range (make_fixnum (search_regs.start[sub]),
			   make_fixnum (search_regs.end[sub]));

Which is a weird way to check for this, surely?  It looks more like a
sanity checks that checks whether start[sub] and end[sub] is inside the
buffer, and that the start is before the end.

This happens to actually work here because when there is no match,
start[sub] is -1, which is always less than BEGV.

And this explains where the -1 in the args out of range comes from.

The sanity check seems sensible (because the buffer may have been
narrowed before calling replace-match), so I propose to add an
additional check that actually explicitly checks whether the subexp is
valid.

Hm...  or is this what the check right above it tried to do?

  if (NILP (subexp))
    sub = 0;
  else
    {
      CHECK_FIXNUM (subexp);
      if (! (0 <= XFIXNUM (subexp) && XFIXNUM (subexp) < search_regs.num_regs))
	args_out_of_range (subexp, make_fixnum (search_regs.num_regs));
      sub = XFIXNUM (subexp);
    }

Yoda conditionals and a !...  Let's see...

No, that's just another sanity check -- num_regs is the max allowed
number of sub-matches.  (I've now added some comments to clarify.)

I've now added a new check to check for this situation, and I've used
your suggested error string.

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

Previous Next

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