unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#27718: 26.0.50; (cl-typep instance-ab 'class-a) gives wrong answer when compiled
@ 2017-07-15 21:50 npostavs
  2017-07-21 12:23 ` npostavs
  0 siblings, 1 reply; 11+ messages in thread
From: npostavs @ 2017-07-15 21:50 UTC (permalink / raw)
  To: 27718; +Cc: Alex

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


    ~/src/emacs$ .../emacs -Q -batch -f batch-byte-compile cl-typep-subclass.el
    ~/src/emacs$ .../emacs -Q -batch -l cl-typep-subclass.el
    (cl-typep eitest-ab ’class-a) => t
    ~/src/emacs$ .../emacs -Q -batch -l cl-typep-subclass.elc
    (cl-typep eitest-ab ’class-a) => nil

Where cl-typep-subclass.el is


[-- Attachment #2: test file --]
[-- Type: text/plain, Size: 382 bytes --]

(require 'eieio)
(require 'eieio-base)
(require 'eieio-opt)

(eval-when-compile (require 'cl-lib))

(defclass class-a ()
  ()
  "Class A")

(defclass class-b ()
  ()
  "Class B")

(defclass class-ab (class-a class-b)
  ()
  "Class A and B combined.")

(defvar eitest-ab nil)
(setq eitest-ab (class-ab))

(message "(cl-typep eitest-ab 'class-a) => %S" (cl-typep eitest-ab 'class-a))

[-- Attachment #3: Type: text/plain, Size: 930 bytes --]


This test case is a reduction from
test/lisp/emacs-lisp/eieio-tests/eieio-tests.el.  You can see that tests
23 and 24 will fail when you do 'make eieio-tests TEST_LOAD_EL=no'.


In GNU Emacs 26.0.50 (build 64, x86_64-unknown-linux-gnu, X toolkit, Xaw scroll bars)
 of 2017-07-15 built on zony
Repository revision: b30ee0c9225bad6e3fd0b511a6c5d9a64b8fd66a
Windowing system distributor 'The X.Org Foundation', version 11.0.11901000
Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.

Configured using:
 'configure --cache-file=../debug-config.cache 'CFLAGS=-O0 -g3
 -march=native' --enable-checking=yes,glyphs
 --enable-check-lisp-object-type MAKEINFO=makeinfo-4.13a
 --with-x-toolkit=lucid --with-toolkit-scroll-bars --with-gif=no
 --with-jpeg=no'

Configured features:
XPM TIFF PNG RSVG SOUND GPM DBUS GSETTINGS NOTIFY ACL GNUTLS LIBXML2
FREETYPE XFT ZLIB TOOLKIT_SCROLL_BARS LUCID X11 LIBSYSTEMD

^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#27718: 26.0.50; (cl-typep instance-ab 'class-a) gives wrong answer when compiled
  2017-07-15 21:50 bug#27718: 26.0.50; (cl-typep instance-ab 'class-a) gives wrong answer when compiled npostavs
@ 2017-07-21 12:23 ` npostavs
  2017-07-21 13:19   ` Stefan Monnier
  0 siblings, 1 reply; 11+ messages in thread
From: npostavs @ 2017-07-21 12:23 UTC (permalink / raw)
  To: 27718; +Cc: Stefan Monnier, Alex

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

tags 27718 + patch
quit

npostavs@users.sourceforge.net writes:

>     ~/src/emacs$ .../emacs -Q -batch -f batch-byte-compile cl-typep-subclass.el
>     ~/src/emacs$ .../emacs -Q -batch -l cl-typep-subclass.el
>     (cl-typep eitest-ab ’class-a) => t
>     ~/src/emacs$ .../emacs -Q -batch -l cl-typep-subclass.elc
>     (cl-typep eitest-ab ’class-a) => nil

lisp/emacs-lisp/eieio.el:260:

    (defmacro defclass (name superclasses slots &rest options-and-doc)
      ...
           ;; When using typep, (typep OBJ 'myclass) returns t for objects which
           ;; are subclasses of myclass.  For our predicates, however, it is
           ;; important for EIEIO to be backwards compatible, where
           ;; myobject-p, and myobject-child-p are different.
           ;; "cl" uses this technique to specify symbols with specific typep
           ;; test, so we can let typep have the CLOS documented behavior
           ;; while keeping our above predicate clean.

           (put ',name 'cl-deftype-satisfies #',testsym2)

The problem is that the `put' doesn't take effect until runtime, so the
compiler inlines `cl-typep' incorrectly.  Using `function-put' here (and
`function-get' in `cl-typep') solves it (requires also the patches for
#27016[1]), although it seems a bit coincidental that `class-a' happens
to be a function as well, so I'm not sure if this is the right thing.
Stefan, any thoughts?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 2390 bytes --]

From 4ea34c9da04c8eabc754f2ca64ef4e2ac7c59cac Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Fri, 21 Jul 2017 07:59:21 -0400
Subject: [PATCH v1] Fix (cl-typep instance-ab 'class-a) compilation
 (Bug#27718)

Using `function-put' instead of `put' for the `cl-deftype-satisfies'
property lets the compiler know about it while compiling the file so
that any calls to `cl-typep' will be inlined correctly.
* lisp/emacs-lisp/eieio.el (defclass): Use `function-put' for the
`cl-deftype-satisfies' property.
* lisp/emacs-lisp/cl-macs.el (cl-typep): Also use `function-get' to
check the 'cl-deftype-satisfies' property.
---
 lisp/emacs-lisp/cl-macs.el | 7 +++++--
 lisp/emacs-lisp/eieio.el   | 2 +-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el
index 451e6490d7..f5a4f99aad 100644
--- a/lisp/emacs-lisp/cl-macs.el
+++ b/lisp/emacs-lisp/cl-macs.el
@@ -2991,8 +2991,11 @@ (define-inline cl-typep (val type)
       ((and (pred symbolp) type (guard (get type 'cl-deftype-handler)))
        (inline-quote
         (cl-typep ,val ',(funcall (get type 'cl-deftype-handler)))))
-      ((and (pred symbolp) type (guard (get type 'cl-deftype-satisfies)))
-       (inline-quote (funcall #',(get type 'cl-deftype-satisfies) ,val)))
+      ((and (pred symbolp) type (guard (or (function-get type 'cl-deftype-satisfies)
+                                           (get type 'cl-deftype-satisfies))))
+       (inline-quote (funcall #',(or (function-get type 'cl-deftype-satisfies)
+                                     (Get type 'cl-deftype-satisfies))
+                              ,val)))
       ((and (or 'nil 't) type) (inline-quote ',type))
       ((and (pred symbolp) type)
        (let* ((name (symbol-name type))
diff --git a/lisp/emacs-lisp/eieio.el b/lisp/emacs-lisp/eieio.el
index 1a7de55fce..2c333c16f4 100644
--- a/lisp/emacs-lisp/eieio.el
+++ b/lisp/emacs-lisp/eieio.el
@@ -246,7 +246,7 @@ (defmacro defclass (name superclasses slots &rest options-and-doc)
        ;; test, so we can let typep have the CLOS documented behavior
        ;; while keeping our above predicate clean.
 
-       (put ',name 'cl-deftype-satisfies #',testsym2)
+       (function-put ',name 'cl-deftype-satisfies #',testsym2)
 
        (eieio-defclass-internal ',name ',superclasses ',slots ',options-and-doc)
 
-- 
2.11.1


[-- Attachment #3: Type: text/plain, Size: 286 bytes --]


> You can see that tests 23 and 24 will fail when you do 'make
> eieio-tests TEST_LOAD_EL=no'.

This actually isn't true until the patch for #24402[2] is applied.

[1]: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27016#140
[2]: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=24402#71

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* bug#27718: 26.0.50; (cl-typep instance-ab 'class-a) gives wrong answer when compiled
  2017-07-21 12:23 ` npostavs
@ 2017-07-21 13:19   ` Stefan Monnier
  2017-07-22 17:51     ` npostavs
  0 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier @ 2017-07-21 13:19 UTC (permalink / raw)
  To: npostavs; +Cc: 27718, Alex

> The problem is that the `put' doesn't take effect until runtime, so the
> compiler inlines `cl-typep' incorrectly.  Using `function-put' here (and
> `function-get' in `cl-typep') solves it (requires also the patches for
> #27016[1]), although it seems a bit coincidental that `class-a' happens
> to be a function as well, so I'm not sure if this is the right thing.
> Stefan, any thoughts?

Hmm... I agree that using the same approach as that of bug#27016 is
right, but indeed `name` doesn't refer to a function but to the name of
a type (the property doesn't apply to the value stored in the
symbol-function cell and shouldn't follow alias definitions).

So maybe rather than hijack function-get/put in bug#27016, we should
introduce a new "definitional put" with a corresponding `get`, and then
use that here as well as in function-put/get.

Maybe we could simply change put&get directly without introducing
yet-another slightly different get/put?



        Stefan


> From 4ea34c9da04c8eabc754f2ca64ef4e2ac7c59cac Mon Sep 17 00:00:00 2001
> From: Noam Postavsky <npostavs@gmail.com>
> Date: Fri, 21 Jul 2017 07:59:21 -0400
> Subject: [PATCH v1] Fix (cl-typep instance-ab 'class-a) compilation
>  (Bug#27718)

> Using `function-put' instead of `put' for the `cl-deftype-satisfies'
> property lets the compiler know about it while compiling the file so
> that any calls to `cl-typep' will be inlined correctly.
> * lisp/emacs-lisp/eieio.el (defclass): Use `function-put' for the
> `cl-deftype-satisfies' property.
> * lisp/emacs-lisp/cl-macs.el (cl-typep): Also use `function-get' to
> check the 'cl-deftype-satisfies' property.
> ---
>  lisp/emacs-lisp/cl-macs.el | 7 +++++--
>  lisp/emacs-lisp/eieio.el   | 2 +-
>  2 files changed, 6 insertions(+), 3 deletions(-)

> diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el
> index 451e6490d7..f5a4f99aad 100644
> --- a/lisp/emacs-lisp/cl-macs.el
> +++ b/lisp/emacs-lisp/cl-macs.el
> @@ -2991,8 +2991,11 @@ (define-inline cl-typep (val type)
>        ((and (pred symbolp) type (guard (get type 'cl-deftype-handler)))
>         (inline-quote
>          (cl-typep ,val ',(funcall (get type 'cl-deftype-handler)))))
> -      ((and (pred symbolp) type (guard (get type 'cl-deftype-satisfies)))
> -       (inline-quote (funcall #',(get type 'cl-deftype-satisfies) ,val)))
> +      ((and (pred symbolp) type (guard (or (function-get type 'cl-deftype-satisfies)
> +                                           (get type 'cl-deftype-satisfies))))
> +       (inline-quote (funcall #',(or (function-get type 'cl-deftype-satisfies)
> +                                     (Get type 'cl-deftype-satisfies))
> +                              ,val)))
>        ((and (or 'nil 't) type) (inline-quote ',type))
>        ((and (pred symbolp) type)
>         (let* ((name (symbol-name type))
> diff --git a/lisp/emacs-lisp/eieio.el b/lisp/emacs-lisp/eieio.el
> index 1a7de55fce..2c333c16f4 100644
> --- a/lisp/emacs-lisp/eieio.el
> +++ b/lisp/emacs-lisp/eieio.el
> @@ -246,7 +246,7 @@ (defmacro defclass (name superclasses slots &rest options-and-doc)
>         ;; test, so we can let typep have the CLOS documented behavior
>         ;; while keeping our above predicate clean.
 
> -       (put ',name 'cl-deftype-satisfies #',testsym2)
> +       (function-put ',name 'cl-deftype-satisfies #',testsym2)
 
>         (eieio-defclass-internal ',name ',superclasses ',slots ',options-and-doc)
 
> -- 
> 2.11.1


>> You can see that tests 23 and 24 will fail when you do 'make
>> eieio-tests TEST_LOAD_EL=no'.

> This actually isn't true until the patch for #24402[2] is applied.

> [1]: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27016#140
> [2]: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=24402#71






^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#27718: 26.0.50; (cl-typep instance-ab 'class-a) gives wrong answer when compiled
  2017-07-21 13:19   ` Stefan Monnier
@ 2017-07-22 17:51     ` npostavs
  2017-07-24 15:30       ` Stefan Monnier
  0 siblings, 1 reply; 11+ messages in thread
From: npostavs @ 2017-07-22 17:51 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 27718, Alex

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

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

> So maybe rather than hijack function-get/put in bug#27016, we should
> introduce a new "definitional put" with a corresponding `get`, and then
> use that here as well as in function-put/get.
>
> Maybe we could simply change put&get directly without introducing
> yet-another slightly different get/put?

Something like this?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 2850 bytes --]

From 7645c7ff5078aa0bc937969fb1550b0f794d793e Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 22 Jul 2017 13:46:58 -0400
Subject: [PATCH] Let `put' take effect during compilation (Bug#27718)

* lisp/emacs-lisp/bytecomp.el: Use `byte-compile-function-put' to
handle `put' as well.
* src/fns.c (Fget): Consult `byte-compile-plist-environment'.
* lisp/subr.el (function-get): Let `get' take care of consuling
`byte-compile-plist-environment'.
---
 lisp/emacs-lisp/bytecomp.el |  1 +
 lisp/subr.el                |  7 +------
 src/fns.c                   | 10 ++++++++++
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index 4dda3bdc2b..3fead4229e 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -4719,6 +4719,7 @@ (defun byte-compile-form-make-variable-buffer-local (form)
   (byte-compile-keep-pending form 'byte-compile-normal-call))
 
 (put 'function-put 'byte-hunk-handler 'byte-compile-function-put)
+(put 'put 'byte-hunk-handler 'byte-compile-function-put)
 (defun byte-compile-function-put (form)
   (pcase form
     ((and `(,op ,fun ,prop ,val)
diff --git a/lisp/subr.el b/lisp/subr.el
index 3e4a3dedf5..42b4e1c211 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -2971,12 +2971,7 @@ (defun function-get (f prop &optional autoload)
 if it's an autoloaded macro."
   (let ((val nil))
     (while (and (symbolp f)
-                (null (setq val (or (plist-get
-                                     (alist-get
-                                      f (bound-and-true-p
-                                         byte-compile-plist-environment))
-                                     prop)
-                                    (get f prop))))
+                (null (setq val (get f prop)))
                 (fboundp f))
       (let ((fundef (symbol-function f)))
         (if (and autoload (autoloadp fundef)
diff --git a/src/fns.c b/src/fns.c
index d849618f2b..655422aa0f 100644
--- a/src/fns.c
+++ b/src/fns.c
@@ -1987,6 +1987,14 @@ DEFUN ("get", Fget, Sget, 2, 2, 0,
   (Lisp_Object symbol, Lisp_Object propname)
 {
   CHECK_SYMBOL (symbol);
+  Lisp_Object plist_env = find_symbol_value (Qbyte_compile_plist_environment);
+  if (CONSP (plist_env))
+    {
+      Lisp_Object propval = Fplist_get (CDR (Fassq (symbol, plist_env)),
+                                        propname);
+      if (!NILP (propval))
+        return propval;
+    }
   return Fplist_get (XSYMBOL (symbol)->plist, propname);
 }
 
@@ -5125,6 +5133,8 @@ syms_of_fns (void)
   DEFSYM (Qkey_or_value, "key-or-value");
   DEFSYM (Qkey_and_value, "key-and-value");
 
+  DEFSYM (Qbyte_compile_plist_environment, "byte-compile-plist-environment");
+
   defsubr (&Ssxhash_eq);
   defsubr (&Ssxhash_eql);
   defsubr (&Ssxhash_equal);
-- 
2.11.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* bug#27718: 26.0.50; (cl-typep instance-ab 'class-a) gives wrong answer when compiled
  2017-07-22 17:51     ` npostavs
@ 2017-07-24 15:30       ` Stefan Monnier
  2017-07-24 19:44         ` Stefan Monnier
  0 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier @ 2017-07-24 15:30 UTC (permalink / raw)
  To: npostavs; +Cc: 27718, Alex

>> So maybe rather than hijack function-get/put in bug#27016, we should
>> introduce a new "definitional put" with a corresponding `get`, and then
>> use that here as well as in function-put/get.
>> Maybe we could simply change put&get directly without introducing
>> yet-another slightly different get/put?

> Something like this?

I guess so.  But:

> +  DEFSYM (Qbyte_compile_plist_environment, "byte-compile-plist-environment");

I find having to hardcode byte-compile-plist-environment in get rather ugly.


        Stefan "sorry, I couldn't make it more constructive"





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#27718: 26.0.50; (cl-typep instance-ab 'class-a) gives wrong answer when compiled
  2017-07-24 15:30       ` Stefan Monnier
@ 2017-07-24 19:44         ` Stefan Monnier
  2017-07-25 16:34           ` Noam Postavsky
  0 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier @ 2017-07-24 19:44 UTC (permalink / raw)
  To: npostavs; +Cc: 27718, Alex

>> +  DEFSYM (Qbyte_compile_plist_environment, "byte-compile-plist-environment");

> I find having to hardcode byte-compile-plist-environment in get rather ugly.

Hmm... I'm still not quite sure how best to deal with this, but it does
occur to me that we could introduce a new `define-propval' value, which
would be like `put' but for use at top-level.  It could differ from
`put' in that the byte-compiler could keep track of those in
byte-compile-plist-environment, and that they could record themselves in
load-history for unload-feature to be able to undo them.

It doesn't help on the `get' side, tho.


        Stefan





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#27718: 26.0.50; (cl-typep instance-ab 'class-a) gives wrong answer when compiled
  2017-07-24 19:44         ` Stefan Monnier
@ 2017-07-25 16:34           ` Noam Postavsky
  2017-07-25 21:21             ` Stefan Monnier
  0 siblings, 1 reply; 11+ messages in thread
From: Noam Postavsky @ 2017-07-25 16:34 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 27718, Alex

On Mon, Jul 24, 2017 at 3:44 PM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
>>> +  DEFSYM (Qbyte_compile_plist_environment, "byte-compile-plist-environment");
>
>> I find having to hardcode byte-compile-plist-environment in get rather ugly.
>
> Hmm... I'm still not quite sure how best to deal with this, but it does
> occur to me that we could introduce a new `define-propval' value, which
> would be like `put' but for use at top-level.  It could differ from
> `put' in that the byte-compiler could keep track of those in
> byte-compile-plist-environment, and that they could record themselves in
> load-history for unload-feature to be able to undo them.
>
> It doesn't help on the `get' side, tho.

Right. We need `get' to be aware of what the compiler has read, so it
will need to have a reference to the compiler in there somewhere. We
could name it differently and point the reference in the other
direction, as in DEFSYM (Qalternate_plist_environment...) and have the
byte compiler use that instead, which might be slightly more
aesthetically pleasing. The other possibility is to introduce a
different kind of `get' and use that in places that need to be aware
of what the compiler has read.

I don't see a third option.





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#27718: 26.0.50; (cl-typep instance-ab 'class-a) gives wrong answer when compiled
  2017-07-25 16:34           ` Noam Postavsky
@ 2017-07-25 21:21             ` Stefan Monnier
  2017-08-05  1:06               ` npostavs
  0 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier @ 2017-07-25 21:21 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: 27718, Alex

> Right. We need `get' to be aware of what the compiler has read, so it
> will need to have a reference to the compiler in there somewhere. We
> could name it differently and point the reference in the other
> direction, as in DEFSYM (Qalternate_plist_environment...) and have the
> byte compiler use that instead, which might be slightly more
> aesthetically pleasing. The other possibility is to introduce a
> different kind of `get' and use that in places that need to be aware
> of what the compiler has read.

> I don't see a third option.

Right, so we have 3 options, indeed:
- `get` looks up to byte-compile-plist-environment
- byte-compile manipulates `get`s overriding-plist-environment
- provide a new get-like function.

Either one is fine by me,


        Stefan





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#27718: 26.0.50; (cl-typep instance-ab 'class-a) gives wrong answer when compiled
  2017-07-25 21:21             ` Stefan Monnier
@ 2017-08-05  1:06               ` npostavs
  2017-08-05 12:49                 ` Stefan Monnier
  0 siblings, 1 reply; 11+ messages in thread
From: npostavs @ 2017-08-05  1:06 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 27718, Alex

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

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> Right, so we have 3 options, indeed:
> - `get` looks up to byte-compile-plist-environment
> - byte-compile manipulates `get`s overriding-plist-environment
> - provide a new get-like function.
>
> Either one is fine by me,

I'm thinking the 2nd looks best.  Is it correct to `put'
`byte-compile-define-symbol-prop' as the hunk handler for both
`define-symbol-prop' and `function-put'?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 5388 bytes --]

From 3e8759320ecba4156cd3315ef02ea902ff408ae3 Mon Sep 17 00:00:00 2001
From: Stefan Monnier <monnier@IRO.UMontreal.CA>
Date: Fri, 14 Jul 2017 00:32:34 -0400
Subject: [PATCH v2 1/2] Let `define-symbol-prop' take effect during
 compilation

* src/fns.c (syms_of_fns): New variable `overriding-plist-environment'.
(Fget): Consult it.
* lisp/emacs-lisp/bytecomp.el (byte-compile-close-variables): Let-bind
it to nil.
(byte-compile-define-symbol-prop): New function, handles compilation
of top-level `define-symbol-prop' and `function-put' calls by putting
the symbol setting into `overriding-plist-environment'.

Co-authored-by: Noam Postavsky <npostavs@gmail.com>
---
 lisp/emacs-lisp/bytecomp.el            | 29 +++++++++++++++++++++++++++++
 src/fns.c                              | 11 +++++++++++
 test/lisp/emacs-lisp/bytecomp-tests.el | 17 +++++++++++++++++
 3 files changed, 57 insertions(+)

diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index fdd4276e4e..a1626c0b9d 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -1572,6 +1572,7 @@ (defmacro byte-compile-close-variables (&rest body)
           ;; macroenvironment.
           (copy-alist byte-compile-initial-macro-environment))
          (byte-compile--outbuffer nil)
+         (overriding-plist-environment nil)
          (byte-compile-function-environment nil)
          (byte-compile-bound-variables nil)
          (byte-compile-lexical-variables nil)
@@ -4712,6 +4713,34 @@ (put 'make-variable-buffer-local
      'byte-hunk-handler 'byte-compile-form-make-variable-buffer-local)
 (defun byte-compile-form-make-variable-buffer-local (form)
   (byte-compile-keep-pending form 'byte-compile-normal-call))
+
+(put 'function-put 'byte-hunk-handler 'byte-compile-define-symbol-prop)
+(put 'define-symbol-prop 'byte-hunk-handler 'byte-compile-define-symbol-prop)
+(defun byte-compile-define-symbol-prop (form)
+  (pcase form
+    ((and `(,op ,fun ,prop ,val)
+          (guard (and (macroexp-const-p fun)
+                      (macroexp-const-p prop)
+                      (or (macroexp-const-p val)
+                          ;; Also accept anonymous functions, since
+                          ;; we're at top-level which implies they're
+                          ;; also constants.
+                          (pcase val (`(function (lambda . ,_)) t))))))
+     (byte-compile-push-constant op)
+     (byte-compile-form fun)
+     (byte-compile-form prop)
+     (let* ((fun (eval fun))
+            (prop (eval prop))
+            (val (if (macroexp-const-p val)
+                     (eval val)
+                   (byte-compile-lambda (cadr val)))))
+       (push `(,fun
+               . (,prop ,val ,@(alist-get fun overriding-plist-environment)))
+             overriding-plist-environment)
+       (byte-compile-push-constant val)
+       (byte-compile-out 'byte-call 3)))
+
+    (_ (byte-compile-keep-pending form))))
 \f
 ;;; tags
 
diff --git a/src/fns.c b/src/fns.c
index d849618f2b..00b6ed6a28 100644
--- a/src/fns.c
+++ b/src/fns.c
@@ -1987,6 +1987,10 @@ DEFUN ("get", Fget, Sget, 2, 2, 0,
   (Lisp_Object symbol, Lisp_Object propname)
 {
   CHECK_SYMBOL (symbol);
+  Lisp_Object propval = Fplist_get (CDR (Fassq (symbol, Voverriding_plist_environment)),
+                                    propname);
+  if (!NILP (propval))
+    return propval;
   return Fplist_get (XSYMBOL (symbol)->plist, propname);
 }
 
@@ -5163,6 +5167,13 @@ syms_of_fns (void)
   DEFSYM (Qcursor_in_echo_area, "cursor-in-echo-area");
   DEFSYM (Qwidget_type, "widget-type");
 
+  DEFVAR_LISP ("overriding-plist-environment", Voverriding_plist_environment,
+               doc: /* An alist overrides the plists of the symbols which it lists.
+Used by the byte-compiler to apply `define-symbol-prop' during
+compilation.  */);
+  Voverriding_plist_environment = Qnil;
+  DEFSYM (Qoverriding_plist_environment, "overriding-plist-environment");
+
   staticpro (&string_char_byte_cache_string);
   string_char_byte_cache_string = Qnil;
 
diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el
index d15bd8b6e6..8ef2ce7025 100644
--- a/test/lisp/emacs-lisp/bytecomp-tests.el
+++ b/test/lisp/emacs-lisp/bytecomp-tests.el
@@ -545,6 +545,23 @@ (ert-deftest bytecomp-tests--old-style-backquotes ()
 This functionality has been obsolete for more than 10 years already
 and will be removed soon.  See (elisp)Backquote in the manual.")))))))
 
+
+(ert-deftest bytecomp-tests-function-put ()
+  "Check `function-put' operates during compilation."
+  (should (boundp 'lread--old-style-backquotes))
+  (bytecomp-tests--with-temp-file source
+    (dolist (form '((function-put 'bytecomp-tests--foo 'foo 1)
+                    (function-put 'bytecomp-tests--foo 'bar 2)
+                    (defmacro bytecomp-tests--foobar ()
+                      `(cons ,(function-get 'bytecomp-tests--foo 'foo)
+                             ,(function-get 'bytecomp-tests--foo 'bar)))
+                    (defvar bytecomp-tests--foobar 1)
+                    (setq bytecomp-tests--foobar (bytecomp-tests--foobar))))
+      (print form (current-buffer)))
+    (write-region (point-min) (point-max) source nil 'silent)
+    (byte-compile-file source t)
+    (should (equal bytecomp-tests--foobar (cons 1 2)))))
+
 ;; Local Variables:
 ;; no-byte-compile: t
 ;; End:
-- 
2.11.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: patch --]
[-- Type: text/x-diff, Size: 1014 bytes --]

From 848a5eb1ed15bb975fe308464b2891b160c5a420 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Fri, 4 Aug 2017 19:50:21 -0400
Subject: [PATCH v2 2/2] Let the cl-typep effects of defclass work during
 compilation (Bug#27718)

* lisp/emacs-lisp/eieio.el (defclass): Use `define-symbol-prop'
instead of `put'.
---
 lisp/emacs-lisp/eieio.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/eieio.el b/lisp/emacs-lisp/eieio.el
index 1a7de55fce..8b92d5b7ac 100644
--- a/lisp/emacs-lisp/eieio.el
+++ b/lisp/emacs-lisp/eieio.el
@@ -246,7 +246,7 @@ (defmacro defclass (name superclasses slots &rest options-and-doc)
        ;; test, so we can let typep have the CLOS documented behavior
        ;; while keeping our above predicate clean.
 
-       (put ',name 'cl-deftype-satisfies #',testsym2)
+       (define-symbol-prop ',name 'cl-deftype-satisfies #',testsym2)
 
        (eieio-defclass-internal ',name ',superclasses ',slots ',options-and-doc)
 
-- 
2.11.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* bug#27718: 26.0.50; (cl-typep instance-ab 'class-a) gives wrong answer when compiled
  2017-08-05  1:06               ` npostavs
@ 2017-08-05 12:49                 ` Stefan Monnier
  2017-08-08  1:16                   ` npostavs
  0 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier @ 2017-08-05 12:49 UTC (permalink / raw)
  To: npostavs; +Cc: 27718, Alex

>> Right, so we have 3 options, indeed:
>> - `get` looks up to byte-compile-plist-environment
>> - byte-compile manipulates `get`s overriding-plist-environment
>> - provide a new get-like function.
>> Either one is fine by me,
> I'm thinking the 2nd looks best.  Is it correct to `put'
> `byte-compile-define-symbol-prop' as the hunk handler for both
> `define-symbol-prop' and `function-put'?

I think so, yes.


        Stefan





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#27718: 26.0.50; (cl-typep instance-ab 'class-a) gives wrong answer when compiled
  2017-08-05 12:49                 ` Stefan Monnier
@ 2017-08-08  1:16                   ` npostavs
  0 siblings, 0 replies; 11+ messages in thread
From: npostavs @ 2017-08-08  1:16 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 27718, Alex

tags 27718 fixed
close 27718 26.1
quit

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>>> Right, so we have 3 options, indeed:
>>> - `get` looks up to byte-compile-plist-environment
>>> - byte-compile manipulates `get`s overriding-plist-environment
>>> - provide a new get-like function.
>>> Either one is fine by me,
>> I'm thinking the 2nd looks best.  Is it correct to `put'
>> `byte-compile-define-symbol-prop' as the hunk handler for both
>> `define-symbol-prop' and `function-put'?
>
> I think so, yes.

Pushed to master.

[1: cc30d77ecd]: 2017-08-07 18:54:49 -0400
  Let `define-symbol-prop' take effect during compilation
  http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=cc30d77ecdd1b9155ade3d0656a84a0839ee2795

[2: b5c8e9898d]: 2017-08-07 18:54:49 -0400
  Let the cl-typep effects of defclass work during compilation (Bug#27718)
  http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=b5c8e9898d9dbd4145c40d08e8eef84a5e32008a





^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-08-08  1:16 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-15 21:50 bug#27718: 26.0.50; (cl-typep instance-ab 'class-a) gives wrong answer when compiled npostavs
2017-07-21 12:23 ` npostavs
2017-07-21 13:19   ` Stefan Monnier
2017-07-22 17:51     ` npostavs
2017-07-24 15:30       ` Stefan Monnier
2017-07-24 19:44         ` Stefan Monnier
2017-07-25 16:34           ` Noam Postavsky
2017-07-25 21:21             ` Stefan Monnier
2017-08-05  1:06               ` npostavs
2017-08-05 12:49                 ` Stefan Monnier
2017-08-08  1:16                   ` npostavs

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).