all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* byte-compile-maybe-guarded
@ 2008-03-01 20:07 Glenn Morris
  2008-03-02  1:00 ` byte-compile-maybe-guarded Dan Nicolaescu
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Glenn Morris @ 2008-03-01 20:07 UTC (permalink / raw)
  To: Dan Nicolaescu; +Cc: emacs-devel


Dan,


FYI I've restored the code in byte-compile-maybe-guarded that you
commented out 2007-11-10. Without this code, a false warning is
produced in the following case:

foo.el:
(defsubst foo ()
  (if (featurep 'xemacs) (setq foo t)))   ; foo is a free variable
(provide 'foo)

bar.el:
(require 'foo)
(foo)


emacs -Q -batch --eval "(add-to-list 'load-path default-directory)" \
   -f batch-byte-compile bar.el

In toplevel form:
bar.el:2:2:Warning: assignment to free variable `foo'




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

* Re: byte-compile-maybe-guarded
  2008-03-01 20:07 byte-compile-maybe-guarded Glenn Morris
@ 2008-03-02  1:00 ` Dan Nicolaescu
  2008-03-02  5:50 ` byte-compile-maybe-guarded Stefan Monnier
  2008-03-02 22:38 ` byte-compile-maybe-guarded Stefan Monnier
  2 siblings, 0 replies; 5+ messages in thread
From: Dan Nicolaescu @ 2008-03-02  1:00 UTC (permalink / raw)
  To: Glenn Morris; +Cc: emacs-devel

Glenn Morris <rgm@gnu.org> writes:

  > FYI I've restored the code in byte-compile-maybe-guarded that you
  > commented out 2007-11-10. Without this code, a false warning is
  > produced in the following case:

Although restoring that code fixes the warning that you showed, is it
the right thing to do?

  > foo.el:
  > (defsubst foo ()
  >   (if (featurep 'xemacs) (setq foo t)))   ; foo is a free variable
  > (provide 'foo)
  > 
  > bar.el:
  > (require 'foo)
  > (foo)
  > 
  > 
  > emacs -Q -batch --eval "(add-to-list 'load-path default-directory)" \
  >    -f batch-byte-compile bar.el

If the defsubst in foo.el to is:

(defsubst foo ()
  (if (and t (featurep 'xemacs)) (setq foo t)))   ; foo is a free variable

the warning is still produced even after your change...





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

* Re: byte-compile-maybe-guarded
  2008-03-01 20:07 byte-compile-maybe-guarded Glenn Morris
  2008-03-02  1:00 ` byte-compile-maybe-guarded Dan Nicolaescu
@ 2008-03-02  5:50 ` Stefan Monnier
  2008-03-03  2:01   ` byte-compile-maybe-guarded Glenn Morris
  2008-03-02 22:38 ` byte-compile-maybe-guarded Stefan Monnier
  2 siblings, 1 reply; 5+ messages in thread
From: Stefan Monnier @ 2008-03-02  5:50 UTC (permalink / raw)
  To: Glenn Morris; +Cc: Dan Nicolaescu, emacs-devel

> FYI I've restored the code in byte-compile-maybe-guarded that you
> commented out 2007-11-10.

I don't think it's the right place to fix it.
But at least, now we can add a clear comment describing the problem.


        Stefan




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

* Re: byte-compile-maybe-guarded
  2008-03-01 20:07 byte-compile-maybe-guarded Glenn Morris
  2008-03-02  1:00 ` byte-compile-maybe-guarded Dan Nicolaescu
  2008-03-02  5:50 ` byte-compile-maybe-guarded Stefan Monnier
@ 2008-03-02 22:38 ` Stefan Monnier
  2 siblings, 0 replies; 5+ messages in thread
From: Stefan Monnier @ 2008-03-02 22:38 UTC (permalink / raw)
  To: Glenn Morris; +Cc: Dan Nicolaescu, emacs-devel

> FYI I've restored the code in byte-compile-maybe-guarded that you
> commented out 2007-11-10. Without this code, a false warning is
> produced in the following case:

> foo.el:
> (defsubst foo ()
>   (if (featurep 'xemacs) (setq foo t)))   ; foo is a free variable
> (provide 'foo)

> bar.el:
> (require 'foo)
> (foo)


It seems the problem is in the (lack of) optimization of forms like

  ((lambda (args) body) exps)

You can see it by byte-compiling

  (defun foo ()
    ((lambda () (if (featurep 'xemacs) (setq foo t)))))

I.e. the patch below seems to fix the problem the right way.
I'm just not 100% sure it can't introduce an infinite optimization loop.


        Stefan


--- orig/lisp/emacs-lisp/byte-opt.el
+++ mod/lisp/emacs-lisp/byte-opt.el
@@ -383,7 +383,9 @@
 		form))
 	  ((or (byte-code-function-p fn)
 	       (eq 'lambda (car-safe fn)))
-	   (byte-compile-unfold-lambda form))
+           (byte-optimize-form
+            (byte-compile-unfold-lambda form)
+            for-effect))
 	  ((memq fn '(let let*))
 	   ;; recursively enter the optimizer for the bindings and body
 	   ;; of a let or let*.  This for depth-firstness: forms that




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

* Re: byte-compile-maybe-guarded
  2008-03-02  5:50 ` byte-compile-maybe-guarded Stefan Monnier
@ 2008-03-03  2:01   ` Glenn Morris
  0 siblings, 0 replies; 5+ messages in thread
From: Glenn Morris @ 2008-03-03  2:01 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Dan Nicolaescu, emacs-devel

Stefan Monnier wrote:

>> FYI I've restored the code in byte-compile-maybe-guarded that you
>> commented out 2007-11-10.
>
> I don't think it's the right place to fix it.
> But at least, now we can add a clear comment describing the problem.

I confess I didn't think about it much. I thought Dan commented it out
because it was doing nothing, so when I saw it was doing something, I
put it back.




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

end of thread, other threads:[~2008-03-03  2:01 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-03-01 20:07 byte-compile-maybe-guarded Glenn Morris
2008-03-02  1:00 ` byte-compile-maybe-guarded Dan Nicolaescu
2008-03-02  5:50 ` byte-compile-maybe-guarded Stefan Monnier
2008-03-03  2:01   ` byte-compile-maybe-guarded Glenn Morris
2008-03-02 22:38 ` byte-compile-maybe-guarded Stefan Monnier

Code repositories for project(s) associated with this external index

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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.