Skip to content

Commit 21175bc

Browse files
fogusstuarthalloway
fogus
authored and
stuarthalloway
committed
Changes to shore up the shortcomings and fix bugs found in defrecord read/print form from 1.3.0-alpha7. See CLJ-800
Signed-off-by: Stuart Halloway <stu@thinkrelevance.com>
1 parent e03cff5 commit 21175bc

6 files changed

Lines changed: 199 additions & 121 deletions

File tree

‎src/clj/clojure/core_deftype.clj‎

Lines changed: 32 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,31 @@
229229
:implements ~(vec i)
230230
~@m))))))
231231

232+
(defn- build-positional-factory
233+
"Used to build a positional factory for a given type/record. Because of the
234+
limitation of 20 arguments to Clojure functions, this factory needs to be
235+
constructed to deal with more arguments. It does this by building a straight
236+
forward type/record ctor call in the <=20 case, and a call to the same
237+
ctor pulling the extra args out of the & overage parameter. Finally, the
238+
arity is constrained to the number of expected fields and an ArityException
239+
will be thrown at runtime if the actual arg count does not match."
240+
[nom classname fields]
241+
(let [fn-name (symbol (str '-> nom))
242+
[field-args over] (split-at 20 fields)
243+
field-count (count fields)
244+
arg-count (count field-args)
245+
over-count (count over)]
246+
`(defn ~fn-name
247+
[~@field-args ~@(if (seq over) '[& overage] [])]
248+
~(if (seq over)
249+
`(if (= (count ~'overage) ~over-count)
250+
(new ~classname
251+
~@field-args
252+
~@(for [i (range 0 (count over))]
253+
(list `nth 'overage i)))
254+
(throw (clojure.lang.ArityException. (+ ~arg-count (count ~'overage)) (name '~fn-name))))
255+
`(new ~classname ~@field-args)))))
256+
232257
(defmacro defrecord
233258
"Alpha - subject to change
234259
@@ -301,17 +326,16 @@
301326
`(let []
302327
~(emit-defrecord name gname (vec hinted-fields) (vec interfaces) methods)
303328
(import ~classname)
304-
(defn ~(symbol (str '-> name))
305-
([~@fields] (new ~classname ~@fields nil nil))
306-
([~@fields meta# extmap#] (new ~classname ~@fields meta# extmap#)))
307-
(defn ~(symbol (str 'map-> name))
329+
~(build-positional-factory gname classname fields)
330+
(defn ~(symbol (str 'map-> gname))
308331
([m#] (~(symbol (str classname "/create")) m#)))
309332
~classname)))
310333

311334
(defn- emit-deftype*
312335
"Do not use this directly - use deftype"
313336
[tagname name fields interfaces methods]
314-
(let [classname (with-meta (symbol (str (namespace-munge *ns*) "." name)) (meta name))]
337+
(let [classname (with-meta (symbol (str (namespace-munge *ns*) "." name)) (meta name))
338+
interfaces (conj interfaces 'clojure.lang.IType)]
315339
`(deftype* ~tagname ~classname ~fields
316340
:implements ~interfaces
317341
~@methods)))
@@ -382,32 +406,14 @@
382406
ns-part (namespace-munge *ns*)
383407
classname (symbol (str ns-part "." gname))
384408
hinted-fields fields
385-
fields (vec (map #(with-meta % nil) fields))]
409+
fields (vec (map #(with-meta % nil) fields))
410+
[field-args over] (split-at 20 fields)]
386411
`(let []
387412
~(emit-deftype* name gname (vec hinted-fields) (vec interfaces) methods)
388413
(import ~classname)
389-
(defmethod print-method ~classname [o# w#]
390-
((var print-deftype) o# w#))
391-
(defmethod print-dup ~classname [o# w#]
392-
((var printdup-deftype) o# w#))
393-
(defn ~(symbol (str '-> name))
394-
([~@fields] (new ~classname ~@fields)))
414+
~(build-positional-factory gname classname fields)
395415
~classname)))
396416

397-
(defn- print-deftype [o ^Writer w]
398-
(.write w "#")
399-
(.write w (.getName (class o)))
400-
(let [basii (for [fld (map str (clojure.lang.Reflector/invokeStaticMethod (class o) "getBasis" (to-array [])))]
401-
(clojure.lang.Reflector/getInstanceField o fld))]
402-
(print-sequential "[" pr-on ", " "]" basii w)))
403-
404-
(defn- printdup-deftype [o ^Writer w]
405-
(.write w "#")
406-
(.write w (.getName (class o)))
407-
(let [basii (for [fld (map str (clojure.lang.Reflector/invokeStaticMethod (class o) "getBasis" (to-array [])))]
408-
(clojure.lang.Reflector/getInstanceField o fld))]
409-
(print-sequential "[" pr-on ", " "]" basii w)))
410-
411417
;;;;;;;;;;;;;;;;;;;;;;; protocols ;;;;;;;;;;;;;;;;;;;;;;;;
412418

413419
(defn- expand-method-impl-cache [^clojure.lang.MethodImplCache cache c f]

‎src/clj/clojure/core_print.clj‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,8 @@
214214
(print-map m print-dup w)
215215
(.write w ")"))
216216

217+
;; Records
218+
217219
(defmethod print-method clojure.lang.IRecord [r, ^Writer w]
218220
(print-meta r w)
219221
(.write w "#")
@@ -235,6 +237,22 @@
235237
(prefer-method print-dup clojure.lang.IRecord clojure.lang.IPersistentCollection)
236238
(prefer-method print-dup clojure.lang.IRecord java.util.Map)
237239

240+
;; Types
241+
242+
(defn- print-deftype [o ^Writer w]
243+
(.write w "#")
244+
(.write w (.getName (class o)))
245+
(let [basii (for [fld (map str (clojure.lang.Reflector/invokeStaticMethod (class o) "getBasis" (to-array [])))]
246+
(clojure.lang.Reflector/getInstanceField o fld))]
247+
(print-sequential "[" pr-on ", " "]" basii w)))
248+
249+
(defmethod print-method clojure.lang.IType [o ^Writer w]
250+
(print-deftype o w))
251+
252+
(defmethod print-dup clojure.lang.IType [o ^Writer w]
253+
(print-deftype o w))
254+
255+
238256
(defmethod print-method clojure.lang.IPersistentSet [s, ^Writer w]
239257
(print-meta s w)
240258
(print-sequential "#{" pr-on " " "}" (seq s) w))

‎src/jvm/clojure/lang/Compiler.java‎

Lines changed: 40 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -4295,17 +4295,34 @@ else if(value instanceof Var)
42954295
gen.push(var.sym.toString());
42964296
gen.invokeStatic(RT_TYPE, Method.getMethod("clojure.lang.Var var(String,String)"));
42974297
}
4298-
else if(value instanceof IRecord)
4298+
else if(value instanceof IType)
42994299
{
4300-
Method createMethod = Method.getMethod(value.getClass().getName() + " create(clojure.lang.IPersistentMap)");
4301-
List entries = new ArrayList();
4302-
for(Map.Entry entry : (Set<Map.Entry>) ((Map) value).entrySet())
4300+
Method ctor = new Method("<init>", Type.getConstructorDescriptor(value.getClass().getConstructors()[0]));
4301+
gen.newInstance(Type.getType(value.getClass()));
4302+
gen.dup();
4303+
IPersistentVector fields = (IPersistentVector) Reflector.invokeStaticMethod(value.getClass(), "getBasis", new Object[]{});
4304+
for(ISeq s = RT.seq(fields); s != null; s = s.next())
43034305
{
4304-
entries.add(entry.getKey());
4305-
entries.add(entry.getValue());
4306+
Symbol field = (Symbol) s.first();
4307+
Class k = tagClass(tagOf(field));
4308+
Object val = Reflector.getInstanceField(value, field.name);
4309+
emitValue(val, gen);
4310+
4311+
if(k.isPrimitive())
4312+
{
4313+
Type b = Type.getType(boxClass(k));
4314+
String p = Type.getType(k).getDescriptor();
4315+
String n = k.getName();
4316+
4317+
gen.invokeVirtual(b, new Method(n+"Value", "()"+p));
4318+
}
43064319
}
4307-
emitListAsObjectArray(entries, gen);
4308-
gen.invokeStatic(RT_TYPE, Method.getMethod("clojure.lang.IPersistentMap map(Object[])"));
4320+
gen.invokeConstructor(Type.getType(value.getClass()), ctor);
4321+
}
4322+
else if(value instanceof IRecord)
4323+
{
4324+
Method createMethod = Method.getMethod(value.getClass().getName() + " create(clojure.lang.IPersistentMap)");
4325+
emitValue(PersistentArrayMap.create((java.util.Map) value), gen);
43094326
gen.invokeStatic(getType(value.getClass()), createMethod);
43104327
}
43114328
else if(value instanceof IPersistentMap)
@@ -6142,6 +6159,8 @@ else if(form instanceof IPersistentVector)
61426159
return VectorExpr.parse(context, (IPersistentVector) form);
61436160
else if(form instanceof IRecord)
61446161
return new ConstantExpr(form);
6162+
else if(form instanceof IType)
6163+
return new ConstantExpr(form);
61456164
else if(form instanceof IPersistentMap)
61466165
return MapExpr.parse(context, (IPersistentMap) form);
61476166
else if(form instanceof IPersistentSet)
@@ -6376,12 +6395,13 @@ public static Object eval(Object form, boolean freshLoader) {
63766395
eval(RT.first(s), false);
63776396
return eval(RT.first(s), false);
63786397
}
6379-
else if(form instanceof IPersistentCollection
6380-
&& !(RT.first(form) instanceof Symbol
6381-
&& ((Symbol) RT.first(form)).name.startsWith("def")))
6398+
else if((form instanceof IType) ||
6399+
(form instanceof IPersistentCollection
6400+
&& !(RT.first(form) instanceof Symbol
6401+
&& ((Symbol) RT.first(form)).name.startsWith("def"))))
63826402
{
63836403
ObjExpr fexpr = (ObjExpr) analyze(C.EXPRESSION, RT.list(FN, PersistentVector.EMPTY, form),
6384-
"eval" + RT.nextID());
6404+
"eval" + RT.nextID());
63856405
IFn fn = (IFn) fexpr.eval();
63866406
return fn.invoke();
63876407
}
@@ -7809,23 +7829,22 @@ static Class boxClass(Class p) {
78097829
return p;
78107830

78117831
Class c = null;
7812-
Type t = Type.getType(p);
78137832

7814-
if(t == Type.INT_TYPE)
7833+
if(p == Integer.TYPE)
78157834
c = Integer.class;
7816-
else if(t == Type.LONG_TYPE)
7835+
else if(p == Long.TYPE)
78177836
c = Long.class;
7818-
else if(t == Type.FLOAT_TYPE)
7837+
else if(p == Float.TYPE)
78197838
c = Float.class;
7820-
else if(t == Type.DOUBLE_TYPE)
7839+
else if(p == Double.TYPE)
78217840
c = Double.class;
7822-
else if(t == Type.CHAR_TYPE)
7841+
else if(p == Character.TYPE)
78237842
c = Character.class;
7824-
else if(t == Type.SHORT_TYPE)
7843+
else if(p == Short.TYPE)
78257844
c = Short.class;
7826-
else if(t == Type.BYTE_TYPE)
7845+
else if(p == Byte.TYPE)
78277846
c = Byte.class;
7828-
else if(t == Type.BOOLEAN_TYPE)
7847+
else if(p == Boolean.TYPE)
78297848
c = Boolean.class;
78307849

78317850
return c;

‎src/jvm/clojure/lang/IType.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
/**
2+
* Copyright (c) Rich Hickey. All rights reserved.
3+
* The use and distribution terms for this software are covered by the
4+
* Eclipse Public License 1.0 (http://opensource.org/licenses/eclipse-1.0.php)
5+
* which can be found in the file epl-v10.html at the root of this distribution.
6+
* By using this software in any fashion, you are agreeing to be bound by
7+
* the terms of this license.
8+
* You must not remove this notice, or any other, from this software.
9+
**/
10+
11+
package clojure.lang;
12+
13+
public interface IType {
14+
}

‎src/jvm/clojure/lang/LispReader.java‎

Lines changed: 13 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1148,9 +1148,13 @@ public Object invoke(Object reader, Object firstChar){
11481148

11491149
Object recordName = read(r, true, null, false);
11501150
Class recordClass = RT.classForName(recordName.toString());
1151-
int ch = read1(r);
11521151
char endch;
11531152
boolean shortForm = true;
1153+
int ch = read1(r);
1154+
1155+
// flush whitespace
1156+
//while(isWhitespace(ch))
1157+
// ch = read1(r);
11541158

11551159
// A defrecord ctor can take two forms. Check for map->R version first.
11561160
if(ch == '{')
@@ -1177,61 +1181,21 @@ else if (ch == '[')
11771181
if(!ctorFound)
11781182
throw Util.runtimeException("Unexpected number of constructor arguments to " + recordClass.toString() + ": got " + recordEntries.length);
11791183

1180-
ret = Reflector.invokeConstructor(recordClass, RT.seqToArray(resolveEach(recordEntries)));
1184+
ret = Reflector.invokeConstructor(recordClass, recordEntries);
11811185
}
11821186
else
11831187
{
1184-
ret = Reflector.invokeStaticMethod(recordClass, "create", new Object[]{RT.map(RT.seqToArray(resolveEach(recordEntries)))});
1185-
}
1186-
1187-
return ret;
1188-
}
11891188

1190-
static public ISeq resolveEach(Object[] a) {
1191-
ISeq ret = null;
1192-
for(int i = a.length - 1; i >= 0; --i)
1193-
ret = (ISeq) RT.cons(resolve(a[i]), ret);
1194-
return ret;
1195-
}
1196-
1197-
static private Object resolve(Object o) {
1198-
if(o instanceof Symbol)
1199-
{
1200-
try
1189+
IPersistentMap vals = RT.map(recordEntries);
1190+
for(ISeq s = RT.keys(vals); s != null; s = s.next())
12011191
{
1202-
return RT.classForName(o.toString());
1192+
if(!(s.first() instanceof Keyword))
1193+
throw Util.runtimeException("Unreadable defrecord form: key must be of type clojure.lang.Keyword, got " + s.first().toString());
12031194
}
1204-
catch(Exception cfe)
1205-
{
1206-
throw new IllegalArgumentException("Constructor literal can only contain constants or statics. "
1207-
+ o.toString()
1208-
+ " does not name a known class.");
1209-
}
1210-
}
1211-
else if(o instanceof ISeq)
1212-
{
1213-
Symbol fs = (Symbol) RT.first(o);
1214-
1215-
if(fs == null && o == PersistentList.EMPTY)
1216-
{
1217-
return o;
1218-
}
1219-
1220-
throw new IllegalArgumentException("Constructor literal can only contain constants or statics. " + o.toString());
1195+
ret = Reflector.invokeStaticMethod(recordClass, "create", new Object[]{vals});
12211196
}
1222-
else if(o instanceof IPersistentCollection && ((IPersistentCollection) o).count() == 0 ||
1223-
o instanceof IPersistentCollection ||
1224-
o instanceof Number ||
1225-
o instanceof String ||
1226-
o instanceof Keyword ||
1227-
o instanceof Symbol ||
1228-
o == Boolean.TRUE ||
1229-
o == Boolean.FALSE ||
1230-
o == null) {
1231-
return o;
1232-
}
1233-
else
1234-
throw new IllegalArgumentException("Constructor literal can only contain constants or statics. " + o.toString());
1197+
1198+
return ret;
12351199
}
12361200
}
12371201

0 commit comments

Comments
 (0)