bug#39529: 28.0.50; Metahelp does not contain help text

[Paul Eggert <eggert@cs.ucla.edu>]

[-- Attachment #1: Type: text/plain, Size: 2257 bytes --]

On 2/16/20 8:51 AM, Pip Cet wrote:

> it relies on different objects having different hashes, and they might,
> by pure chance, have the same hash.

That could happen only if two Lisp vector addresses disagree only in the 
high-order bit during dumping while on a USE_LSB_TAG platform, something that I 
didn't think possible on practical Emacs targets. However, the potential problem 
is easy enough to work around; I installed the attached.

> We need to fix the underlying issue, or at least go for the correct
> "quick fix", which is to make equal = eq for bytecode objects (this is
> almost indistinguishable from the previous behavior, before
> sxhash-equal was "fixed").

Give the attached patch I hope no such quick fix is needed.

For what it's worth, the patched code is similar to what Fautoload does; so if 
there's something wrong here, a similar wrongness has likely been present in 
Fautoload for some time.

> The pure-cons hash, and many other places, assume "equal" means
> "equivalent" in some way. That's not true for bytecode objects, where
> a function always returning `nil' can be equal to one always returning
> `t'.

Could you give an example of this? When I byte-compiled this:

(defun always-t () t)
(defun always-nil () nil)

the .elc file had different bytecode objects #[nil "\300\207" [t] 1] and #[nil 
"\300\207" [nil] 1]. (The strings are the same, but that's not the issue here.)

> the right fix is not to make equal look at
> more state than sxhash-equal used to, particularly for Emacs 27.

That would indeed make 'equal' and 'sxhash-equal' consistent. But wouldn't it 
potentially break a different set of user programs, that (say) rely on 'equal' 
returning nil when markers point to different locations? So I'd be leery about 
messing with Emacs 27 in this area.

For the master branch, it's more of a question of what 'equal' should mean. 
Obviously 'sxhash-equal' must be consistent with 'equal'. If we decide that 
'equal' should not inspect marker contents etc., then we should change 
'sxhash-equal' accordingly.

(To my mind a better fix would be to introduce the notion of immutability, and 
for sxhash-equal to inspect only the immutable parts of key contents. But now 
it's time for me to duck and run....)

[-- Attachment #2: 0001-Improve-C-h-C-h-bug-fix.patch --]
[-- Type: text/x-patch, Size: 1482 bytes --]

From 556cc727e5076d590f8286406e4f46cff3cee41e Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 16 Feb 2020 11:36:19 -0800
Subject: [PATCH] Improve C-h C-h bug fix

* src/lread.c (read1): Guard against two 'struct Lisp_Vector *'
pointers differing only in their most significant bit.  Problem
reported by Pip Cet (Bug#39529#22).
---
 src/lread.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/lread.c b/src/lread.c
index 1613719eb1..70984d37e1 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -2982,10 +2982,13 @@ read1 (Lisp_Object readcharfun, int *pch, bool first_in_list)
 		 as 0.  This placeholder 0 would lead to accidental sharing in
 		 purecopy's hash-consing, so replace it with a (hopefully)
 		 unique integer placeholder, which is negative so that it is
-		 not confused with a DOC file offset.  Eventually
-		 Snarf-documentation should replace the placeholder with the
-		 actual docstring.  */
-	      EMACS_UINT hash = XHASH (tmp) | (INTMASK - INTMASK / 2);
+		 not confused with a DOC file offset (the USE_LSB_TAG shift
+		 relies on the fact that VALMASK is one bit narrower than
+		 INTMASK).  Eventually Snarf-documentation should replace the
+		 placeholder with the actual docstring.  */
+	      verify (INTMASK & ~VALMASK);
+	      EMACS_UINT hash = ((XHASH (tmp) >> USE_LSB_TAG)
+				 | (INTMASK - INTMASK / 2));
 	      ASET (tmp, COMPILED_DOC_STRING, make_ufixnum (hash));
 	    }
 
-- 
2.17.1