GNU bug report logs - #13764
24.3.50; please do not use defsubst for `font-lock-(apply-highlight|fontify-anchored-keywords)'

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 13764 in the body.
You can then email your comments to 13764 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: 24.3.50; please do not use defsubst for
	`font-lock-(apply-highlight|fontify-anchored-keywords)'
Date: Mon, 18 Feb 2013 09:24:46 -0800

These are not tiny, trivial functions.
 
They do not fit the criteria for `defsubst' outlined in (elisp)
`Inline Functions'.  And they introduce all of the disadvantages
listed there, especially this one, listed first:
 
  [I]f you change the definition of the function, calls already
  inlined still use the old definition until you recompile them.
 
One use case: I have highlighting code that uses text property
`font-lock-ignore'.  I modify a few font-lock.el functions so that text
that has this property is ignored.  IOW, this property tells font-lock
"Hands off", so that it will not erase of otherwise interfere with such
highlighting.
 
I see no reason why these function should not be defuns.
 
In GNU Emacs 24.3.50.1 (i386-mingw-nt5.1.2600)
 of 2013-02-17 on VBOX-W7
Bzr revision: 111822 rgm <at> gnu.org-20130217190146-mm9bh3227ev56bus
Windowing system distributor `Microsoft Corp.', version 5.1.2600
Configured using:
 `configure --with-gcc (4.7) --no-opt --enable-checking --cflags
 -IC:/emacs/libs/libXpm-3.5.10/include -IC:/emacs/libs/libXpm-3.5.10/src
 -IC:/emacs/libs/libpng-dev_1.4.3-1_win32/include
 -IC:/emacs/libs/zlib-dev_1.2.5-2_win32/include
 -IC:/emacs/libs/giflib-4.1.4-1-lib/include
 -IC:/emacs/libs/jpeg-6b-4-lib/include
 -IC:/emacs/libs/tiff-3.8.2-1-lib/include
 -IC:/emacs/libs/libxml2-2.7.8-w32-bin/include/libxml2
 -IC:/emacs/libs/gnutls-3.1.8-w32/include
 -IC:/emacs/libs/libiconv-1.14-2-mingw32-dev/include'
 

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: "Drew Adams" <drew.adams <at> oracle.com>
Cc: 13764 <at> debbugs.gnu.org
Subject: Re: bug#13764: 24.3.50; please do not use defsubst for
 `font-lock-(apply-highlight|fontify-anchored-keywords)'
Date: Fri, 29 Apr 2016 00:31:08 +0200

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

> These are not tiny, trivial functions.
>
> They do not fit the criteria for `defsubst' outlined in (elisp)
> `Inline Functions'.  And they introduce all of the disadvantages
> listed there, especially this one, listed first:

[...]

> I see no reason why these function should not be defuns.

Presumably they are that way because of efficiency concerns.  Have you
done benchmarking to compare the impact of these defsubsts turning into
defuns?

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

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

From: Drew Adams <drew.adams <at> oracle.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 13764 <at> debbugs.gnu.org
Subject: RE: bug#13764: 24.3.50; please do not use defsubst for
 `font-lock-(apply-highlight|fontify-anchored-keywords)'
Date: Fri, 29 Apr 2016 09:17:25 -0700 (PDT)

> > These are not tiny, trivial functions.
> >
> > They do not fit the criteria for `defsubst' outlined in (elisp)
> > `Inline Functions'.  And they introduce all of the disadvantages
> > listed there, especially this one, listed first:
> 
> [...]
> 
> > I see no reason why these function should not be defuns.
> 
> Presumably they are that way because of efficiency concerns.  Have you
> done benchmarking to compare the impact of these defsubsts turning into
> defuns?

Oh come on.  Why such a presumption?  Did you look at the code?

Read what you quoted above.  These do not fit the clearly
documented criteria for `defsubst'.

Do you see a reason why these should be `defsubsts'?
Have you done benchmarking to support such a supposition?

As they don't fit the Emacs criteria, and there is no code
comment that calls out why they - exceptionally - should be
defsubsts in spite of not fulfilling the criteria, a priori
they should not be defsubsts.  The burden of proof lies
with a claim that they - exceptionally - should be.

Not to mention that there is less and less need for using
defsubst.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 13764 <at> debbugs.gnu.org
Subject: Re: bug#13764: 24.3.50; please do not use defsubst for
 `font-lock-(apply-highlight|fontify-anchored-keywords)'
Date: Wed, 30 Oct 2019 13:45:35 +0100

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

>> > I see no reason why these function should not be defuns.
>> 
>> Presumably they are that way because of efficiency concerns.  Have you
>> done benchmarking to compare the impact of these defsubsts turning into
>> defuns?
>
> Oh come on.  Why such a presumption?  Did you look at the code?

I take that as a "no", and I'm closing this bug report as a "wontfix".

-- 
(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.