bug#27177: 26.0.50: Macroexpanding cl-loop and friends (make-symbol usage)

bug#27177: 26.0.50: Macroexpanding cl-loop and friends (make-symbol usage)

[Alex]

Consider the following simple cl-loop form:

(cl-loop for x in '(1 2 3)
         for y in '(a b c)
         collect (list x y))


The macroexpanded result is:

(cl-block nil
  (let*
      ((--cl-var--
        '(1 2 3))
       (x nil)
       (--cl-var--
        '(a b c))
       (y nil)
       (--cl-var-- nil))
    (while
        (and
         (consp --cl-var--)
         (progn
           (setq x
                 (car --cl-var--))
           (consp --cl-var--)))
      (setq y
            (car --cl-var--))
      (push
       (list x y)
       --cl-var--)
      (setq --cl-var--
            (cdr --cl-var--))
      (setq --cl-var--
            (cdr --cl-var--)))
    (nreverse --cl-var--)))


It's easy to verify that this expansion doesn't do the same job by
noticing that the macroexpanded form always returns nil.

Note that in Common Lisp (at least in SBCL), macroexpanding and then
evaluating the result works as expected.

This is because cl-macs.el uses make-symbol instead of gensym, like SBCL
does.

Should cl-loop and friends use cl-gensym? One possible disadvantage to
that approach is that since Emacs Lisp lacks CL's bignums, it could lead
to some issues past most-positive-fixnum.

The reason I reported this is actually because I found it difficult to
debug macroexpanded cl-loop forms when all of the uninterned symbols had
the same representation. Using cl-gensym would help with debugging.

bug#27177: 26.0.50: Macroexpanding cl-loop and friends (make-symbol usage)

[Michael Heerdegen]

Alex <agrambot@gmail.com> writes:

> Consider the following simple cl-loop form:
>
> (cl-loop for x in '(1 2 3)
>          for y in '(a b c)
>          collect (list x y))
>
>
> The macroexpanded result is:
>
> (cl-block nil
>   (let*
>       ((--cl-var--
>         '(1 2 3))
>        (x nil)
>        (--cl-var--
>         '(a b c))
>        (y nil)
>        (--cl-var-- nil))
>     (while
>         (and
>          (consp --cl-var--)
>          (progn
>            (setq x
>                  (car --cl-var--))
>            (consp --cl-var--)))
>       (setq y
>             (car --cl-var--))
>       (push
>        (list x y)
>        --cl-var--)
>       (setq --cl-var--
>             (cdr --cl-var--))
>       (setq --cl-var--
>             (cdr --cl-var--)))
>     (nreverse --cl-var--)))
>
>
> It's easy to verify that this expansion doesn't do the same job by
> noticing that the macroexpanded form always returns nil.
>
> Note that in Common Lisp (at least in SBCL), macroexpanding and then
> evaluating the result works as expected.

Also in Elisp:

(eval (macroexpand '(cl-loop for x in '(1 2 3)
                        for y in '(a b c)
                        collect (list x y))))
==>
((1 a)
 (2 b)
 (3 c))

> This is because cl-macs.el uses make-symbol instead of gensym, like SBCL
> does.

Note that `make-symbol' doesn't return an interned symbol - what is
printed as "--cl-var--" above are actually different symbols.  You need
to enable `print-gensym' to make that visible when printing the
macroexpansion.  If you print with print-gensym bound to nil, you don't
get a correct printed representation.

So I think there is not a bug, unless your complaint is about human
readability or the default value of `print-gensym'.


Michael.

bug#27177: 26.0.50: Macroexpanding cl-loop and friends (make-symbol usage)

[Alex]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Also in Elisp:
>
> (eval (macroexpand '(cl-loop for x in '(1 2 3)
>                         for y in '(a b c)
>                         collect (list x y))))
> ==>
> ((1 a)
>  (2 b)
>  (3 c))

Right, sorry for not being clear. I was referring to printing the output
into a buffer and evaluating that instead of feeding it directly into
eval.

Though I was wrong that SBCL does this correctly by default (you have to
set *print-gensym* to nil).

> Note that `make-symbol' doesn't return an interned symbol - what is
> printed as "--cl-var--" above are actually different symbols.  You need
> to enable `print-gensym' to make that visible when printing the
> macroexpansion.  If you print with print-gensym bound to nil, you don't
> get a correct printed representation.

Enabling print-gensym in this case is even worse, since evaluating the
macroexpanded code yields a (void-variable #:--cl-var--) error.

> So I think there is not a bug, unless your complaint is about human
> readability or the default value of `print-gensym'.

Human readability is indeed a large part of my complaint.

Also, evaluating the macroexpanded code directly would allow for easier
debugging, since one could make small adjustments to the output to see
immediate results.

bug#27177: 26.0.50: Macroexpanding cl-loop and friends (make-symbol usage)

[npostavs]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> You need to enable `print-gensym' to make that visible when printing
> the macroexpansion.  If you print with print-gensym bound to nil, you
> don't get a correct printed representation.

You need `print-circle' as well.

bug#27177: 26.0.50: Macroexpanding cl-loop and friends (make-symbol usage)

[npostavs]

Alex <agrambot@gmail.com> writes:

> Enabling print-gensym in this case is even worse, since evaluating the
> macroexpanded code yields a (void-variable #:--cl-var--) error.

Yeah, that's why you need `print-circle' too.

> Also, evaluating the macroexpanded code directly would allow for easier
> debugging, since one could make small adjustments to the output to see
> immediate results.

With print-circle, the output is debuggable in Emacs 26 (see Bug#23660).

bug#27177: 26.0.50: Macroexpanding cl-loop and friends (make-symbol usage)

[Michael Heerdegen]

Alex <agrambot@gmail.com> writes:

> > Note that `make-symbol' doesn't return an interned symbol - what is
> > printed as "--cl-var--" above are actually different symbols.  You need
> > to enable `print-gensym' to make that visible when printing the
> > macroexpansion.  If you print with print-gensym bound to nil, you don't
> > get a correct printed representation.
>
> Enabling print-gensym in this case is even worse, since evaluating the
> macroexpanded code yields a (void-variable #:--cl-var--) error.

Oh, you need to bind `print-circle' as well, thanks npostavs.

> > So I think there is not a bug, unless your complaint is about human
> > readability or the default value of `print-gensym'.
>
> Human readability is indeed a large part of my complaint.

> Also, evaluating the macroexpanded code directly would allow for easier
> debugging, since one could make small adjustments to the output to see
> immediate results.

With print-circle and print-gensym bound, I think the result does not
really read worse than how it would look like with with `cl-gensym'
generated code.


Michael.

bug#27177: 26.0.50: Macroexpanding cl-loop and friends (make-symbol usage)

[Alex]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> With print-circle and print-gensym bound, I think the result does not
> really read worse than how it would look like with with `cl-gensym'
> generated code.

That's much better, though I still think it could/should be better. For
example, if you have multiple uninterned symbols with different
symbol-names, they're all referenced by #number, and use the same
counter.

It also seems to make the output uglier as well. Consider:

(macroexpand '(cl-loop for x in '(1 2 3)
                       for y in '(a b c)
                       repeat 10
                       repeat 20
                       collect (list x y)))

Note the expressions using #5#. I suppose the 0 is being shared.

It would also be nice if instead of many --cl-var-- variables,
particular clauses would result in different symbol-names. For instance,
if the `repeat' clause made a symbol called --cl-repeat--. This would
further help readability.

Also, using gensym could help 3rd-party packages. I usually use
macrostep to expand macros and the value of print-circle has no effect
on its expansions. macrostep individually prints out each uninterned
symbol using prin1; can this approach be easily modified to get the same
result as macroexpand?

PS: The first line of the documentation of print-circle only mentions
that it affects recursive structures. Perhaps it should mention the
"shared substructures" part in the first line for emphasis?

bug#27177: 26.0.50: Macroexpanding cl-loop and friends (make-symbol usage)

[Michael Heerdegen]

Alex <agrambot@gmail.com> writes:

> That's much better, though I still think it could/should be better. For
> example, if you have multiple uninterned symbols with different
> symbol-names, they're all referenced by #number, and use the same
> counter.
>
> It also seems to make the output uglier as well. Consider:
>
> (macroexpand '(cl-loop for x in '(1 2 3)
>                        for y in '(a b c)
>                        repeat 10
>                        repeat 20
>                        collect (list x y)))
>
> Note the expressions using #5#. I suppose the 0 is being shared.

Yes, or at least, a cons containing the 0.  That's how things look like
with print-circle enabled.

> It would also be nice if instead of many --cl-var-- variables,
> particular clauses would result in different symbol-names. For instance,
> if the `repeat' clause made a symbol called --cl-repeat--. This would
> further help readability.

I'm not sure if that is doable without rewriting the implementation,
since the macro expansion is automatically written code.

> Also, using gensym could help 3rd-party packages. I usually use
> macrostep to expand macros and the value of print-circle has no effect
> on its expansions. macrostep individually prints out each uninterned
> symbol using prin1; can this approach be easily modified to get the same
> result as macroexpand?

AFAICT `print-circle' and `print-gensym' also control how `prin1'
prints.


Note that when we changed the code to use `cl-gensym', we would not have
a really clean solution for the readability problem: if you print with
p-gensym and p-circle on, it would not look much different than now.  If
you print with those flags off, you (still) print to different (not
equivalent) code: when you read (evaluate) it, all uninterned symbols
would be replaced with interned symbols.  Though, with numbered symbol
names, you will be probably be lucky in most cases that the difference
doesn't matter.

But I see your point: the readability is a real problem.  Maybe we could
instead improve how things are printed?  Unfortunately that lies beyond
my knowledge.


> PS: The first line of the documentation of print-circle only mentions
> that it affects recursive structures. Perhaps it should mention the
> "shared substructures" part in the first line for emphasis?

I agree but somewhat hesitate because of the variable's name, which is
even more a source of confusion.


Michael.

bug#27177: 26.0.50: Macroexpanding cl-loop and friends (make-symbol usage)

[Alex]

Michael Heerdegen <michael_heerdegen@web.de> writes:

>> It would also be nice if instead of many --cl-var-- variables,
>> particular clauses would result in different symbol-names. For instance,
>> if the `repeat' clause made a symbol called --cl-repeat--. This would
>> further help readability.
>
> I'm not sure if that is doable without rewriting the implementation,
> since the macro expansion is automatically written code.

I'm not sure what you mean. Since they're uninterned symbols their
symbol-names shouldn't matter unless a procedure calls #'symbol-name on
them. In cl-macs.el I don't see any examples of that.

It appears that the code sometimes does use different names already in a
couple places. For example, see --cl-vec-- and --cl-idx-- at about line
1294 in cl-macs.el.

Actually, looking at the git-blame for those lines, it looks like Stefan
switched from gensym to make-symbol all the way back in e542ea4bed!

Stefan, why did you make the switch? Using cl-gensym would help a ton
with readability of cl-loop's macroexpansion.

>> Also, using gensym could help 3rd-party packages. I usually use
>> macrostep to expand macros and the value of print-circle has no effect
>> on its expansions. macrostep individually prints out each uninterned
>> symbol using prin1; can this approach be easily modified to get the same
>> result as macroexpand?
>
> AFAICT `print-circle' and `print-gensym' also control how `prin1'
> prints.

Does print-circle? Consider:

(prin1 `(cons ,(make-symbol "hello")
	      ,(make-symbol "hello")))

print-gensym certainly makes a difference in the output, but
print-circle doesn't seem to.

However, I don't know how prin1 would keep track of the uninterned
symbols across many different procedure calls, which it would need to do
for it to know what is being shared.

> Note that when we changed the code to use `cl-gensym', we would not have
> a really clean solution for the readability problem: if you print with
> p-gensym and p-circle on, it would not look much different than now.  If
> you print with those flags off, you (still) print to different (not
> equivalent) code: when you read (evaluate) it, all uninterned symbols
> would be replaced with interned symbols.  Though, with numbered symbol
> names, you will be probably be lucky in most cases that the difference
> doesn't matter.

In the case with both flags off and with make-symbol calls changed to
cl-gensym, I got:

(cl-block nil
  (let*
      ((--cl-var--248
        '(1 2 3))
       (x nil)
       (--cl-var--249
        '(a b c))
       (y nil)
       (--cl-var--250 nil))
    (while
        (and
         (consp --cl-var--248)
         (progn
           (setq x
                 (car --cl-var--248))
           (consp --cl-var--249)))
      (setq y
            (car --cl-var--249))
      (push
       (list x y)
       --cl-var--250)
      (setq --cl-var--248
            (cdr --cl-var--248))
      (setq --cl-var--249
            (cdr --cl-var--249)))
    (nreverse --cl-var--250)))

Which is equivalent code to the original cl-loop. I also believe it's
much more readable than the current macroexpansion as you can actually
differentiate between the different --cl-var-- variables. Using
different symbol names as discussed above would help a lot as well, but
I still think this part is important as well.

> But I see your point: the readability is a real problem.  Maybe we could
> instead improve how things are printed?  Unfortunately that lies beyond
> my knowledge.

I'm not sure what you mean, but improvement is certainly welcome.

>> PS: The first line of the documentation of print-circle only mentions
>> that it affects recursive structures. Perhaps it should mention the
>> "shared substructures" part in the first line for emphasis?
>
> I agree but somewhat hesitate because of the variable's name, which is
> even more a source of confusion.

Right, I was a bit confused as well, but according to the Hyperspec
*print-circle* also detects sharing in objects in Common Lisp as well.

bug#27177: 26.0.50: Macroexpanding cl-loop and friends (make-symbol usage)

[Michael Heerdegen]

Alex <agrambot@gmail.com> writes:

> > AFAICT `print-circle' and `print-gensym' also control how `prin1'
> > prints.
>
> Does print-circle? Consider:
>
> (prin1 `(cons ,(make-symbol "hello")
>               ,(make-symbol "hello")))
>
> print-gensym certainly makes a difference in the output, but
> print-circle doesn't seem to.

There are no shared parts in your example expression, so it doesn't make
a difference in this particular case.

> However, I don't know how prin1 would keep track of the uninterned
> symbols across many different procedure calls, which it would need to do
> for it to know what is being shared.

It doesn't AFAICT.


Michael.

bug#27177: 26.0.50: Macroexpanding cl-loop and friends (make-symbol usage)

[npostavs]

Alex <agrambot@gmail.com> writes:

>> AFAICT `print-circle' and `print-gensym' also control how `prin1'
>> prints.
>
> Does print-circle? Consider:
>
> (prin1 `(cons ,(make-symbol "hello")
> 	      ,(make-symbol "hello")))
>
> print-gensym certainly makes a difference in the output, but
> print-circle doesn't seem to.

You're producing 2 different symbols, try

    (let ((sym (make-symbol "hello")))
      (prin1 `(cons ,sym ,sym)))

> However, I don't know how prin1 would keep track of the uninterned
> symbols across many different procedure calls, which it would need to do
> for it to know what is being shared.

It looks like `print-continuous-numbering' and `print-number-table'
might be relevant.

bug#27177: 26.0.50: Macroexpanding cl-loop and friends (make-symbol usage)

[Alex]

npostavs@users.sourceforge.net writes:

> Alex <agrambot@gmail.com> writes:
>
>>> AFAICT `print-circle' and `print-gensym' also control how `prin1'
>>> prints.
>>
>> Does print-circle? Consider:
>>
>> (prin1 `(cons ,(make-symbol "hello")
>> 	      ,(make-symbol "hello")))
>>
>> print-gensym certainly makes a difference in the output, but
>> print-circle doesn't seem to.
>
> You're producing 2 different symbols, try
>
>     (let ((sym (make-symbol "hello")))
>       (prin1 `(cons ,sym ,sym)))

Right, sorry for the bad example. I don't know what I was thinking.

>> However, I don't know how prin1 would keep track of the uninterned
>> symbols across many different procedure calls, which it would need to do
>> for it to know what is being shared.
>
> It looks like `print-continuous-numbering' and `print-number-table'
> might be relevant.

That does look like an option, thanks. I'd still like for cl-macs.el to
produce readable output without all of these extra print-* variables
set, but if it's infeasible to do that, then I'll look into using those.

bug#27177: 26.0.50: Macroexpanding cl-loop and friends (make-symbol usage)

[Stefan Monnier]

> Stefan, why did you make the switch? Using cl-gensym would help a ton
> with readability of cl-loop's macroexpansion.

I don't consider human-readability of the result to be something that
a macro should have to pay attention to.


        Stefan

bug#27177: 26.0.50: Macroexpanding cl-loop and friends (make-symbol usage)

[Alex]

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

>> Stefan, why did you make the switch? Using cl-gensym would help a ton
>> with readability of cl-loop's macroexpansion.
>
> I don't consider human-readability of the result to be something that
> a macro should have to pay attention to.

I can understand that when talking about smaller macros, but loop is a
fairly complex macro, so I believe human-readability is a desirable
trait to have if feasible. It would help with debugging and overall
understanding of cl-loop.

I've browsed around for a few common loop implementations and they all
use gensym (CCL uses gentemp) and descriptive naming:

SBCL: https://github.com/sbcl/sbcl/blob/master/src/code/loop.lisp

CLISP: https://sourceforge.net/p/clisp/clisp/ci/default/tree/src/loop.lisp

CCL: https://github.com/Clozure/ccl/blob/master/library/loop.lisp

ABCL (admittedly they adapted it from SBCL):
http://abcl.org/trac/browser/trunk/abcl/src/org/armedbear/lisp/loop.lisp

I also found a CHICKEN Scheme egg for CL's loop, and it uses gensym (but
generic names, unfortunately).

If there's a good reason to not use gensym, then that's fine, but if the
problem is easy enough to work around (perhaps per-expansion counter so
that it will never realistically hit most-positive-fixnum), then I think
cl-loop should use it.

bug#27177: 26.0.50: Macroexpanding cl-loop and friends (make-symbol usage)

[Stefan Monnier]

> I've browsed around for a few common loop implementations and they all
> use gensym (CCL uses gentemp) and descriptive naming:

gensym is the indeed what is commonly used in Common-Lisp, whereas
make-symbol is what is commonly used in ELisp.

> I also found a CHICKEN Scheme egg for CL's loop, and it uses gensym (but
> generic names, unfortunately).

Does Scheme have make-symbol or something equivalent?

> If there's a good reason to not use gensym, then that's fine, but if the
> problem is easy enough to work around (perhaps per-expansion counter so
> that it will never realistically hit most-positive-fixnum), then I think
> cl-loop should use it.

I'd prefer to solve it in the printer, but that's just my opinion.
FWIW, I've found print-gensym to be sufficient.


        Stefan

bug#27177: 26.0.50: Macroexpanding cl-loop and friends (make-symbol usage)

[Alex]

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

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

>> I've browsed around for a few common loop implementations and they all
>> use gensym (CCL uses gentemp) and descriptive naming:
>
> gensym is the indeed what is commonly used in Common-Lisp, whereas
> make-symbol is what is commonly used in ELisp.

Right, but it seems natural that the CL compatibility macros would use
gensym since CL does it that way.

>> I also found a CHICKEN Scheme egg for CL's loop, and it uses gensym (but
>> generic names, unfortunately).
>
> Does Scheme have make-symbol or something equivalent?

The RnRS standards don't specify uninterned symbols, but they mention
that some implementations have them. A quick check shows that Guile has
make-symbol, and CHICKEN and Racket have equivalents to make-symbol
(string->uninterned-symbol). All 3 have gensym.

>> If there's a good reason to not use gensym, then that's fine, but if the
>> problem is easy enough to work around (perhaps per-expansion counter so
>> that it will never realistically hit most-positive-fixnum), then I think
>> cl-loop should use it.
>
> I'd prefer to solve it in the printer, but that's just my opinion.
> FWIW, I've found print-gensym to be sufficient.

How do you want to solve it in the printer? One way I thought of was to
keep a counter similar to cl--gensym-counter, and make a hash table of
uninterned symbols with values being the prefix to concat to the end of
the print name of the symbol. At this point, though, why not port gensym
to ELisp proper and encourage use of gensym? I don't see the use for
extra complexity if it's not needed.

A seemingly simple option, as mentioned before, is to bind
cl--gensym-counter to 0 at the start of cl-loop. That means cl-loop
won't increase the global counter, and it fixes the readability of the
macro. I attached a sample diff below, which seems to work fine.


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

diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el
index db1518ce61..5b711a2a79 100644
--- a/lisp/emacs-lisp/cl-macs.el
+++ b/lisp/emacs-lisp/cl-macs.el
@@ -544,7 +544,7 @@ cl--do-arglist
 	  (laterarg nil) (exactarg nil) minarg)
       (or num (setq num 0))
       (setq restarg (if (listp (cadr restarg))
-                        (make-symbol "--cl-rest--")
+                        (cl-gensym "--cl-rest--")
                       (cadr restarg)))
       (push (list restarg expr) cl--bind-lets)
       (if (eq (car args) '&whole)
@@ -617,7 +617,7 @@ cl--do-arglist
                    (look `(plist-member ,restarg ',karg)))
 	      (and def cl--bind-enquote (setq def `',def))
 	      (if (cddr arg)
-		  (let* ((temp (or (nth 2 arg) (make-symbol "--cl-var--")))
+		  (let* ((temp (or (nth 2 arg) (cl-gensym "--cl-var--")))
 			 (val `(car (cdr ,temp))))
 		    (cl--do-arglist temp look)
 		    (cl--do-arglist varg
@@ -636,7 +636,7 @@ cl--do-arglist
       (setq keys (nreverse keys))
       (or (and (eq (car args) '&allow-other-keys) (pop args))
 	  (null keys) (= safety 0)
-	  (let* ((var (make-symbol "--cl-keys--"))
+	  (let* ((var (cl-gensym "--cl-keys--"))
 		 (allow '(:allow-other-keys))
 		 (check `(while ,var
                            (cond
@@ -936,7 +936,8 @@ cl-loop
 	  (cl--loop-accum-var nil)	(cl--loop-accum-vars nil)
 	  (cl--loop-initially nil)	(cl--loop-finally nil)
 	  (cl--loop-iterator-function nil) (cl--loop-first-flag nil)
-          (cl--loop-symbol-macs nil))
+          (cl--loop-symbol-macs nil)
+          (cl--gensym-counter 0))
       ;; Here is more or less how those dynbind vars are used after looping
       ;; over cl--parse-loop-clause:
       ;;
@@ -1225,9 +1226,9 @@ cl--parse-loop-clause
 		       (step (and (eq (car cl--loop-args) 'by)
                                   (cl--pop2 cl--loop-args)))
 		       (end-var (and (not (macroexp-const-p end))
-				     (make-symbol "--cl-var--")))
+				     (cl-gensym "--cl-var--")))
 		       (step-var (and (not (macroexp-const-p step))
-				      (make-symbol "--cl-var--"))))
+				      (cl-gensym "--cl-var--"))))
 		  (and step (numberp step) (<= step 0)
 		       (error "Loop `by' value is not positive: %s" step))
 		  (push (list var (or start 0)) loop-for-bindings)
@@ -1246,7 +1247,7 @@ cl--parse-loop-clause
 	       ((memq word '(in in-ref on))
 		(let* ((on (eq word 'on))
 		       (temp (if (and on (symbolp var))
-				 var (make-symbol "--cl-var--"))))
+				 var (cl-gensym "--cl-var--"))))
 		  (push (list temp (pop cl--loop-args)) loop-for-bindings)
 		  (push `(consp ,temp) cl--loop-body)
 		  (if (eq word 'in-ref)
@@ -1278,7 +1279,7 @@ cl--parse-loop-clause
 			(push `(,var
 				(if ,(or cl--loop-first-flag
 					 (setq cl--loop-first-flag
-					       (make-symbol "--cl-var--")))
+					       (cl-gensym "--cl-var--")))
 				    ,start ,var))
 			      loop-for-sets)
 			(push (list var then) loop-for-steps))
@@ -1286,13 +1287,13 @@ cl--parse-loop-clause
 				(if (eq start then) start
 				  `(if ,(or cl--loop-first-flag
 					    (setq cl--loop-first-flag
-						  (make-symbol "--cl-var--")))
+						  (cl-gensym "--cl-var--")))
 				       ,start ,then)))
 			  loop-for-sets))))
 
 	       ((memq word '(across across-ref))
-		(let ((temp-vec (make-symbol "--cl-vec--"))
-		      (temp-idx (make-symbol "--cl-idx--")))
+		(let ((temp-vec (cl-gensym "--cl-vec--"))
+		      (temp-idx (cl-gensym "--cl-idx--")))
 		  (push (list temp-vec (pop cl--loop-args)) loop-for-bindings)
 		  (push (list temp-idx -1) loop-for-bindings)
 		  (push `(< (setq ,temp-idx (1+ ,temp-idx))
@@ -1310,18 +1311,18 @@ cl--parse-loop-clause
 			       (and (not (memq (car cl--loop-args) '(in of)))
 				    (error "Expected `of'"))))
 		      (seq (cl--pop2 cl--loop-args))
-		      (temp-seq (make-symbol "--cl-seq--"))
+		      (temp-seq (cl-gensym "--cl-seq--"))
 		      (temp-idx
                        (if (eq (car cl--loop-args) 'using)
                            (if (and (= (length (cadr cl--loop-args)) 2)
                                     (eq (cl-caadr cl--loop-args) 'index))
                                (cadr (cl--pop2 cl--loop-args))
                              (error "Bad `using' clause"))
-                         (make-symbol "--cl-idx--"))))
+                         (cl-gensym "--cl-idx--"))))
 		  (push (list temp-seq seq) loop-for-bindings)
 		  (push (list temp-idx 0) loop-for-bindings)
 		  (if ref
-		      (let ((temp-len (make-symbol "--cl-len--")))
+		      (let ((temp-len (cl-gensym "--cl-len--")))
 			(push (list temp-len `(length ,temp-seq))
 			      loop-for-bindings)
 			(push (list var `(elt ,temp-seq ,temp-idx))
@@ -1350,7 +1351,7 @@ cl--parse-loop-clause
                                      (not (eq (cl-caadr cl--loop-args) word)))
                                 (cadr (cl--pop2 cl--loop-args))
                               (error "Bad `using' clause"))
-                          (make-symbol "--cl-var--"))))
+                          (cl-gensym "--cl-var--"))))
 		  (if (memq word '(hash-value hash-values))
 		      (setq var (prog1 other (setq other var))))
 		  (cl--loop-set-iterator-function
@@ -1377,14 +1378,14 @@ cl--parse-loop-clause
 		  (cl--loop-set-iterator-function
                    'overlays (lambda (body)
                                `(cl--map-overlays
-                                 (lambda (,var ,(make-symbol "--cl-var--"))
+                                 (lambda (,var ,(cl-gensym "--cl-var--"))
                                    (progn . ,body) nil)
                                  ,buf ,from ,to)))))
 
 	       ((memq word '(interval intervals))
 		(let ((buf nil) (prop nil) (from nil) (to nil)
-		      (var1 (make-symbol "--cl-var1--"))
-		      (var2 (make-symbol "--cl-var2--")))
+		      (var1 (cl-gensym "--cl-var1--"))
+		      (var2 (cl-gensym "--cl-var2--")))
 		  (while (memq (car cl--loop-args) '(in of property from to))
 		    (cond ((eq (car cl--loop-args) 'from)
                            (setq from (cl--pop2 cl--loop-args)))
@@ -1413,7 +1414,7 @@ cl--parse-loop-clause
                                     (not (eq (cl-caadr cl--loop-args) word)))
                                (cadr (cl--pop2 cl--loop-args))
                              (error "Bad `using' clause"))
-                         (make-symbol "--cl-var--"))))
+                         (cl-gensym "--cl-var--"))))
 		  (if (memq word '(key-binding key-bindings))
 		      (setq var (prog1 other (setq other var))))
 		  (cl--loop-set-iterator-function
@@ -1423,7 +1424,7 @@ cl--parse-loop-clause
                              (lambda (,var ,other) . ,body) ,cl-map)))))
 
 	       ((memq word '(frame frames screen screens))
-		(let ((temp (make-symbol "--cl-var--")))
+		(let ((temp (cl-gensym "--cl-var--")))
 		  (push (list var  '(selected-frame))
 			loop-for-bindings)
 		  (push (list temp nil) loop-for-bindings)
@@ -1436,8 +1437,8 @@ cl--parse-loop-clause
 	       ((memq word '(window windows))
 		(let ((scr (and (memq (car cl--loop-args) '(in of))
                                 (cl--pop2 cl--loop-args)))
-		      (temp (make-symbol "--cl-var--"))
-		      (minip (make-symbol "--cl-minip--")))
+		      (temp (cl-gensym "--cl-var--"))
+		      (minip (cl-gensym "--cl-minip--")))
 		  (push (list var (if scr
 				      `(frame-selected-window ,scr)
 				    '(selected-window)))
@@ -1481,7 +1482,7 @@ cl--parse-loop-clause
 		  cl--loop-steps))))
 
      ((eq word 'repeat)
-      (let ((temp (make-symbol "--cl-var--")))
+      (let ((temp (cl-gensym "--cl-var--")))
 	(push (list (list temp (pop cl--loop-args))) cl--loop-bindings)
 	(push `(>= (setq ,temp (1- ,temp)) 0) cl--loop-body)))
 
@@ -1560,22 +1561,22 @@ cl--parse-loop-clause
 
      ((eq word 'always)
       (or cl--loop-finish-flag
-          (setq cl--loop-finish-flag (make-symbol "--cl-flag--")))
+          (setq cl--loop-finish-flag (cl-gensym "--cl-flag--")))
       (push `(setq ,cl--loop-finish-flag ,(pop cl--loop-args)) cl--loop-body)
       (setq cl--loop-result t))
 
      ((eq word 'never)
       (or cl--loop-finish-flag
-          (setq cl--loop-finish-flag (make-symbol "--cl-flag--")))
+          (setq cl--loop-finish-flag (cl-gensym "--cl-flag--")))
       (push `(setq ,cl--loop-finish-flag (not ,(pop cl--loop-args)))
 	    cl--loop-body)
       (setq cl--loop-result t))
 
      ((eq word 'thereis)
       (or cl--loop-finish-flag
-          (setq cl--loop-finish-flag (make-symbol "--cl-flag--")))
+          (setq cl--loop-finish-flag (cl-gensym "--cl-flag--")))
       (or cl--loop-result-var
-          (setq cl--loop-result-var (make-symbol "--cl-var--")))
+          (setq cl--loop-result-var (cl-gensym "--cl-var--")))
       (push `(setq ,cl--loop-finish-flag
                    (not (setq ,cl--loop-result-var ,(pop cl--loop-args))))
 	    cl--loop-body))
@@ -1607,9 +1608,9 @@ cl--parse-loop-clause
 
      ((eq word 'return)
       (or cl--loop-finish-flag
-          (setq cl--loop-finish-flag (make-symbol "--cl-var--")))
+          (setq cl--loop-finish-flag (cl-gensym "--cl-var--")))
       (or cl--loop-result-var
-          (setq cl--loop-result-var (make-symbol "--cl-var--")))
+          (setq cl--loop-result-var (cl-gensym "--cl-var--")))
       (push `(setq ,cl--loop-result-var ,(pop cl--loop-args)
                    ,cl--loop-finish-flag nil)
             cl--loop-body))
@@ -1640,7 +1641,7 @@ cl--loop-let
           (setq par nil)
           (dolist (spec specs)
             (or (macroexp-const-p (cadr spec))
-                (let ((temp (make-symbol "--cl-var--")))
+                (let ((temp (cl-gensym "--cl-var--")))
                   (push (list temp (cadr spec)) temps)
                   (setcar (cdr spec) temp)))))))
     (while specs
@@ -1657,7 +1658,7 @@ cl--loop-let
                           (and (eq body 'setq) (cl--unused-var-p temp)))
                   ;; Prefer a fresh uninterned symbol over "_to", to avoid
                   ;; warnings that we set an unused variable.
-                  (setq temp (make-symbol "--cl-var--"))
+                  (setq temp (cl-gensym "--cl-var--"))
                   ;; Make sure this temp variable is locally declared.
                   (when (eq body 'setq)
                     (push (list (list temp)) cl--loop-bindings)))
@@ -1685,7 +1686,7 @@ cl--loop-handle-accum
     (or cl--loop-accum-var
 	(progn
 	  (push (list (list
-                       (setq cl--loop-accum-var (make-symbol "--cl-var--"))
+                       (setq cl--loop-accum-var (cl-gensym "--cl-var--"))
                        def))
                 cl--loop-bindings)
 	  (setq cl--loop-result (if func (list func cl--loop-accum-var)
diff --git a/lisp/ffap.el b/lisp/ffap.el
index 87531110b8..3eee3f3878 100644
--- a/lisp/ffap.el
+++ b/lisp/ffap.el
@@ -1326,7 +1326,8 @@ ffap-file-at-point
 	 ;; If it contains a colon, get rid of it (and return if exists)
 	 ((and (string-match path-separator name)
 	       (setq name (ffap-string-at-point 'nocolon))
-	       (ffap-file-exists-string name)))
+               (not (string-empty-p name))
+               (ffap-file-exists-string name)))
 	 ;; File does not exist, try the alist:
 	 ((let ((alist ffap-alist) tem try case-fold-search)
 	    (while (and alist (not try))

bug#27177: 26.0.50: Macroexpanding cl-loop and friends (make-symbol usage)

[Lars Ingebrigtsen]

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

>> Stefan, why did you make the switch? Using cl-gensym would help a ton
>> with readability of cl-loop's macroexpansion.
>
> I don't consider human-readability of the result to be something that
> a macro should have to pay attention to.

There didn't seem to be much enthusiasm for changing the code generation
here, so I'm closing this bug report.  If there is something more to be
done here, please respond to the debbugs address and we'll reopen.

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