From 595df5e1c3100985d6e3bd87ad27b5b3d9a667bd Mon Sep 17 00:00:00 2001 From: fogus Date: Mon, 12 Sep 2011 13:36:16 -0400 Subject: [PATCH 1/2] Fixes CLJ-837 by allowing any field prefixed with double-underscores except __meta and __extmap. Attempting to use these names will throw and assertion error at compile time. Added the restriction on these names to the relevant docstrings and added regression tests. Signed-off-by: Stuart Halloway --- src/clj/clojure/core_deftype.clj | 17 +++++++++++++++-- src/jvm/clojure/lang/Compiler.java | 2 +- test/clojure/test_clojure/protocols.clj | 14 ++++++++++++++ 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/src/clj/clojure/core_deftype.clj b/src/clj/clojure/core_deftype.clj index f331b5c..9199b59 100644 --- a/src/clj/clojure/core_deftype.clj +++ b/src/clj/clojure/core_deftype.clj @@ -254,6 +254,13 @@ (throw (clojure.lang.ArityException. (+ ~arg-count (count ~'overage)) (name '~fn-name)))) `(new ~classname ~@field-args))))) +(defn- validate-fields + "" + [fields] + (let [specials #{'__meta '__extmap}] + (when (some specials fields) + (throw (AssertionError. (str "The names in " specials " cannot be used as field names for types or records.")))))) + (defmacro defrecord "Alpha - subject to change @@ -314,10 +321,13 @@ Two constructors will be defined, one taking the designated fields followed by a metadata map (nil for none) and an extension field map (nil for none), and one taking only the fields (using nil for - meta and extension fields)." + meta and extension fields). Note that the field names __meta + and __extmap are currently reserved and should not be used when + defining your own records." {:added "1.2"} [name [& fields] & opts+specs] + (validate-fields fields) (let [gname name [interfaces methods opts] (parse-opts+specs opts+specs) ns-part (namespace-munge *ns*) @@ -400,10 +410,13 @@ given name (a symbol), prepends the current ns as the package, and writes the .class file to the *compile-path* directory. - One constructors will be defined, taking the designated fields." + One constructor will be defined, taking the designated fields. Note + that the field names __meta and __extmap are currently reserved and + should not be used when defining your own types." {:added "1.2"} [name [& fields] & opts+specs] + (validate-fields fields) (let [gname name [interfaces methods opts] (parse-opts+specs opts+specs) ns-part (namespace-munge *ns*) diff --git a/src/jvm/clojure/lang/Compiler.java b/src/jvm/clojure/lang/Compiler.java index cbcf58d..fb2e491 100644 --- a/src/jvm/clojure/lang/Compiler.java +++ b/src/jvm/clojure/lang/Compiler.java @@ -7244,7 +7244,7 @@ static public class NewInstanceExpr extends ObjExpr{ //use array map to preserve ctor order ret.closes = new PersistentArrayMap(closesvec); ret.fields = fmap; - for(int i=fieldSyms.count()-1;i >= 0 && ((Symbol)fieldSyms.nth(i)).name.startsWith("__");--i) + for(int i=fieldSyms.count()-1;i >= 0 && (((Symbol)fieldSyms.nth(i)).name.equals("__meta") || ((Symbol)fieldSyms.nth(i)).name.equals("__extmap"));--i) ret.altCtorDrops++; } //todo - set up volatiles diff --git a/test/clojure/test_clojure/protocols.clj b/test/clojure/test_clojure/protocols.clj index 1613291..593126f 100644 --- a/test/clojure/test_clojure/protocols.clj +++ b/test/clojure/test_clojure/protocols.clj @@ -432,6 +432,20 @@ (is (= "#clojure.test_clojure.protocols.RecordToTestPrinting{:a 1, :b 2}" (binding [*print-dup* true *verbose-defrecords* true] (pr-str r))))))) +(defrecord RecordToTest__ [__a ___b]) +(defrecord TypeToTest__ [__a ___b]) + +(deftest test-record-and-type-field-names + (testing "that types and records allow names starting with double-underscore. + This is a regression test for CLJ-837." + (let [r (RecordToTest__. 1 2) + t (TypeToTest__. 3 4)] + (are [x y] = + 1 (:__a r) + 2 (:___ b r) + 3 (.__a t) + 4 (.___b t))))) + (defrecord RecordToTestLongHint [^long a]) (defrecord RecordToTestByteHint [^byte a]) (defrecord RecordToTestBoolHint [^boolean a]) -- 1.7.4.1 From b0abd11d5e60be1f0a57e697c4756eb8ecfa40bd Mon Sep 17 00:00:00 2001 From: Stuart Halloway Date: Fri, 2 Dec 2011 14:50:25 -0500 Subject: [PATCH 2/2] make test actually test, then fix it --- test/clojure/test_clojure/protocols.clj | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/clojure/test_clojure/protocols.clj b/test/clojure/test_clojure/protocols.clj index 593126f..3e2d280 100644 --- a/test/clojure/test_clojure/protocols.clj +++ b/test/clojure/test_clojure/protocols.clj @@ -440,9 +440,9 @@ This is a regression test for CLJ-837." (let [r (RecordToTest__. 1 2) t (TypeToTest__. 3 4)] - (are [x y] = + (are [x y] (= x y) 1 (:__a r) - 2 (:___ b r) + 2 (:___b r) 3 (.__a t) 4 (.___b t))))) -- 1.7.4.1