From 6e1bd67a9e932050492df945333bea54dede7253 Mon Sep 17 00:00:00 2001 From: Ben Smith-Mannschott Date: Sat, 15 Oct 2011 21:55:52 +0200 Subject: [PATCH] =?UTF-8?q?[RFC]=20CLJ-852:=20add=20special=20cases=20for=20?= =?UTF-8?q?"int"=E2=80=A6"boolean"=20to=20tagToClass()?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This causes the test to pass, however, there are still some open questions: Why does tagToClass first call maybeClass and then possibly overwrite the result it gets back without even looking at it? This strikes me as surprising. I would have expected a check for null, and then only running the if/else chain of special cases if maybeClass failed to return a non-null result. The suppressed exception in maybeClass commented with "//aargh" makes me wonder if maybeClass and tagToClass are factored correctly. --- src/jvm/clojure/lang/Compiler.java | 44 +++++++++++++++++++++++++++--------- 1 files changed, 33 insertions(+), 11 deletions(-) diff --git a/src/jvm/clojure/lang/Compiler.java b/src/jvm/clojure/lang/Compiler.java index 55e1268..494f561 100644 --- a/src/jvm/clojure/lang/Compiler.java +++ b/src/jvm/clojure/lang/Compiler.java @@ -939,6 +939,8 @@ static public abstract class HostExpr implements Expr, MaybePrimitiveExpr{ } catch(Exception e){ //aargh +// CLJ-852: Aargh? Is this the point where the if/else chain in +// tagToClass comes into play? } } } @@ -973,11 +975,15 @@ static public abstract class HostExpr implements Expr, MaybePrimitiveExpr{ */ static Class tagToClass(Object tag) { Class c = maybeClass(tag, true); +// CLJ-852: Why do we first ask for maybeClass and then ... if(tag instanceof Symbol) { Symbol sym = (Symbol) tag; if(sym.ns == null) //if ns-qualified can't be classname { +// CLJ-852: ... overwrite what maybeClass() returned without even +// consdiering it. (Isn't this whole if/else cascade only +// necessary if c comes back null? if(sym.name.equals("objects")) c = Object[].class; else if(sym.name.equals("ints")) @@ -985,17 +991,33 @@ static public abstract class HostExpr implements Expr, MaybePrimitiveExpr{ else if(sym.name.equals("longs")) c = long[].class; else if(sym.name.equals("floats")) - c = float[].class; - else if(sym.name.equals("doubles")) - c = double[].class; - else if(sym.name.equals("chars")) - c = char[].class; - else if(sym.name.equals("shorts")) - c = short[].class; - else if(sym.name.equals("bytes")) - c = byte[].class; - else if(sym.name.equals("booleans")) - c = boolean[].class; + c = float[].class; + else if(sym.name.equals("doubles")) + c = double[].class; + else if(sym.name.equals("chars")) + c = char[].class; + else if(sym.name.equals("shorts")) + c = short[].class; + else if(sym.name.equals("bytes")) + c = byte[].class; + else if(sym.name.equals("booleans")) + c = boolean[].class; + else if(sym.name.equals("int")) + c = Integer.TYPE; + else if(sym.name.equals("long")) + c = Long.TYPE; + else if(sym.name.equals("float")) + c = Float.TYPE; + else if(sym.name.equals("double")) + c = Double.TYPE; + else if(sym.name.equals("char")) + c = Character.TYPE; + else if(sym.name.equals("short")) + c = Short.TYPE; + else if(sym.name.equals("byte")) + c = Byte.TYPE; + else if(sym.name.equals("boolean")) + c = Boolean.TYPE; } } if(c != null) -- 1.7.6.1