Patch and audit: Use vectors for bindings in doseq, et al.
Chouser
in their syntax:
(dotimes i (range 10)
(prn i))
The attached patch modifies the usage of these macros to be more
like let:
(dotimes [i (range 10)]
(prn i))
I walked through all the macros in boot.clj and identified and
updated these:
doseq
dotimes
with-open
when-first
if-let
when-let
amap and areduce take names to be bound, but don't take specific
values for those names, so I've left them alone.
All except doseq should produce identical code as before. doseq
is now based on for, but since it's used in boot.clj before
destructuring is available (as for requires), there's a simpler
version of doseq in play for part of boot.clj.
Under most circumstances if you try to use the old syntax after
this patch, you'll get an error like:
java.lang.IllegalArgumentException: if-let now requires a vector
for its binding
However, some uses are harder to detect, like this old-style use
of destructuring:
(when-let [a b] my-list (dofoo a b))
If you have code like that, you might see a more cryptic error,
such as "Unable to resolve symbol" or perhaps "Unsupported
binding form".
This patch also includes fixes to all the places in boot, proxy,
gen-class, and xml (that I found) that use these macros. I
assume every lib in clojure-contrib and in fact nearly every
piece of clojure code more that 10 lines long will break.
Have fun! :-)
--Chouser
James Reeves
> The attached patch modifies the usage of these macros to be more
> like let:
>
> (dotimes [i (range 10)]
> (prn i))
(dotimes i 10
(prn i))
And in which case, your vector syntax could be misleading, because it
seems to imply you're assigning i to 10:
(dotimes [i 10]
(prn i))
Second, with your patch, is the following valid:
(doseq [i (range 10)
j (range 10)]
(prn i)
(prn j))
And if it is, what does it do? Does it iterate through all
permutations of i and j, or does it zip i and j together?
If the above isn't valid, your syntax again seems a little misleading,
since with the let special form, you can assign as many vars as you
want.
Third, even if doseq allows for multiple local vars, you'd still have
problems with when-let and if-let. For instance:
(when-let [x (foo)
y (bar)]
(prn x)
(prn y))
Under what conditions is the body evaluated? If x OR y is true, or if
x AND y is true, or if the last value of the let clause, y, is true?
It's my opinion that the let syntax only makes sense when you have
multiple vars to bind. Otherwise the same syntax has different
meanings depending on the macro; sometimes you can only bind one,
sometimes you can bind many. Currently in Clojure this syntax only has
one meaning, and I think that's more consistent.
- James
Chouser
>
> A few questions. First, isn't the syntax for dotimes:
>
> (dotimes i 10
> (prn i))
You're absolutely right. Sorry about that.
> And in which case, your vector syntax could be misleading, because it
> seems to imply you're assigning i to 10:
>
> (dotimes [i 10]
> (prn i))
Vectors are also used for "for":
(for [i (range 10)]
(* 2 i))
Here i is not bound to the seq (range 10) but to each of the numbers
in turn. While on the other hand, "let" and "binding" indeed only
bind to the specified value. So the vector syntax itself doesn't give
you enough information, you need to look at the macro name too. If the
macro is named "dotimes" don't expect it to bind to just the final
value.
> Second, with your patch, is the following valid:
>
> (doseq [i (range 10)
> j (range 10)]
> (prn i)
> (prn j))
>
> And if it is, what does it do? Does it iterate through all
> permutations of i and j, or does it zip i and j together?
It behaves the same as "for" does, that is with j in an inner loop.
It also supports :while and :when as "for" does.
> Third, even if doseq allows for multiple local vars, you'd still have
> problems with when-let and if-let. For instance:
>
> (when-let [x (foo)
> y (bar)]
> (prn x)
> (prn y))
>
> Under what conditions is the body evaluated? If x OR y is true, or if
> x AND y is true, or if the last value of the let clause, y, is true?
Currently everything after (foo) in your example is ignored, but that
doesn't seem very good.
My first inclination is to disallow it -- add a check to make sure
only one binding pair is given. Alternatively it could act as if it
were nested, as "for" (and now "doseq") do, in which case it would act
like an "and", and both x and y would be bound to non-false values.
--Chouser
James Reeves
On Nov 7, 9:32 pm, Chouser <chou...@gmail.com> wrote:
> > And in which case, your vector syntax could be misleading, because it
> > seems to imply you're assigning i to 10:
>
> > (dotimes [i 10]
> > (prn i))
>
> Vectors are also used for "for":
>
> (for [i (range 10)]
> (* 2 i))
>
> Here i is not bound to the seq (range 10) but to each of the numbers
> in turn.
assignments, where a value is assigned to a symbol as in let and
binding, and sequence assignments, where each item in the sequence is
assigned to a symbol. Adding a vector to dotimes would add a third
type, and I don't think it's obvious what the [i 10] does. I mean, you
originally put it down as [i (range 10)], so you were thinking in
terms of [symbol sequence] too.
> > Second, with your patch, is the following valid:
>
> > (doseq [i (range 10)
> > j (range 10)]
> > (prn i)
> > (prn j))
>
> It also supports :while and :when as "for" does.
Consistency with the for macro seems reasonable.
> My first inclination is to disallow it -- add a check to make sure
> only one binding pair is given. Alternatively it could act as if it
> were nested, as "for" (and now "doseq") do, in which case it would act
> like an "and", and both x and y would be bound to non-false values.
macros. If this is implemented, nesting ifs would be my preference for
this.
- James
Rich Hickey
On Nov 7, 5:09 pm, James Reeves <weavejes...@googlemail.com> wrote:
> On Nov 7, 9:32 pm, Chouser <chou...@gmail.com> wrote:
>
> > > And in which case, your vector syntax could be misleading, because it
> > > seems to imply you're assigning i to 10:
>
> > > (dotimes [i 10]
> > > (prn i))
>
> > Vectors are also used for "for":
>
> > (for [i (range 10)]
> > (* 2 i))
>
> > Here i is not bound to the seq (range 10) but to each of the numbers
> > in turn.
>
> I'm still not convinced on this one. Currently, you have single
> assignments, where a value is assigned to a symbol as in let and
> binding, and sequence assignments, where each item in the sequence is
> assigned to a symbol. Adding a vector to dotimes would add a third
> type, and I don't think it's obvious what the [i 10] does. I mean, you
> originally put it down as [i (range 10)], so you were thinking in
> terms of [symbol sequence] too.
>
names will do so in vector syntax. The nature of the bindings is going
to be open, as is the set of macros - doseq/dotimes/let/loop all have
different semantics. As it stands, it is confusing for people because
they don't know if they need a vector or not, for each macro.
> > > Second, with your patch, is the following valid:
>
> > > (doseq [i (range 10)
> > > j (range 10)]
> > > (prn i)
> > > (prn j))
>
> > It behaves the same as "for" does, that is with j in an inner loop.
> > It also supports :while and :when as "for" does.
>
> Well, not that my opinion matters ;) - but you've sold me on this one.
> Consistency with the for macro seems reasonable.
>
effects and forget dorun. Plus it will be faster for that use.
> > My first inclination is to disallow it -- add a check to make sure
> > only one binding pair is given. Alternatively it could act as if it
> > were nested, as "for" (and now "doseq") do, in which case it would act
> > like an "and", and both x and y would be bound to non-false values.
>
> Hm. A nested if would be consistent with the nested for and doseq
> macros. If this is implemented, nesting ifs would be my preference for
> this.
>
and otherwise just be a syntax change for all the others.
Rich
Chouser
implementation of "doseq", this has a "doseq" that uses loop/recure.
The interface is the same, but it should run faster. This also means
"doseq" is only defined once (nor redefined as in the earlier patch).
This patch is against 1089, the "Interim checkin - DO NOT USE!!" version of SVN.
--Chouser
Rich Hickey
On Nov 8, 8:52 am, Chouser <chou...@gmail.com> wrote:
> Attached is an updated patch. Instead of using "for" in the
> implementation of "doseq", this has a "doseq" that uses loop/recure.
> The interface is the same, but it should run faster. This also means
> "doseq" is only defined once (nor redefined as in the earlier patch).
>
> This patch is against 1089, the "Interim checkin - DO NOT USE!!" version of SVN.
>
Rich
> 27KViewDownload