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

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

[Drew Adams]

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@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'
 

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

[Lars Ingebrigtsen]

"Drew Adams" <drew.adams@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

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

[Drew Adams]

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

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

[Lars Ingebrigtsen]

Drew Adams <drew.adams@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