CLJ-2621: Make InstanceMethodExpr.emit discard unused primitive returns without boxing

alexander-yakushev · puredanger · commit b32e35ae89d2 · 2022-01-13T13:53:44.000-06:00
Signed-off-by: Alex Miller &lt;alex.miller@cognitect.com&gt;
diff --git a/src/jvm/clojure/lang/Compiler.java b/src/jvm/clojure/lang/Compiler.java
@@ -1612,8 +1612,16 @@ public void emit(C context, ObjExpr objx, GeneratorAdapter gen){
 				gen.invokeInterface(type, m);
 			else
 				gen.invokeVirtual(type, m);
-			//if(context != C.STATEMENT || method.getReturnType() == Void.TYPE)
-			HostExpr.emitBoxReturn(objx, gen, method.getReturnType());
+			Class retClass = method.getReturnType();
+			if(context == C.STATEMENT)
+				{
+				if(retClass == long.class || retClass == double.class)
+					gen.pop2();
+				else if(retClass != void.class)
+					gen.pop();
+				}
+			else
+				HostExpr.emitBoxReturn(objx, gen, retClass);
 			}
 		else
 			{
@@ -1627,9 +1635,9 @@ public void emit(C context, ObjExpr objx, GeneratorAdapter gen){
 				method.emitClearLocals(gen);
 				}
 			gen.invokeStatic(REFLECTOR_TYPE, invokeInstanceMethodMethod);
+			if(context == C.STATEMENT)
+				gen.pop();
 			}
-		if(context == C.STATEMENT)
-			gen.pop();
 	}
 
 	public boolean hasJavaClass(){
diff --git a/test/clojure/test_clojure/java_interop.clj b/test/clojure/test_clojure/java_interop.clj
@@ -13,7 +13,8 @@
   (:use clojure.test)
   (:require [clojure.inspector]
             [clojure.set :as set])
-  (:import java.util.Base64))
+  (:import java.util.Base64
+           (java.util.concurrent.atomic AtomicLong AtomicInteger)))
 
 ; http://clojure.org/java_interop
 ; http://clojure.org/compilation
@@ -589,3 +590,18 @@
   (is (= (char \a) \a)))
 
 ;; Note: More coercions in numbers.clj
+
+; Test that primitive boxing elision in statement context works
+; correctly (CLJ-2621)
+
+(defn inc-atomic-int [^AtomicInteger l]
+  (.incrementAndGet l)
+  nil)
+
+(defn inc-atomic-long [^AtomicLong l]
+  (.incrementAndGet l)
+  nil)
+
+(deftest test-boxing-prevention-when-compiling-statements
+  (is (= 1 (.get (doto (AtomicInteger. 0) inc-atomic-int))))
+  (is (= 1 (.get (doto (AtomicLong. 0) inc-atomic-long)))))

Original file line number	Diff line number	Diff line change
`@@ -1612,8 +1612,16 @@ public void emit(C context, ObjExpr objx, GeneratorAdapter gen){`
`1612`	`1612`	`gen.invokeInterface(type, m);`
`1613`	`1613`	`else`
`1614`	`1614`	`gen.invokeVirtual(type, m);`
`1615`		`- //if(context != C.STATEMENT \|\| method.getReturnType() == Void.TYPE)`
`1616`		`- HostExpr.emitBoxReturn(objx, gen, method.getReturnType());`
	`1615`	`+ Class retClass = method.getReturnType();`
	`1616`	`+ if(context == C.STATEMENT)`
	`1617`	`+ {`
	`1618`	`+ if(retClass == long.class \|\| retClass == double.class)`
	`1619`	`+ gen.pop2();`
	`1620`	`+ else if(retClass != void.class)`
	`1621`	`+ gen.pop();`
	`1622`	`+ }`
	`1623`	`+ else`
	`1624`	`+ HostExpr.emitBoxReturn(objx, gen, retClass);`
`1617`	`1625`	`}`
`1618`	`1626`	`else`
`1619`	`1627`	`{`
`@@ -1627,9 +1635,9 @@ public void emit(C context, ObjExpr objx, GeneratorAdapter gen){`
`1627`	`1635`	`method.emitClearLocals(gen);`
`1628`	`1636`	`}`
`1629`	`1637`	`gen.invokeStatic(REFLECTOR_TYPE, invokeInstanceMethodMethod);`
	`1638`	`+ if(context == C.STATEMENT)`
	`1639`	`+ gen.pop();`
`1630`	`1640`	`}`
`1631`		`- if(context == C.STATEMENT)`
`1632`		`- gen.pop();`
`1633`	`1641`	`}`
`1634`	`1642`
`1635`	`1643`	`public boolean hasJavaClass(){`