bug#34781: 27.0.50; integer in pcase sometimes compared by eq

bug#34781: 27.0.50; integer in pcase sometimes compared by eq

[Mattias Engdegård]

(defun f (x)
  (pcase x
    ((or 'a #x10000000000000000) t)))

(f #x10000000000000000) => nil

Just replacing integerp with fixnump fixes this, but I'm not sure if
more of the same lurks somewhere.

--- a/lisp/emacs-lisp/pcase.el
+++ b/lisp/emacs-lisp/pcase.el
@@ -792,7 +792,7 @@ pcase--u1
                    (let ((upat (cddr alt)))
                      (eq (car-safe upat) 'quote)))
               (let ((val (cadr (cddr alt))))
-                (unless (or (integerp val) (symbolp val))
+                (unless (or (fixnump val) (symbolp val))
                   (setq memq-ok nil))
                 (push (cadr (cddr alt)) simples))
             (push alt others))))

bug#34781: Acknowledgement (27.0.50; integer in pcase sometimes compared by eq)

[Mattias Engdegård]

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

Tags: patch

Complete patch with test case.


[-- Attachment #2: 0001-Don-t-match-bignums-with-memq-in-pcase.patch --]
[-- Type: text/x-patch, Size: 2008 bytes --]

From b156563ce680f2f63cf57e976f2416d53a94ab4e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Tue, 12 Mar 2019 13:19:35 +0100
Subject: [PATCH] Don't match bignums with `memq' in `pcase'

* lisp/emacs-lisp/pcase.el (pcase--u1):
Use fixnump instead of integerp as criterion for memq (Bug#34781).
* test/lisp/emacs-lisp/pcase-tests.el (pcase-tests-member): Test the above.
---
 lisp/emacs-lisp/pcase.el            | 2 +-
 test/lisp/emacs-lisp/pcase-tests.el | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/pcase.el b/lisp/emacs-lisp/pcase.el
index 9de2401549..c26a3d7708 100644
--- a/lisp/emacs-lisp/pcase.el
+++ b/lisp/emacs-lisp/pcase.el
@@ -792,7 +792,7 @@ pcase--u1
                    (let ((upat (cddr alt)))
                      (eq (car-safe upat) 'quote)))
               (let ((val (cadr (cddr alt))))
-                (unless (or (integerp val) (symbolp val))
+                (unless (or (fixnump val) (symbolp val))
                   (setq memq-ok nil))
                 (push (cadr (cddr alt)) simples))
             (push alt others))))
diff --git a/test/lisp/emacs-lisp/pcase-tests.el b/test/lisp/emacs-lisp/pcase-tests.el
index 1e9d37fbfa..a7c66dbf1e 100644
--- a/test/lisp/emacs-lisp/pcase-tests.el
+++ b/test/lisp/emacs-lisp/pcase-tests.el
@@ -56,6 +56,12 @@ pcase-tests-grep
            'member (macroexpand-all '(pcase x ((or "a" 2 3) body)))))
   (should-not (pcase-tests-grep
                'memq (macroexpand-all '(pcase x ((or "a" 2 3) body)))))
+  (should (pcase-tests-grep
+           'member (macroexpand-all '(pcase x ((or #x10000000000000000 2 3)
+                                               body)))))
+  (should-not (pcase-tests-grep
+               'memq (macroexpand-all '(pcase x ((or #x10000000000000000 2 3)
+                                                 body)))))
   (let ((exp (macroexpand-all
                       '(pcase x
                          ("a" body1)
-- 
2.20.1

bug#34781: 27.0.50; integer in pcase sometimes compared by eq

[Mattias Engdegård]

I can't find any similar problem elsewhere in the pcase code; singular bignum comparison was taken care of long ago.
Is the patch acceptable?

bug#34781: 27.0.50; integer in pcase sometimes compared by eq

[Paul Eggert]

> I can't find any similar problem elsewhere in the pcase code; singular bignum comparison was taken care of long ago.
> Is the patch acceptable?

Thanks, it looks good to me; please install into the master branch

What happens if a source file that uses pcase is compiled on a 64-bit
machine that has wide fixnums, and is then loaded and run on a 32-bit
machine that has narrow fixnums? Will this pcase code still work? And if
not, are there similar bugs elsewhere in the pcase code?

bug#34781: 27.0.50; integer in pcase sometimes compared by eq

[Michael Heerdegen]

Paul Eggert <eggert@cs.ucla.edu> writes:

> What happens if a source file that uses pcase is compiled on a 64-bit
> machine that has wide fixnums, and is then loaded and run on a 32-bit
> machine that has narrow fixnums? Will this pcase code still work? And
> if not, are there similar bugs elsewhere in the pcase code?

AFAIU the patch only corrects an optimization (use `memq' instead of
`member') that doesn't work for bignums.  This can never be harmful.

I had a quick look at pcase.el, and also tried the obvious cases (quote,
backquote), and didn't find any other obvious bug.


Michael.

bug#34781: 27.0.50; integer in pcase sometimes compared by eq

[Paul Eggert]

On 3/28/19 12:47 PM, Michael Heerdegen wrote:
> AFAIU the patch only corrects an optimization (use `memq' instead of
> `member') that doesn't work for bignums.  This can never be harmful.

Isn't it harmful if a 64-bit Emacs decides that the optimization is safe
for the fixnum 1000000000 and thus generates the faster code, but the
code is put into an .elc file and then loaded by a 32-bit emacs that
treats 1000000000 as a bignum?

(An alternative to this annoying most-positive-portable-bignum business
would be to require --with-wide-int on all platforms. :-)

bug#34781: 27.0.50; integer in pcase sometimes compared by eq

[Michael Heerdegen]

Paul Eggert <eggert@cs.ucla.edu> writes:

> On 3/28/19 12:47 PM, Michael Heerdegen wrote:
> > AFAIU the patch only corrects an optimization (use `memq' instead of
> > `member') that doesn't work for bignums.  This can never be harmful.
>
> Isn't it harmful if a 64-bit Emacs decides that the optimization is safe
> for the fixnum 1000000000 and thus generates the faster code, but the
> code is put into an .elc file and then loaded by a 32-bit emacs that
> treats 1000000000 as a bignum?

Ah ok - yes, I guess that would indeed happen.

Michael.

bug#34781: 27.0.50; integer in pcase sometimes compared by eq

[Mattias Engdegård]

tor 2019-03-28 klockan 11:25 -0700 skrev Paul Eggert:
> 
> Thanks, it looks good to me; please install into the master branch
> 
> What happens if a source file that uses pcase is compiled on a 64-bit
> machine that has wide fixnums, and is then loaded and run on a 32-bit
> machine that has narrow fixnums? Will this pcase code still work? And
> if
> not, are there similar bugs elsewhere in the pcase code?

Not that I can see; the singular case uses eql for integerp.
I didn't think of the case you described, thanks. We then need a new
function:

  portable-fixnum-p
  guaranteed-fixnum-p
  always-fixnum-p
  fixnum-everywhere-p
  here-a-fixnum-there-a-fixnum-everywhere-a-fixnum-p

and names for the bounds:

  portable-most-{positive,negative}-fixnum
  ...

Name suggestions welcome. Meanwhile, I'll make a new patch.

bug#34781: 27.0.50; integer in pcase sometimes compared by eq

[Paul Eggert]

On 3/28/19 12:51 PM, Mattias Engdegård wrote:
> Not that I can see; the singular case uses eql for integerp.
> I didn't think of the case you described, thanks. We then need a new
> function:
>
>   portable-fixnum-p
> ...
>
> and names for the bounds:
>
>   portable-most-{positive,negative}-fixnum

If we have the bounds, then portable-fixnum-p is merely a convenience, no?

I'd prefer the names most-negative-portable-fixnum and
most-positive-portable-fixnum. Their documentation should make it clear
what the portability test is for. Presumably the test applies just to
this version of Emacs, since future versions might change the portable
fixnum bounds.

bug#34781: 27.0.50; integer in pcase sometimes compared by eq

[Mattias Engdegård]

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

tor 2019-03-28 klockan 13:30 -0700 skrev Paul Eggert:

> If we have the bounds, then portable-fixnum-p is merely a
> convenience, no?

Yes. We can drop it if you prefer it to be open-coded in pcase and
elsewhere, but I thought the predicate would make sense.

> I'd prefer the names most-negative-portable-fixnum and
> most-positive-portable-fixnum. Their documentation should make it
> clear
> what the portability test is for. Presumably the test applies just to
> this version of Emacs, since future versions might change the
> portable
> fixnum bounds.

Here is a patch for that, and an updated pcase patch.


[-- Attachment #2: 0001-Add-bounds-for-portable-fixnums-and-portable-fixnum-.patch --]
[-- Type: text/x-patch, Size: 4717 bytes --]

From f5fe5d987090e69b0f7438435f69c6aab4215e5c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Thu, 28 Mar 2019 22:12:37 +0100
Subject: [PATCH 1/2] Add bounds for portable fixnums, and portable-fixnum-p

These are useful for macros that need to detect whether a number is a
fixnum on any machine, so that the bytecode becomes portable (Bug#34781).

* src/lisp.h (LEAST_EMACS_INT_MAX, MOST_POSITIVE_PORTABLE_FIXNUM,
MOST_NEGATIVE_PORTABLE_FIXNUM):
* src/data.c (most-positive-portable-fixnum, most-negative-portable-fixnum):
* lisp/subr.el (portable-fixnum-p):
New.
* etc/NEWS (Lisp Changes): Mention portable-fixnum-p.
---
 etc/NEWS     |  4 ++++
 lisp/subr.el |  6 ++++++
 src/data.c   | 18 ++++++++++++++++++
 src/lisp.h   | 12 ++++++++++++
 4 files changed, 40 insertions(+)

diff --git a/etc/NEWS b/etc/NEWS
index 7486d6bcfe..a2933fbbd3 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1424,6 +1424,10 @@ like 'file-attributes' that compute file sizes and other attributes,
 functions like 'process-id' that compute process IDs, and functions like
 'user-uid' and 'group-gid' that compute user and group IDs.
 
+Since the size of fixnums varies between platforms, the new predicate
+'portable-fixnum-p' can be used to determine whether a number is
+a fixnum on any machine running the current Emacs version.
+
 +++
 ** Although the default timestamp format is still (HI LO US PS),
 it is planned to change in a future Emacs version, to exploit bignums.
diff --git a/lisp/subr.el b/lisp/subr.el
index f1a1dddd81..950a0b58e3 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -376,6 +376,12 @@ bignump
   "Return t if OBJECT is a bignum."
   (and (integerp object) (not (fixnump object))))
 
+(defun portable-fixnum-p (object)
+  "Return t if OBJECT is a fixnum on any machine running the current
+Emacs version."
+  (and (integerp object)
+       (<= most-negative-portable-fixnum object most-positive-portable-fixnum)))
+
 (defun lsh (value count)
   "Return VALUE with its bits shifted left by COUNT.
 If COUNT is negative, shifting is actually to the right.
diff --git a/src/data.c b/src/data.c
index 15b6106cfe..2969f2df82 100644
--- a/src/data.c
+++ b/src/data.c
@@ -4110,6 +4110,24 @@ This variable cannot be set; trying to do so will signal an error.  */);
   Vmost_negative_fixnum = make_fixnum (MOST_NEGATIVE_FIXNUM);
   make_symbol_constant (intern_c_string ("most-negative-fixnum"));
 
+  DEFVAR_LISP ("most-positive-portable-fixnum",
+               Vmost_positive_portable_fixnum,
+               doc: /* The greatest integer that is represented efficiently
+on any machine running this version of Emacs.
+This variable can be used to ensure portability of bytecode that works
+with fixnums.  It cannot be set; trying to do so will signal an error.  */);
+  Vmost_positive_portable_fixnum = make_fixnum(MOST_POSITIVE_PORTABLE_FIXNUM);
+  make_symbol_constant (intern_c_string ("most-positive-portable-fixnum"));
+
+  DEFVAR_LISP ("most-negative-portable-fixnum",
+               Vmost_negative_portable_fixnum,
+               doc: /* The least integer that is represented efficiently
+on any machine running this version of Emacs.
+This variable can be used to ensure portability of bytecode that works
+with fixnums.  It cannot be set; trying to do so will signal an error.  */);
+  Vmost_negative_portable_fixnum = make_fixnum(MOST_NEGATIVE_PORTABLE_FIXNUM);
+  make_symbol_constant (intern_c_string ("most-negative-portable-fixnum"));
+
   DEFSYM (Qwatchers, "watchers");
   DEFSYM (Qmakunbound, "makunbound");
   DEFSYM (Qunlet, "unlet");
diff --git a/src/lisp.h b/src/lisp.h
index 178eebed2a..bf1f0a0bf5 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -112,6 +112,13 @@ enum { EMACS_INT_WIDTH = LLONG_WIDTH, EMACS_UINT_WIDTH = ULLONG_WIDTH };
 # endif
 #endif
 
+/* The smallest portable value of EMACS_INT_MAX.  */
+#define LEAST_EMACS_INT_MAX 2147483647   /* 2**31 - 1 */
+
+#if EMACS_INT_MAX < LEAST_EMACS_INT_MAX
+# error "EMACS_INT_MAX less than LEAST_EMACS_INT_MAX"
+#endif
+
 /* Number of bits to put in each character in the internal representation
    of bool vectors.  This should not vary across implementations.  */
 enum {  BOOL_VECTOR_BITS_PER_CHAR =
@@ -1146,6 +1153,11 @@ enum More_Lisp_Bits
 #define MOST_POSITIVE_FIXNUM (EMACS_INT_MAX >> INTTYPEBITS)
 #define MOST_NEGATIVE_FIXNUM (-1 - MOST_POSITIVE_FIXNUM)
 
+/* Largest and smallest values that are guaranteed to be representable
+   as fixnums on any machine.  These are the C values.  */
+#define MOST_POSITIVE_PORTABLE_FIXNUM (LEAST_EMACS_INT_MAX >> INTTYPEBITS)
+#define MOST_NEGATIVE_PORTABLE_FIXNUM (-1 - MOST_POSITIVE_PORTABLE_FIXNUM)
+
 #if USE_LSB_TAG
 
 INLINE Lisp_Object
-- 
2.20.1


[-- Attachment #3: 0002-Don-t-match-integers-with-memq-in-pcase.patch --]
[-- Type: text/x-patch, Size: 2017 bytes --]

From 5054ac21b6bdb522437d97db2a514a53d8ce7773 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Tue, 12 Mar 2019 13:19:35 +0100
Subject: [PATCH 2/2] Don't match integers with `memq' in `pcase'

* lisp/emacs-lisp/pcase.el (pcase--u1):
Use portable-fixnum-p instead of integerp as criterion for memq (Bug#34781).
* test/lisp/emacs-lisp/pcase-tests.el (pcase-tests-member): Test the above.
---
 lisp/emacs-lisp/pcase.el            | 2 +-
 test/lisp/emacs-lisp/pcase-tests.el | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/pcase.el b/lisp/emacs-lisp/pcase.el
index 9de2401549..a13694ed33 100644
--- a/lisp/emacs-lisp/pcase.el
+++ b/lisp/emacs-lisp/pcase.el
@@ -792,7 +792,7 @@ pcase--u1
                    (let ((upat (cddr alt)))
                      (eq (car-safe upat) 'quote)))
               (let ((val (cadr (cddr alt))))
-                (unless (or (integerp val) (symbolp val))
+                (unless (or (portable-fixnum-p val) (symbolp val))
                   (setq memq-ok nil))
                 (push (cadr (cddr alt)) simples))
             (push alt others))))
diff --git a/test/lisp/emacs-lisp/pcase-tests.el b/test/lisp/emacs-lisp/pcase-tests.el
index 1e9d37fbfa..29f02cbaa9 100644
--- a/test/lisp/emacs-lisp/pcase-tests.el
+++ b/test/lisp/emacs-lisp/pcase-tests.el
@@ -56,6 +56,12 @@ pcase-tests-grep
            'member (macroexpand-all '(pcase x ((or "a" 2 3) body)))))
   (should-not (pcase-tests-grep
                'memq (macroexpand-all '(pcase x ((or "a" 2 3) body)))))
+  (should (pcase-tests-grep
+           'member (macroexpand-all '(pcase x ((or #x100000000 2 3)
+                                               body)))))
+  (should-not (pcase-tests-grep
+               'memq (macroexpand-all '(pcase x ((or #x100000000 2 3)
+                                                 body)))))
   (let ((exp (macroexpand-all
                       '(pcase x
                          ("a" body1)
-- 
2.20.1

bug#34781: 27.0.50; integer in pcase sometimes compared by eq

[Paul Eggert]

>
> +Since the size of fixnums varies between platforms, the new predicate
> +'portable-fixnum-p' can be used to determine whether a number is
> +a fixnum on any machine running the current Emacs version.

The news item should also mention most-negative-portable-fixnum etc. Try
to be terser; e.g., "can be used to determine" -> "determines".


>
> +(defun portable-fixnum-p (object)
> +  "Return t if OBJECT is a fixnum on any machine running the current
> +Emacs version."

The usage message can fit on one line.

> +  (and (integerp object)
> +       (<= most-negative-portable-fixnum object
> most-positive-portable-fixnum)))

integerp -> fixnump


> +  DEFVAR_LISP ("most-positive-portable-fixnum",
> +               Vmost_positive_portable_fixnum,
> +               doc: /* The greatest integer that is represented
> efficiently
> +on any machine running this version of Emacs.

Try to have the first line explain things tersely. Something like "The
largest integer representable as a fixnum on any platform." More details
can be in later lines, if needed.

> +  Vmost_positive_portable_fixnum =
> make_fixnum(MOST_POSITIVE_PORTABLE_FIXNUM);

Space before parenthesis (elsewhere, too).


>
> diff --git a/src/lisp.h b/src/lisp.h
> index 178eebed2a..bf1f0a0bf5 100644
> --- a/src/lisp.h
> +++ b/src/lisp.h

These changes should be in data.c not lisp.h, since only data.c needs
them and it's not likely any other code will need them.


>
> +/* The smallest portable value of EMACS_INT_MAX.  */
> +#define LEAST_EMACS_INT_MAX 2147483647   /* 2**31 - 1 */

There's no need to make it a macro. Also, the LEAST_* prefix and *_MAX
suffix are confusing: which takes priority? I suggest sticking to
suffixes, since that seems to be the convention. Something like

  int EMACS_INT_MAX_MIN = 2147483647;

as a local in the only function that needs it, and similarly for the
related macros. Although you can use 'verify' to check that
EMACS_INT_MAX_MIN <= EMACS_INT_MAX, I'm not sure I'd bother as we're
going to add overflow checking to make_fixnum at some point anyway.

This stuff should be documented in the manual, too, next to the
documentation of most-positive-fixnum and fixnump respectively.

Thanks again for taking this on.

bug#34781: 27.0.50; integer in pcase sometimes compared by eq

[Stefan Monnier]

>                     (let ((upat (cddr alt)))
>                       (eq (car-safe upat) 'quote)))
>                (let ((val (cadr (cddr alt))))
> -                (unless (or (integerp val) (symbolp val))
> +                (unless (or (portable-fixnum-p val) (symbolp val))
>                    (setq memq-ok nil))
>                  (push (cadr (cddr alt)) simples))
>              (push alt others))))

Really?
I think the better option is below (since I think we should generally
move away from `eq` and replace it with `eql`).

Actually, the hunk below should have been installed at the same time
I replaced `eq` with `eql` when testing against an integer.  It was
a mere oversight.


        Stefan


diff --git a/lisp/emacs-lisp/pcase.el b/lisp/emacs-lisp/pcase.el
index 9de240154..d9a20b1ff 100644
--- a/lisp/emacs-lisp/pcase.el
+++ b/lisp/emacs-lisp/pcase.el
@@ -802,7 +802,7 @@ pcase--u1
        ((> (length simples) 1)
         (pcase--u1 (cons `(match ,var
                                  . (pred (pcase--flip
-                                          ,(if memq-ok #'memq #'member)
+                                          ,(if memq-ok #'memql #'member)
                                           ',simples)))
                          (cdr matches))
                    code vars

bug#34781: 27.0.50; integer in pcase sometimes compared by eq

[Mattias Engdegård]

28 mars 2019 kl. 23.11 skrev Stefan Monnier <monnier@IRO.UMontreal.CA>:

> Actually, the hunk below should have been installed at the same time
> I replaced `eq` with `eql` when testing against an integer.  It was
> a mere oversight.

And all this work just because I didn't know we had memql. Serves me right.

Thank you Stefan, I'll do the obvious (unless you beat me to it).

I can still think of cases when portable-fixnum-p would be useful. I can clean up the patch (according to Paul's comments), or just sit on it.

bug#34781: 27.0.50; integer in pcase sometimes compared by eq

[Paul Eggert]

On 3/28/19 3:20 PM, Mattias Engdegård wrote:
> I'll do the obvious (unless you beat me to it).

When you do that, please also change the name of the local from memq-ok
to memql-ok, for clarity. Thanks.

bug#34781: 27.0.50; integer in pcase sometimes compared by eq

[Mattias Engdegård]

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

tor 2019-03-28 klockan 15:38 -0700 skrev Paul Eggert:
> On 3/28/19 3:20 PM, Mattias Engdegård wrote:
> > I'll do the obvious (unless you beat me to it).
> 
> When you do that, please also change the name of the local from memq-
> ok
> to memql-ok, for clarity. Thanks.

Done. Thanks for your help! I'm attaching the mostly cleaned-up patch
(minus the necessary doc changes), in case someone will see some use
for it.


[-- Attachment #2: 0001-Add-bounds-for-portable-fixnums-and-portable-fixnum-.patch --]
[-- Type: text/x-patch, Size: 3863 bytes --]

From a60e78be5dfc2172345d140f2cd8df0a77bea0a1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Thu, 28 Mar 2019 22:12:37 +0100
Subject: [PATCH] Add bounds for portable fixnums, and portable-fixnum-p

These are useful for macros that need to detect whether a number is a
fixnum on any machine, so that the bytecode becomes portable (Bug#34781).

* src/data.c (most-positive-portable-fixnum, most-negative-portable-fixnum):
* lisp/subr.el (portable-fixnum-p):
New.
* etc/NEWS (Lisp Changes): Mention portable-fixnum-p.
---
 etc/NEWS     |  5 +++++
 lisp/subr.el |  5 +++++
 src/data.c   | 24 ++++++++++++++++++++++++
 3 files changed, 34 insertions(+)

diff --git a/etc/NEWS b/etc/NEWS
index 7486d6bcfe..2ab3c5b4bb 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1424,6 +1424,11 @@ like 'file-attributes' that compute file sizes and other attributes,
 functions like 'process-id' that compute process IDs, and functions like
 'user-uid' and 'group-gid' that compute user and group IDs.
 
+Since the size of fixnums varies between platforms, the new predicate
+'portable-fixnum-p' determines whether a number is a fixnum on any
+machine running the current Emacs version. The corresponding bounds
+are 'most-negative-portable-fixnum' and 'most-negative-portable-fixnum'.
+
 +++
 ** Although the default timestamp format is still (HI LO US PS),
 it is planned to change in a future Emacs version, to exploit bignums.
diff --git a/lisp/subr.el b/lisp/subr.el
index f1a1dddd81..efa9cd4065 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -376,6 +376,11 @@ bignump
   "Return t if OBJECT is a bignum."
   (and (integerp object) (not (fixnump object))))
 
+(defun portable-fixnum-p (object)
+  "Return t if OBJECT is a fixnum on any machine running this Emacs version."
+  (and (integerp object)
+       (<= most-negative-portable-fixnum object most-positive-portable-fixnum)))
+
 (defun lsh (value count)
   "Return VALUE with its bits shifted left by COUNT.
 If COUNT is negative, shifting is actually to the right.
diff --git a/src/data.c b/src/data.c
index 15b6106cfe..6d50a14bde 100644
--- a/src/data.c
+++ b/src/data.c
@@ -4110,6 +4110,30 @@ This variable cannot be set; trying to do so will signal an error.  */);
   Vmost_negative_fixnum = make_fixnum (MOST_NEGATIVE_FIXNUM);
   make_symbol_constant (intern_c_string ("most-negative-fixnum"));
 
+  /* The smallest portable value of EMACS_INT_MAX.  */
+  int least_emacs_int_max = 2147483647;   /* 2**31 - 1 */
+
+  /* Largest and smallest values that are guaranteed to be representable
+     as fixnums on any machine.  */
+  int most_positive_portable_fixnum = least_emacs_int_max >> INTTYPEBITS;
+  int most_negative_portable_fixnum = -1 - most_positive_portable_fixnum;
+
+  DEFVAR_LISP ("most-positive-portable-fixnum",
+               Vmost_positive_portable_fixnum,
+               doc: /* The largest integer representable as a fixnum on any platform.
+This variable can be used to ensure portability of bytecode that works
+with fixnums.  It cannot be set; trying to do so will signal an error.  */);
+  Vmost_positive_portable_fixnum = make_fixnum (most_positive_portable_fixnum);
+  make_symbol_constant (intern_c_string ("most-positive-portable-fixnum"));
+
+  DEFVAR_LISP ("most-negative-portable-fixnum",
+               Vmost_negative_portable_fixnum,
+               doc: /* The least integer representable as a fixnum on any platform.
+This variable can be used to ensure portability of bytecode that works
+with fixnums.  It cannot be set; trying to do so will signal an error.  */);
+  Vmost_negative_portable_fixnum = make_fixnum (most_negative_portable_fixnum);
+  make_symbol_constant (intern_c_string ("most-negative-portable-fixnum"));
+
   DEFSYM (Qwatchers, "watchers");
   DEFSYM (Qmakunbound, "makunbound");
   DEFSYM (Qunlet, "unlet");
-- 
2.20.1

bug#34781: 27.0.50; integer in pcase sometimes compared by eq

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Thu, 28 Mar 2019 22:51:39 +0100
> Cc: monnier@iro.umontreal.ca, 34781@debbugs.gnu.org

In addition to Paul's comments:

> +Since the size of fixnums varies between platforms, the new predicate
> +'portable-fixnum-p' can be used to determine whether a number is
> +a fixnum on any machine running the current Emacs version.

This entry lacks a header.  NEWS is viewed in Outline mode, so it
should have headers that start with one or more '*' characters.
Please add a header for this item, and please make it short enough to
fit on a single line.

Also, it is advisable to accompany user-visible changes with suitable
changes in documentation, in this case the ELisp manual.  If you do
provide patches for the manuals, the NEWS entry should be marked with
"+++" to indicate that all the documentation will have been updated
when the patch is pushed.

> +(defun portable-fixnum-p (object)
> +  "Return t if OBJECT is a fixnum on any machine running the current
> +Emacs version."

Suggest to rephrase:

    "Non-nil if OBJECT is a fixnum on any platform.
  The value will be nil if OBJECT is not a number, or if its value
  needs more bits than a fixnum can support on some platforms."

> +  DEFVAR_LISP ("most-positive-portable-fixnum",
> +               Vmost_positive_portable_fixnum,
> +               doc: /* The greatest integer that is represented efficiently
> +on any machine running this version of Emacs.

The first line of any doc string must not be a complete sentence.
This is because apropos commands only display the first line of the
doc string.  So I suggest to reword:

  The largest integer representable as fixnum on any platform.

> +  DEFVAR_LISP ("most-negative-portable-fixnum",
> +               Vmost_negative_portable_fixnum,
> +               doc: /* The least integer that is represented efficiently
> +on any machine running this version of Emacs.

Similarly here.

Thanks for working on this.

bug#34781: 27.0.50; integer in pcase sometimes compared by eq

[Mattias Engdegård]

29 mars 2019 kl. 09.48 skrev Eli Zaretskii <eliz@gnu.org>:
> 
> In addition to Paul's comments:

Eli, thanks for your kind review. Since portable-fixnum-p turned out not to be necessary for this bug after all, I decided not to go ahead with the patch. If you think it is still valuable enough on its own, say so and I'll polish it according to your notes.

bug#34781: 27.0.50; integer in pcase sometimes compared by eq

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Fri, 29 Mar 2019 10:52:28 +0100
> Cc: Paul Eggert <eggert@cs.ucla.edu>, monnier@iro.umontreal.ca,
>         34781@debbugs.gnu.org
> 
> 29 mars 2019 kl. 09.48 skrev Eli Zaretskii <eliz@gnu.org>:
> > 
> > In addition to Paul's comments:
> 
> Eli, thanks for your kind review. Since portable-fixnum-p turned out not to be necessary for this bug after all, I decided not to go ahead with the patch. If you think it is still valuable enough on its own, say so and I'll polish it according to your notes.

I do think that something like that would be useful in the long run,
yes.  Not terribly important, though, so if you have better uses for
your time, I won't push.

Thanks.

bug#34781: 27.0.50; integer in pcase sometimes compared by eq

[Michael Heerdegen]

Mattias Engdegård <mattiase@acm.org> writes:

> (defun f (x)
>   (pcase x
>     ((or 'a #x10000000000000000) t)))
>
> (f #x10000000000000000) => nil
>
> Just replacing integerp with fixnump fixes this, but I'm not sure if
> more of the same lurks somewhere.

I had a quick look and didn't find anything obvious.

Michael.