Better handle coord meta elision for (), '() and 'foo

flow-storm · May 14, 2024 · f0e35e8 · f0e35e8
1 parent 5b68a24
commit f0e35e8
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 32 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,13 +1,17 @@
 # Changelog
 
 ## master (unreleased)
-	
+
 ### New Features
 
 ### Changes 
 
 ### Bugs fixed
 
+    - Don't tag ()
+    - After eliding meta (reader and coord) leave nil instead of {} when that was the only meta
+    - Refactor Utils.mergeMeta
+
 ## 1.12.0-alpha11 (unreleased)
 
 ### New Features

diff --git a/src/jvm/clojure/lang/Compiler.java b/src/jvm/clojure/lang/Compiler.java
@@ -301,8 +301,11 @@ static public Object getCompilerOption(Keyword k){
     static Object elideMeta(Object m){
         Collection<Object> elides = (Collection<Object>) getCompilerOption(elideMetaKey);
 
-		// Always elide Storm coordinates meta
+		// Always elide Storm coordinates meta, and if storm coordinates where the only thing
+        // added leave meta as null. If not we are going to endup with an empty map as meta
+        // that wasn't there in the first place
 		m = RT.dissoc(m, LispReader.COORD_KEY);
+        if (RT.count(m) == 0) m = null;
 
         if(elides != null)
             {

diff --git a/src/jvm/clojure/storm/Utils.java b/src/jvm/clojure/storm/Utils.java
@@ -34,37 +34,31 @@
 public class Utils {
 
 	public static Object mergeMeta(Object x, IPersistentMap m) {
-		if (x instanceof clojure.lang.IObj) {
+		if (x instanceof clojure.lang.IObj && RT.count(m) > 0) {
+            // if x supports meta and there is meta to merge
 			IObj o = (IObj) x;
-			if (m != null || RT.meta(o) != null)
-				{ // if there is meta in any of the args
-				IPersistentMap retMeta = PersistentHashMap.EMPTY;
-				IPersistentMap oMeta = RT.meta(o);
 
-				if (oMeta != null)
-					{
-					for (Object meo : oMeta)
-					{
-						MapEntry me = (MapEntry) meo;
-						retMeta = retMeta.assoc(me.getKey(), me.getValue());
-						}
-					}
-					// m meta overrides the input Object meta when both have
-					if (m != null)
-					{
-						for (Object meo : m)
-						{
-							MapEntry me = (MapEntry) meo;
-							retMeta = retMeta.assoc(me.getKey(), me.getValue());
-						}
-					}
-				return o.withMeta(retMeta);
-				} else {
-				return x;
-				}
-			}
-		return x;
-		}
+            IPersistentMap retMeta = PersistentHashMap.EMPTY;
+            IPersistentMap oMeta = RT.meta(o);
+
+            if (oMeta != null) {
+                for (Object meo : oMeta) {
+                    MapEntry me = (MapEntry) meo;
+                    retMeta = retMeta.assoc(me.getKey(), me.getValue());
+                }
+            }
+
+            // m meta overrides the input Object meta when both have
+            for (Object meo : m) {
+                MapEntry me = (MapEntry) meo;
+                retMeta = retMeta.assoc(me.getKey(), me.getValue());
+            }
+
+            return o.withMeta(retMeta);
+        } else {
+            return x;
+        }
+    }
 
 	public static Symbol maybeGetTraceSymbol(Symbol sym, ISeq form){
         // If the form is the expansion of a defmethod we use the symbol of
@@ -219,7 +213,9 @@ public static Object tagFormRecursively(Object form) {
             PersistentVector.EMPTY,
             new AFn() {            
             public Object invoke(Object coord, Object frm) {
-                if ((frm instanceof clojure.lang.ISeq) || (frm instanceof clojure.lang.Symbol))
+                // Tag seqs and symbols but don't tag empty lists 
+                if (((frm instanceof clojure.lang.ISeq) && RT.count(frm) > 0) ||
+                    (frm instanceof clojure.lang.Symbol))
                     return addCoordMeta(frm, (IPersistentVector)coord);
                 else
                     return frm;