bug#48281: buffer_local_value and find_symbol_value duplicate functionality

bug#48281: buffer_local_value and find_symbol_value duplicate functionality

[Spencer Baugh]

find_symbol_value is described as:

  Find the value of a symbol, returning Qunbound if it's not bound.

buffer_local_value does the same, except that it allows one to specify a
buffer.

Yet they both implement symbol lookup, without sharing code.  And given
that the comment above find_symbol_value says "Great care is required
for this.", I'm guessing that one or both of them may have bugs that the
other does not.  Especially because buffer_local_value is simpler than
find_symbol_value, despite doing an ostensibly more complicated job...

How about unifying them into a single function?  Would a patch doing
that be accepted?

Alternatively, maybe I'm missing some detail about why they're
different?

bug#48281: buffer_local_value and find_symbol_value duplicate functionality

[Eli Zaretskii]

> From: Spencer Baugh <sbaugh@catern.com>
> Date: Fri, 07 May 2021 18:44:33 -0400
> 
> 
> find_symbol_value is described as:
> 
>   Find the value of a symbol, returning Qunbound if it's not bound.
> 
> buffer_local_value does the same, except that it allows one to specify a
> buffer.
> 
> Yet they both implement symbol lookup, without sharing code.  And given
> that the comment above find_symbol_value says "Great care is required
> for this.", I'm guessing that one or both of them may have bugs that the
> other does not.  Especially because buffer_local_value is simpler than
> find_symbol_value, despite doing an ostensibly more complicated job...
> 
> How about unifying them into a single function?  Would a patch doing
> that be accepted?
> 
> Alternatively, maybe I'm missing some detail about why they're
> different?

First, such discussion is better conducted on emacs-devel, not here,
as some of the relevant people don't read the bug list.  Adding
Stefan, who made extensive changes to both functions some 10 years
ago.

More to the point, I'm not sure I understand how you intend to
reconcile the differences in these two functions.  They are similar,
but not identical.  What is the plan for dealing with the differences?

Given that we can safely conflate the two implementations, I don't see
why we won't want to do that.

(The "great care" bit refers to the need to block quitting, btw, not
to the code as a whole.)

bug#48281: buffer_local_value and find_symbol_value duplicate functionality

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

>> find_symbol_value is described as:
[...]
>> buffer_local_value does the same, except that it allows one to specify a
>> buffer.
[...]
>> How about unifying them into a single function?  Would a patch doing
>> that be accepted?

Depends on the patch ;-)

>> Alternatively, maybe I'm missing some detail about why they're
>> different?

When I reworked that code I was annoyed by that difference but I had
enough other things to deal with that I didn't bother trying to
reconcile the two functions.

The thing that is important to know about those two functions is that
`find_symbol_value` is performance-critical, whereas
`buffer_local_value` is not.

So any effort to unify the two should focus on not slowing down
`find_symbol_value`.  The fact that here are two functions is acceptable
to me, but indeed the two code paths are uncomfortably different, so I'd
welcome changes (including minor changes) to make them more similar, tho
maybe the best we can do is add comments explaining why the two code
paths give the same result.


        Stefan

bug#48281: buffer_local_value and find_symbol_value duplicate functionality

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> So any effort to unify the two should focus on not slowing down
> `find_symbol_value`.  The fact that here are two functions is acceptable
> to me, but indeed the two code paths are uncomfortably different, so I'd
> welcome changes (including minor changes) to make them more similar, tho
> maybe the best we can do is add comments explaining why the two code
> paths give the same result.

I think the conclusion here is that it's not worth the added complexity
to merge these two functions, so I've now added a comment about this
instead.

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