bug#61395: 28.2; bug-reference warning seems incorrect

bug#61395: 28.2; bug-reference warning seems incorrect

[Tom Tromey]

I got this warning while using bug-reference-prog-mode on a certain file
in gdb:

Warning (bug-reference): The value of ‘bug-reference-bug-regexp’

  "\\(\\b\\(?:[Bb]ug ?#?\\|[Pp]atch ?#\\|RFE ?#\\|PR [a-z+-]+/\\)\\([0-9]+\\(?:#[0-9]+\\)?\\)\\)"

in buffer "finish.exp" doesn’t conform to the contract specified by its
docstring.  The subexpression 1 should define the region of the
bug-reference overlay and cover all other subexpressions up to
subexpression 10. Disable showing Disable logging


However, this warning seems wrong to me.  If you examine the above
regexp, you'll see it does cover all the other subexpressions.

Perhaps it is mis-worded somehow and the actual problem is something
else?  It's worth noting here that this is the default value for this
regexp:

(defcustom bug-reference-bug-regexp
  "\\(\\b\\(?:[Bb]ug ?#?\\|[Pp]atch ?#\\|RFE ?#\\|PR [a-z+-]+/\\)\\([0-9]+\\(?:#[0-9]+\\)?\\)\\)"

In gdb we use this setting for bug-reference-url-format, in case this
matters somehow:

(bug-reference-url-format . "http://sourceware.org/bugzilla/show_bug.cgi?id=%s")



In GNU Emacs 28.2 (build 1, x86_64-redhat-linux-gnu, GTK+ Version 3.24.35, cairo version 1.17.6)
 of 2022-12-31 built on buildvm-x86-18.iad2.fedoraproject.org
Windowing system distributor 'The X.Org Foundation', version 11.0.12201007
System Description: Fedora Linux 36 (Workstation Edition)

Configured using:
 'configure --build=x86_64-redhat-linux-gnu
 --host=x86_64-redhat-linux-gnu --program-prefix=
 --disable-dependency-tracking --prefix=/usr --exec-prefix=/usr
 --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc
 --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib64
 --libexecdir=/usr/libexec --localstatedir=/var
 --sharedstatedir=/var/lib --mandir=/usr/share/man
 --infodir=/usr/share/info --with-dbus --with-gif --with-jpeg --with-png
 --with-rsvg --with-tiff --with-xpm --with-x-toolkit=gtk3 --with-gpm=no
 --with-xwidgets --with-modules --with-harfbuzz --with-cairo --with-json
 --with-native-compilation build_alias=x86_64-redhat-linux-gnu
 host_alias=x86_64-redhat-linux-gnu CC=gcc 'CFLAGS=-DMAIL_USE_LOCKF -O2
 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches
 -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2
 -Wp,-D_GLIBCXX_ASSERTIONS
 -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong
 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic
 -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection'
 LDFLAGS=-Wl,-z,relro
 PKG_CONFIG_PATH=:/usr/lib64/pkgconfig:/usr/share/pkgconfig'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON
LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NATIVE_COMP NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS
X11 XDBE XIM XPM XWIDGETS GTK3 ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Tcl

Minor modes in effect:
  ggtags-mode: t
  bug-reference-prog-mode: t
  flyspell-mode: t
  shell-dirtrack-mode: t
  which-function-mode: t
  erc-services-mode: t
  erc-networks-mode: t
  savehist-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-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
  column-number-mode: t
  line-number-mode: t
  auto-fill-function: do-auto-fill
  indent-tabs-mode: t
  transient-mark-mode: t

Load-path shadows:
/home/tromey/.emacs.d/elpa/bubbles-0.5/bubbles hides /usr/share/emacs/28.2/lisp/play/bubbles
/home/tromey/.emacs.d/elpa/dictionary-1.10/dictionary hides /usr/share/emacs/28.2/lisp/net/dictionary

Features:
(shadow emacsbug rust-mode tabify man sh-script executable compare-w
vc-annotate forth-interaction-mode forth-mode forth-spec forth-smie smie
forth-syntax ert debug backtrace autoload tar-mode lisp-mnt url-http
url-gw url-auth gud gnus-icalendar org-capture org-refile icalendar
cus-edit cus-start cl-print shortdoc asm-mode apropos reposition
conf-mode autoconf autoconf-mode texinfo texinfo-loaddefs m4-mode
make-mode mule-util dired-aux arc-mode archive-mode flow-fill gnus-html
help-fns radix-tree url-cache tcl macros org-bullets org-element
avl-tree ol-eww eww xdg url-queue mm-url ol-rmail ol-mhe ol-irc ol-info
ol-gnus nnselect gnus-search eieio-opt speedbar ezimage dframe
ol-docview doc-view jka-compr image-mode exif ol-bibtex ol-bbdb ol-w3m
ol-doi org-link-doi org ob ob-tangle ob-ref ob-lob ob-table ob-exp
org-macro org-footnote org-src ob-comint org-pcomplete org-list
org-faces org-entities noutline outline org-version ob-emacs-lisp
ob-core ob-eval org-table oc-basic bibtex ol org-keys oc org-compat
org-macs org-loaddefs find-func mailalias supercite regi mail-hist sort
gnus-cite smiley shr-color mm-archive mail-extr gnus-bcklg gnus-async qp
gnus-ml disp-table gnus-topic nndraft nnmh nnfolder utf-7 gnutls
network-stream nsm gnus-agent gnus-srvr gnus-score score-mode nnvirtual
gnus-msg gnus-art mm-uu mml2015 mm-view mml-smime smime dig nntp
gnus-cache gnus-sum shr kinsoku svg dom gnus-group gnus-undo smtpmail
sendmail gnus-start gnus-dbus dbus gnus-cloud nnimap nnmail mail-source
utf7 netrc nnoo gnus-spec gnus-int gnus-range gnus-win gnus nnheader
term/xterm xterm python tramp-sh find-file find-dired ffap dabbrev pulse
log-view goto-addr log-edit message rmc puny dired dired-loaddefs rfc822
mml mml-sec epa epg rfc6068 epg-config gnus-util rmail rmail-loaddefs
mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045 mm-util
ietf-drums mail-prsvr mailabbrev mail-utils gmm-utils mailheader
pcvs-util smerge-mode diff copyright misearch multi-isearch ggtags
hippie-exp derived etags fileloop generator xref project bug-reference
cc-mode cc-fonts cc-guess cc-menus cc-cmds vc-mtn vc-hg vc-git diff-mode
vc-bzr vc-src vc-sccs vc-svn vc-cvs vc-rcs add-log flyspell ispell
diminish projectile ibuf-macs pcase edmacro kmacro grep compile
text-property-search ibuf-ext ibuffer ibuffer-loaddefs dash appt
diary-lib diary-loaddefs cal-menu calendar cal-loaddefs tramp
tramp-loaddefs trampver tramp-integration files-x tramp-compat shell
pcomplete parse-time ls-lisp which-func imenu minimap autorevert
filenotify cus-load erc-track erc-match erc-services erc-networks
erc-hl-nicks easy-mmode color erc-button erc-fill erc-stamp wid-edit
erc-goodies erc erc-backend iso8601 time-date thingatpt pp format-spec
erc-loaddefs comp comp-cstr rx cl-extra help-mode warnings advice vc-dir
ewoc vc vc-dispatcher cc-styles cc-align cc-engine cc-vars cc-defs
ange-ftp comint ansi-color ring server savehist clang-rename
clang-include-fixer let-alist clang-format xml finder-inf
gdb-shell-autoloads lisppaste-autoloads pydoc-info-autoloads info-look
info cl weblogger-autoloads package browse-url url url-proxy url-privacy
url-expand url-methods url-history url-cookie url-domsuf url-util
mailcap url-handlers url-parse auth-source cl-seq eieio eieio-core
cl-macs eieio-loaddefs password-cache json subr-x map url-vars seq
byte-opt gv bytecomp byte-compile cconv cl-loaddefs cl-lib iso-transl
tooltip eldoc paren electric uniquify ediff-hook vc-hooks
lisp-float-type elisp-mode mwheel term/x-win x-win term/common-win x-dnd
tool-bar dnd fontset image regexp-opt fringe tabulated-list replace
newcomment text-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock
font-lock syntax font-core term/tty-colors frame minibuffer 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 composite emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice button loaddefs faces cus-face macroexp files
window text-properties overlay sha1 md5 base64 format env code-pages
mule custom widget hashtable-print-readable backquote threads
xwidget-internal dbusbind inotify dynamic-setting system-font-setting
font-render-setting cairo move-toolbar gtk x-toolkit x multi-tty
make-network-process native-compile emacs)

Memory information:
((conses 16 2993058 272550)
 (symbols 48 88842 38)
 (strings 32 206336 32085)
 (string-bytes 1 11250062)
 (vectors 16 120474)
 (vector-slots 8 3571778 468305)
 (floats 8 573 733)
 (intervals 56 431859 1461)
 (buffers 992 1145))

bug#61395: 28.2; bug-reference warning seems incorrect

[Eli Zaretskii]

> From: Tom Tromey <tom@tromey.com>
> Date: Thu, 09 Feb 2023 12:08:39 -0700
> 
> 
> I got this warning while using bug-reference-prog-mode on a certain file
> in gdb:
> 
> Warning (bug-reference): The value of ‘bug-reference-bug-regexp’
> 
>   "\\(\\b\\(?:[Bb]ug ?#?\\|[Pp]atch ?#\\|RFE ?#\\|PR [a-z+-]+/\\)\\([0-9]+\\(?:#[0-9]+\\)?\\)\\)"
> 
> in buffer "finish.exp" doesn’t conform to the contract specified by its
> docstring.  The subexpression 1 should define the region of the
> bug-reference overlay and cover all other subexpressions up to
> subexpression 10. Disable showing Disable logging
> 
> 
> However, this warning seems wrong to me.  If you examine the above
> regexp, you'll see it does cover all the other subexpressions.
> 
> Perhaps it is mis-worded somehow and the actual problem is something
> else?  It's worth noting here that this is the default value for this
> regexp:
> 
> (defcustom bug-reference-bug-regexp
>   "\\(\\b\\(?:[Bb]ug ?#?\\|[Pp]atch ?#\\|RFE ?#\\|PR [a-z+-]+/\\)\\([0-9]+\\(?:#[0-9]+\\)?\\)\\)"
> 
> In gdb we use this setting for bug-reference-url-format, in case this
> matters somehow:
> 
> (bug-reference-url-format . "http://sourceware.org/bugzilla/show_bug.cgi?id=%s")

Thanks, but could you perhaps post a recipe for reproducing the
warning?  Based on your description, I couldn't understand how to
reproduce it: which variables to change (with the single exception of
bug-reference-url-format), and what should be the buffer text to
trigger the warning.  Could you help me reproducing the problem, so I
could investigate it and see how to fix it?

bug#61395: 28.2; bug-reference warning seems incorrect

[Basil Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii [2023-02-10 10:25 +0200] wrote:

>> From: Tom Tromey <tom@tromey.com>
>> Date: Thu, 09 Feb 2023 12:08:39 -0700
>> 
>> I got this warning while using bug-reference-prog-mode on a certain file
>> in gdb:
>> 
>> Warning (bug-reference): The value of ‘bug-reference-bug-regexp’
>> 
>>   "\\(\\b\\(?:[Bb]ug ?#?\\|[Pp]atch ?#\\|RFE ?#\\|PR [a-z+-]+/\\)\\([0-9]+\\(?:#[0-9]+\\)?\\)\\)"
>> 
>> in buffer "finish.exp" doesn’t conform to the contract specified by its
>> docstring.  The subexpression 1 should define the region of the
>> bug-reference overlay and cover all other subexpressions up to
>> subexpression 10. Disable showing Disable logging
>> 
>> 
>> However, this warning seems wrong to me.  If you examine the above
>> regexp, you'll see it does cover all the other subexpressions.
>> 
>> Perhaps it is mis-worded somehow and the actual problem is something
>> else?  It's worth noting here that this is the default value for this
>> regexp:
>> 
>> (defcustom bug-reference-bug-regexp
>>   "\\(\\b\\(?:[Bb]ug ?#?\\|[Pp]atch ?#\\|RFE ?#\\|PR [a-z+-]+/\\)\\([0-9]+\\(?:#[0-9]+\\)?\\)\\)"
>> 
>> In gdb we use this setting for bug-reference-url-format, in case this
>> matters somehow:
>> 
>> (bug-reference-url-format . "http://sourceware.org/bugzilla/show_bug.cgi?id=%s")
>
> Thanks, but could you perhaps post a recipe for reproducing the
> warning?  Based on your description, I couldn't understand how to
> reproduce it: which variables to change (with the single exception of
> bug-reference-url-format), and what should be the buffer text to
> trigger the warning.  Could you help me reproducing the problem, so I
> could investigate it and see how to fix it?

Copying in Tassilo who worked on this stuff ahead of Emacs 28.

Thanks,

-- 
Basil

bug#61395: 28.2; bug-reference warning seems incorrect

[Tassilo Horn]

Basil Contovounesios <contovob@tcd.ie> writes:

> Copying in Tassilo who worked on this stuff ahead of Emacs 28.

I'll have a look later today!

Bye,
Tassilo

bug#61395: 28.2; bug-reference warning seems incorrect

[Tom Tromey]

> I'll have a look later today!

It seems difficult to reproduce.  I can't do it reliably.

My co-worker Kévin suggested that perhaps the problem is here, in
bug-reference-fontify:

      (while (and (< (point) end-line)
                  (re-search-forward bug-reference-bug-regexp end-line 'move))
        (when (or (not bug-reference-prog-mode)
                  ;; This tests for both comment and string syntax.
                  (nth 8 (syntax-ppss)))

... with the question being whether the call to syntax-ppss needs to be
wrapped in save-match-data.

Tom

bug#61395: 28.2; bug-reference warning seems incorrect

[Tom Tromey]

Eli> Thanks, but could you perhaps post a recipe for reproducing the
Eli> warning?  Based on your description, I couldn't understand how to
Eli> reproduce it: which variables to change (with the single exception of
Eli> bug-reference-url-format), and what should be the buffer text to
Eli> trigger the warning.  Could you help me reproducing the problem, so I
Eli> could investigate it and see how to fix it?

I tried reproducing it by visiting various .exp files in gdb, but I
can't see the warning again.  Is a given warning only issued once per
emacs session?

Anyway, nothing too special was required.  Check out gdb and search for
"PR" underneath gdb/testsuite/.  gdb has a .dir-locals.el that sets the
appropriate variable.  Make sure bug-reference-prog-mode is enabled.

See my other note for a theory of the bug.  This theory would also
explain why it's "intermittent" -- it depends on how much of the buffer
has already been scanned.

Tom

bug#61395: 28.2; bug-reference warning seems incorrect

[Tassilo Horn]

Tom Tromey <tom@tromey.com> writes:

>> I'll have a look later today!
>
> It seems difficult to reproduce.  I can't do it reliably.
>
> My co-worker Kévin suggested that perhaps the problem is here, in
> bug-reference-fontify:
>
>       (while (and (< (point) end-line)
>                   (re-search-forward bug-reference-bug-regexp end-line 'move))
>         (when (or (not bug-reference-prog-mode)
>                   ;; This tests for both comment and string syntax.
>                   (nth 8 (syntax-ppss)))
>
> ... with the question being whether the call to syntax-ppss needs to
> be wrapped in save-match-data.

Hm, I've tried somthing like

  (while (re-search-forward "\\(NILP\\)\\(.*\\)$" nil t)
    (syntax-ppss-flush-cache (point-min))
    (let ((md (match-data)))
      (syntax-ppss)
      (unless (equal md (match-data))
        (error "Match-data clobbered: %S %S" md (match-data)))))

many times in buffer.c but `syntax-ppss' doesn't seem to clobber
match-data.

If you can reproduce the issue, try changing
`bug-reference--overlay-bounds' so that the check

  (member bug-reference-bug-regexp
          bug-reference--nonconforming-regexps)

isn't performed inhibiting the warning for this regexp and add some more
info to the warning message like (match-string N) for N in 0..9.  If the
match-data isn't about matches of `bug-reference-bug-regexp', we are on
the right track, i.e., someone clobbers our match data.

Bye,
Tassilo

bug#61395: 28.2; bug-reference warning seems incorrect

[Tom Tromey]

Tassilo> many times in buffer.c but `syntax-ppss' doesn't seem to clobber
Tassilo> match-data.

Seems like it would depend on whether the major mode uses regexps here?
My case is using tcl-mode FWIW.

However a superficial look at the syntax-ppss code shows it's calling
save-match-data in some spots, so perhaps this isn't the bug.  Just a
theory after all...

thanks,
Tom

bug#61395: 28.2; bug-reference warning seems incorrect

[Tassilo Horn]

Tom Tromey <tom@tromey.com> writes:

> Tassilo> many times in buffer.c but `syntax-ppss' doesn't seem to
> Tassilo> clobber match-data.
>
> Seems like it would depend on whether the major mode uses regexps
> here?  My case is using tcl-mode FWIW.

Hah, indeed!  It was easy to reproduce with a tcl file.

> However a superficial look at the syntax-ppss code shows it's calling
> save-match-data in some spots, so perhaps this isn't the bug.

It is.  I've fixed it on the emacs-29 branch.

Thanks a lot and 100 bonus points to Kévin who has been right with his
analysis.

Bye,
Tassilo

bug#61395: 28.2; bug-reference warning seems incorrect

[Tom Tromey]

Tassilo> If you can reproduce the issue, try changing
Tassilo> `bug-reference--overlay-bounds' so that the check

Tassilo>   (member bug-reference-bug-regexp
Tassilo>           bug-reference--nonconforming-regexps)

Ok, I can reproduce it now.

I removed this bit from bug-reference--overlay-bounds:

        (setq bug-reference--nonconforming-regexps
              (cons bug-reference-bug-regexp
                    bug-reference--nonconforming-regexps))

Then I set bug-reference--nonconforming-regexps to nil.
This combination means I can see the warning any time.

The gdb .dir-locals.el has this setting:

 (bug-reference-url-format . "http://sourceware.org/bugzilla/show_bug.cgi?id=%s")

I can send the whole file if you want, but I think this is the only
relevant part.

I've appended a file from gdb.  This is a .exp (dejagnu script) file,
which should use tcl-mode.

I have this code to enable bug reference mode:

(defun tromey-maybe-enable-bug-url-mode ()
  (and (boundp 'bug-reference-url-format)
       (stringp bug-reference-url-format)
       (if (or (derived-mode-p 'prog-mode)
	       (eq major-mode 'tcl-mode)	;emacs 23 bug
	       (eq major-mode 'makefile-mode)) ;emacs 23 bug
	   (bug-reference-prog-mode t)
	 (bug-reference-mode t))))

(add-hook 'find-file-hook #'tromey-maybe-enable-bug-url-mode)


LOL workarounds for emacs 23.

Anyway, I switched back to the .exp buffer and re-opened the file (C-x
C-v RET).  I saw the warning again.

thanks,
Tom

# Copyright (C) 2019-2023 Free Software Foundation, Inc.

# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program.  If not, see <http://www.gnu.org/licenses/>.

standard_testfile realname-expand.c realname-expand-real.c

# Compiling on a remote host does not support a filename with
# directory.
require {!is_remote host}

set srcdirabs [file join [pwd] $srcdir]
set srcfilelink [standard_output_file realname-expand-link.c]

remote_exec build "ln -sf ${srcdirabs}/${subdir}/${srcfile2} $srcfilelink"

if { [file type $srcfilelink] != "link" } {
    unsupported "target directory cannot have symbolic links"
    return -1
}

if { [gdb_compile "${srcdir}/${subdir}/${srcfile} ${srcfilelink}" "${binfile}" \
		  executable {debug}] != "" } {
    untested "failed to compile"
    return -1
}

clean_restart ${testfile}

if {![runto_main]} {
    return -1
}

gdb_breakpoint func message

gdb_test_no_output "set annotate 1"

# The patch to cause this output was reverted.
# See PR breakpoints/24915.
setup_xfail *-*-* 24915
gdb_test "continue" \
    "Breakpoint .* func .*realname-expand-link.c:$decimal\r\n\032\032.*realname-expand-link.c:.*"

bug#61395: 28.2; bug-reference warning seems incorrect

[Tom Tromey]

Tassilo> It is.  I've fixed it on the emacs-29 branch.

Thank you very much.

Tom

bug#61395: 28.2; bug-reference warning seems incorrect

[Kévin Le Gouguec]

Tassilo Horn <tsdh@gnu.org> writes:

> Thanks a lot and 100 bonus points to Kévin who has been right with his
> analysis.

s/analysis/'irrational fear of anything match-data related'/

…

s/irrational/'not always entirely unwarranted'/


Just living by Dr Monnier's wise words from bug#35564:

>>                I just tend to get superstitious about messing with match
>> data.
> 
> Instead, you should get superstitious about using the match data too
> far from the corresponding match.

Amen 🙏


(I've since learned that "too far" really means "one function call away,
if that function is declared neither 'pure nor 'side-effect-free")

bug#61395: 28.2; bug-reference warning seems incorrect

[Tom Tromey]

Kévin> (I've since learned that "too far" really means "one function call away,
Kévin> if that function is declared neither 'pure nor 'side-effect-free")

It'd be good to have an alternative regexp API that doesn't suffer from
these problems.  Both regexps themselves and match-data could just be
ordinary objects instead of hidden things.

Tom