Mark line number after emitting children.

pjstadig · stuarthalloway · commit 715754d3f69e · 2015-03-20T13:18:22.000-04:00
Marking the line number before emitting children leads to incorrect line numbers
when a runtime error occurs.  For example, when

 (foo bar
      baz)

is emitted the compiler will visit the line number for the expression, then emit
the children expressions ('bar' and 'baz') which will mark their own line
numbers, then come back and emit the invoke bytecode for 'foo', but since the
last line number to be marked was that of 'baz', if 'foo' throws an exception
the line number of 'baz' will be reported instead of the line number for the
expression as a whole.

This same issue was being manifested with special forms and inlined functions,
and was especially bad in the case of the threading macro '-&gt;', because it is
usually spread across several lines, and the line number reported could end up
being very different than the line actually causing an exception.

Signed-off-by: Stuart Halloway &lt;stu@cognitect.com&gt;
diff --git a/src/jvm/clojure/lang/Compiler.java b/src/jvm/clojure/lang/Compiler.java
@@ -1165,10 +1165,10 @@ public boolean canEmitPrimitive(){
 	}
 
 	public void emitUnboxed(C context, ObjExpr objx, GeneratorAdapter gen){
-		gen.visitLineNumber(line, gen.mark());
 		if(targetClass != null && field != null)
 			{
 			target.emit(C.EXPRESSION, objx, gen);
+			gen.visitLineNumber(line, gen.mark());
 			gen.checkCast(getType(targetClass));
 			gen.getField(getType(targetClass), fieldName, Type.getType(field.getType()));
 			}
@@ -1177,10 +1177,10 @@ public void emitUnboxed(C context, ObjExpr objx, GeneratorAdapter gen){
 	}
 
 	public void emit(C context, ObjExpr objx, GeneratorAdapter gen){
-		gen.visitLineNumber(line, gen.mark());
 		if(targetClass != null && field != null)
 			{
 			target.emit(C.EXPRESSION, objx, gen);
+			gen.visitLineNumber(line, gen.mark());
 			gen.checkCast(getType(targetClass));
 			gen.getField(getType(targetClass), fieldName, Type.getType(field.getType()));
 			//if(context != C.STATEMENT)
@@ -1193,6 +1193,7 @@ public void emit(C context, ObjExpr objx, GeneratorAdapter gen){
 		else
 			{
 			target.emit(C.EXPRESSION, objx, gen);
+			gen.visitLineNumber(line, gen.mark());
 			gen.push(fieldName);
 			gen.push(requireField);
 			gen.invokeStatic(REFLECTOR_TYPE, invokeNoArgInstanceMember);
@@ -1215,12 +1216,12 @@ public Object evalAssign(Expr val) {
 
 	public void emitAssign(C context, ObjExpr objx, GeneratorAdapter gen,
 	                       Expr val){
-		gen.visitLineNumber(line, gen.mark());
 		if(targetClass != null && field != null)
 			{
 			target.emit(C.EXPRESSION, objx, gen);
 			gen.checkCast(Type.getType(targetClass));
 			val.emit(C.EXPRESSION, objx, gen);
+			gen.visitLineNumber(line, gen.mark());
 			gen.dupX1();
 			HostExpr.emitUnboxArg(objx, gen, field.getType());
 			gen.putField(Type.getType(targetClass), fieldName, Type.getType(field.getType()));
@@ -1230,6 +1231,7 @@ public void emitAssign(C context, ObjExpr objx, GeneratorAdapter gen,
 			target.emit(C.EXPRESSION, objx, gen);
 			gen.push(fieldName);
 			val.emit(C.EXPRESSION, objx, gen);
+			gen.visitLineNumber(line, gen.mark());
 			gen.invokeStatic(REFLECTOR_TYPE, setInstanceFieldMethod);
 			}
 		if(context == C.STATEMENT)
@@ -1310,8 +1312,8 @@ public Object evalAssign(Expr val) {
 
 	public void emitAssign(C context, ObjExpr objx, GeneratorAdapter gen,
 	                       Expr val){
-		gen.visitLineNumber(line, gen.mark());
 		val.emit(C.EXPRESSION, objx, gen);
+		gen.visitLineNumber(line, gen.mark());
 		gen.dup();
 		HostExpr.emitUnboxArg(objx, gen, field.getType());
 		gen.putStatic(Type.getType(c), fieldName, Type.getType(field.getType()));
@@ -1532,14 +1534,14 @@ public boolean canEmitPrimitive(){
 	}
 
 	public void emitUnboxed(C context, ObjExpr objx, GeneratorAdapter gen){
-		gen.visitLineNumber(line, gen.mark());
 		if(method != null)
 			{
 			Type type = Type.getType(method.getDeclaringClass());
 			target.emit(C.EXPRESSION, objx, gen);
 			//if(!method.getDeclaringClass().isInterface())
 			gen.checkCast(type);
 			MethodExpr.emitTypedArgs(objx, gen, method.getParameterTypes(), args);
+			gen.visitLineNumber(line, gen.mark());
 			if(context == C.RETURN)
 				{
 				ObjMethod method = (ObjMethod) METHOD.deref();
@@ -1556,14 +1558,14 @@ public void emitUnboxed(C context, ObjExpr objx, GeneratorAdapter gen){
 	}
 
 	public void emit(C context, ObjExpr objx, GeneratorAdapter gen){
-		gen.visitLineNumber(line, gen.mark());
 		if(method != null)
 			{
 			Type type = Type.getType(method.getDeclaringClass());
 			target.emit(C.EXPRESSION, objx, gen);
 			//if(!method.getDeclaringClass().isInterface())
 			gen.checkCast(type);
 			MethodExpr.emitTypedArgs(objx, gen, method.getParameterTypes(), args);
+			gen.visitLineNumber(line, gen.mark());
 			if(context == C.RETURN)
 				{
 				ObjMethod method = (ObjMethod) METHOD.deref();
@@ -1582,6 +1584,7 @@ public void emit(C context, ObjExpr objx, GeneratorAdapter gen){
 			target.emit(C.EXPRESSION, objx, gen);
 			gen.push(methodName);
 			emitArgsAsArray(args, objx, gen);
+			gen.visitLineNumber(line, gen.mark());
 			if(context == C.RETURN)
 				{
 				ObjMethod method = (ObjMethod) METHOD.deref();
@@ -1727,10 +1730,10 @@ public void emitIntrinsicPredicate(C context, ObjExpr objx, GeneratorAdapter gen
 	}
 
 	public void emitUnboxed(C context, ObjExpr objx, GeneratorAdapter gen){
-		gen.visitLineNumber(line, gen.mark());
 		if(method != null)
 			{
 			MethodExpr.emitTypedArgs(objx, gen, method.getParameterTypes(), args);
+			gen.visitLineNumber(line, gen.mark());
 			//Type type = Type.getObjectType(className.replace('.', '/'));
 			if(context == C.RETURN)
 				{
@@ -1760,10 +1763,10 @@ public void emitUnboxed(C context, ObjExpr objx, GeneratorAdapter gen){
 	}
 
 	public void emit(C context, ObjExpr objx, GeneratorAdapter gen){
-		gen.visitLineNumber(line, gen.mark());
 		if(method != null)
 			{
 			MethodExpr.emitTypedArgs(objx, gen, method.getParameterTypes(), args);
+			gen.visitLineNumber(line, gen.mark());
 			//Type type = Type.getObjectType(className.replace('.', '/'));
 			if(context == C.RETURN)
 				{
@@ -1789,10 +1792,12 @@ else if(retClass != void.class)
 			}
 		else
 			{
+			gen.visitLineNumber(line, gen.mark());
 			gen.push(c.getName());
 			gen.invokeStatic(RT_TYPE, forNameMethod);
 			gen.push(methodName);
 			emitArgsAsArray(args, objx, gen);
+			gen.visitLineNumber(line, gen.mark());
 			if(context == C.RETURN)
 				{
 				ObjMethod method = (ObjMethod) METHOD.deref();
@@ -3255,6 +3260,7 @@ public void emit(C context, ObjExpr objx, GeneratorAdapter gen){
         gen.getStatic(objx.objtype, objx.thunkNameStatic(siteIndex),ObjExpr.ILOOKUP_THUNK_TYPE);
         gen.dup();  //thunk, thunk
         target.emit(C.EXPRESSION, objx, gen); //thunk,thunk,target
+        gen.visitLineNumber(line, gen.mark());
         gen.dupX2();                          //target,thunk,thunk,target
         gen.invokeInterface(ObjExpr.ILOOKUP_THUNK_TYPE, Method.getMethod("Object get(Object)")); //target,thunk,result
         gen.dupX2();                          //result,target,thunk,result
@@ -3626,15 +3632,16 @@ public Object eval() {
 	}
 
 	public void emit(C context, ObjExpr objx, GeneratorAdapter gen){
-		gen.visitLineNumber(line, gen.mark());
 		if(isProtocol)
 			{
+			gen.visitLineNumber(line, gen.mark());
 			emitProto(context,objx,gen);
 			}
 
 		else
 			{
 			fexpr.emit(C.EXPRESSION, objx, gen);
+			gen.visitLineNumber(line, gen.mark());
 			gen.checkCast(IFN_TYPE);
 			emitArgsAndCall(0, context,objx,gen);
 			}
@@ -3704,6 +3711,7 @@ void emitArgsAndCall(int firstArgToEmit, C context, ObjExpr objx, GeneratorAdapt
 				}
 			MethodExpr.emitArgsAsArray(restArgs, objx, gen);
 			}
+		gen.visitLineNumber(line, gen.mark());
 
 		if(context == C.RETURN)
 			{

Original file line number	Diff line number	Diff line change
`@@ -1165,10 +1165,10 @@ public boolean canEmitPrimitive(){`
`1165`	`1165`	`}`
`1166`	`1166`
`1167`	`1167`	`public void emitUnboxed(C context, ObjExpr objx, GeneratorAdapter gen){`
`1168`		`- gen.visitLineNumber(line, gen.mark());`
`1169`	`1168`	`if(targetClass != null && field != null)`
`1170`	`1169`	`{`
`1171`	`1170`	`target.emit(C.EXPRESSION, objx, gen);`
	`1171`	`+ gen.visitLineNumber(line, gen.mark());`
`1172`	`1172`	`gen.checkCast(getType(targetClass));`
`1173`	`1173`	`gen.getField(getType(targetClass), fieldName, Type.getType(field.getType()));`
`1174`	`1174`	`}`
`@@ -1177,10 +1177,10 @@ public void emitUnboxed(C context, ObjExpr objx, GeneratorAdapter gen){`
`1177`	`1177`	`}`
`1178`	`1178`
`1179`	`1179`	`public void emit(C context, ObjExpr objx, GeneratorAdapter gen){`
`1180`		`- gen.visitLineNumber(line, gen.mark());`
`1181`	`1180`	`if(targetClass != null && field != null)`
`1182`	`1181`	`{`
`1183`	`1182`	`target.emit(C.EXPRESSION, objx, gen);`
	`1183`	`+ gen.visitLineNumber(line, gen.mark());`
`1184`	`1184`	`gen.checkCast(getType(targetClass));`
`1185`	`1185`	`gen.getField(getType(targetClass), fieldName, Type.getType(field.getType()));`
`1186`	`1186`	`//if(context != C.STATEMENT)`
`@@ -1193,6 +1193,7 @@ public void emit(C context, ObjExpr objx, GeneratorAdapter gen){`
`1193`	`1193`	`else`
`1194`	`1194`	`{`
`1195`	`1195`	`target.emit(C.EXPRESSION, objx, gen);`
	`1196`	`+ gen.visitLineNumber(line, gen.mark());`
`1196`	`1197`	`gen.push(fieldName);`
`1197`	`1198`	`gen.push(requireField);`
`1198`	`1199`	`gen.invokeStatic(REFLECTOR_TYPE, invokeNoArgInstanceMember);`
`@@ -1215,12 +1216,12 @@ public Object evalAssign(Expr val) {`
`1215`	`1216`
`1216`	`1217`	`public void emitAssign(C context, ObjExpr objx, GeneratorAdapter gen,`
`1217`	`1218`	`Expr val){`
`1218`		`- gen.visitLineNumber(line, gen.mark());`
`1219`	`1219`	`if(targetClass != null && field != null)`
`1220`	`1220`	`{`
`1221`	`1221`	`target.emit(C.EXPRESSION, objx, gen);`
`1222`	`1222`	`gen.checkCast(Type.getType(targetClass));`
`1223`	`1223`	`val.emit(C.EXPRESSION, objx, gen);`
	`1224`	`+ gen.visitLineNumber(line, gen.mark());`
`1224`	`1225`	`gen.dupX1();`
`1225`	`1226`	`HostExpr.emitUnboxArg(objx, gen, field.getType());`
`1226`	`1227`	`gen.putField(Type.getType(targetClass), fieldName, Type.getType(field.getType()));`
`@@ -1230,6 +1231,7 @@ public void emitAssign(C context, ObjExpr objx, GeneratorAdapter gen,`
`1230`	`1231`	`target.emit(C.EXPRESSION, objx, gen);`
`1231`	`1232`	`gen.push(fieldName);`
`1232`	`1233`	`val.emit(C.EXPRESSION, objx, gen);`
	`1234`	`+ gen.visitLineNumber(line, gen.mark());`
`1233`	`1235`	`gen.invokeStatic(REFLECTOR_TYPE, setInstanceFieldMethod);`
`1234`	`1236`	`}`
`1235`	`1237`	`if(context == C.STATEMENT)`
`@@ -1310,8 +1312,8 @@ public Object evalAssign(Expr val) {`
`1310`	`1312`
`1311`	`1313`	`public void emitAssign(C context, ObjExpr objx, GeneratorAdapter gen,`
`1312`	`1314`	`Expr val){`
`1313`		`- gen.visitLineNumber(line, gen.mark());`
`1314`	`1315`	`val.emit(C.EXPRESSION, objx, gen);`
	`1316`	`+ gen.visitLineNumber(line, gen.mark());`
`1315`	`1317`	`gen.dup();`
`1316`	`1318`	`HostExpr.emitUnboxArg(objx, gen, field.getType());`
`1317`	`1319`	`gen.putStatic(Type.getType(c), fieldName, Type.getType(field.getType()));`
`@@ -1532,14 +1534,14 @@ public boolean canEmitPrimitive(){`
`1532`	`1534`	`}`
`1533`	`1535`
`1534`	`1536`	`public void emitUnboxed(C context, ObjExpr objx, GeneratorAdapter gen){`
`1535`		`- gen.visitLineNumber(line, gen.mark());`
`1536`	`1537`	`if(method != null)`
`1537`	`1538`	`{`
`1538`	`1539`	`Type type = Type.getType(method.getDeclaringClass());`
`1539`	`1540`	`target.emit(C.EXPRESSION, objx, gen);`
`1540`	`1541`	`//if(!method.getDeclaringClass().isInterface())`
`1541`	`1542`	`gen.checkCast(type);`
`1542`	`1543`	`MethodExpr.emitTypedArgs(objx, gen, method.getParameterTypes(), args);`
	`1544`	`+ gen.visitLineNumber(line, gen.mark());`
`1543`	`1545`	`if(context == C.RETURN)`
`1544`	`1546`	`{`
`1545`	`1547`	`ObjMethod method = (ObjMethod) METHOD.deref();`
`@@ -1556,14 +1558,14 @@ public void emitUnboxed(C context, ObjExpr objx, GeneratorAdapter gen){`
`1556`	`1558`	`}`
`1557`	`1559`
`1558`	`1560`	`public void emit(C context, ObjExpr objx, GeneratorAdapter gen){`
`1559`		`- gen.visitLineNumber(line, gen.mark());`
`1560`	`1561`	`if(method != null)`
`1561`	`1562`	`{`
`1562`	`1563`	`Type type = Type.getType(method.getDeclaringClass());`
`1563`	`1564`	`target.emit(C.EXPRESSION, objx, gen);`
`1564`	`1565`	`//if(!method.getDeclaringClass().isInterface())`
`1565`	`1566`	`gen.checkCast(type);`
`1566`	`1567`	`MethodExpr.emitTypedArgs(objx, gen, method.getParameterTypes(), args);`
	`1568`	`+ gen.visitLineNumber(line, gen.mark());`
`1567`	`1569`	`if(context == C.RETURN)`
`1568`	`1570`	`{`
`1569`	`1571`	`ObjMethod method = (ObjMethod) METHOD.deref();`
`@@ -1582,6 +1584,7 @@ public void emit(C context, ObjExpr objx, GeneratorAdapter gen){`
`1582`	`1584`	`target.emit(C.EXPRESSION, objx, gen);`
`1583`	`1585`	`gen.push(methodName);`
`1584`	`1586`	`emitArgsAsArray(args, objx, gen);`
	`1587`	`+ gen.visitLineNumber(line, gen.mark());`
`1585`	`1588`	`if(context == C.RETURN)`
`1586`	`1589`	`{`
`1587`	`1590`	`ObjMethod method = (ObjMethod) METHOD.deref();`
`@@ -1727,10 +1730,10 @@ public void emitIntrinsicPredicate(C context, ObjExpr objx, GeneratorAdapter gen`
`1727`	`1730`	`}`
`1728`	`1731`
`1729`	`1732`	`public void emitUnboxed(C context, ObjExpr objx, GeneratorAdapter gen){`
`1730`		`- gen.visitLineNumber(line, gen.mark());`
`1731`	`1733`	`if(method != null)`
`1732`	`1734`	`{`
`1733`	`1735`	`MethodExpr.emitTypedArgs(objx, gen, method.getParameterTypes(), args);`
	`1736`	`+ gen.visitLineNumber(line, gen.mark());`
`1734`	`1737`	`//Type type = Type.getObjectType(className.replace('.', '/'));`
`1735`	`1738`	`if(context == C.RETURN)`
`1736`	`1739`	`{`
`@@ -1760,10 +1763,10 @@ public void emitUnboxed(C context, ObjExpr objx, GeneratorAdapter gen){`
`1760`	`1763`	`}`
`1761`	`1764`
`1762`	`1765`	`public void emit(C context, ObjExpr objx, GeneratorAdapter gen){`
`1763`		`- gen.visitLineNumber(line, gen.mark());`
`1764`	`1766`	`if(method != null)`
`1765`	`1767`	`{`
`1766`	`1768`	`MethodExpr.emitTypedArgs(objx, gen, method.getParameterTypes(), args);`
	`1769`	`+ gen.visitLineNumber(line, gen.mark());`
`1767`	`1770`	`//Type type = Type.getObjectType(className.replace('.', '/'));`
`1768`	`1771`	`if(context == C.RETURN)`
`1769`	`1772`	`{`
`@@ -1789,10 +1792,12 @@ else if(retClass != void.class)`
`1789`	`1792`	`}`
`1790`	`1793`	`else`
`1791`	`1794`	`{`
	`1795`	`+ gen.visitLineNumber(line, gen.mark());`
`1792`	`1796`	`gen.push(c.getName());`
`1793`	`1797`	`gen.invokeStatic(RT_TYPE, forNameMethod);`
`1794`	`1798`	`gen.push(methodName);`
`1795`	`1799`	`emitArgsAsArray(args, objx, gen);`
	`1800`	`+ gen.visitLineNumber(line, gen.mark());`
`1796`	`1801`	`if(context == C.RETURN)`
`1797`	`1802`	`{`
`1798`	`1803`	`ObjMethod method = (ObjMethod) METHOD.deref();`
`@@ -3255,6 +3260,7 @@ public void emit(C context, ObjExpr objx, GeneratorAdapter gen){`
`3255`	`3260`	`gen.getStatic(objx.objtype, objx.thunkNameStatic(siteIndex),ObjExpr.ILOOKUP_THUNK_TYPE);`
`3256`	`3261`	`gen.dup(); //thunk, thunk`
`3257`	`3262`	`target.emit(C.EXPRESSION, objx, gen); //thunk,thunk,target`
	`3263`	`+ gen.visitLineNumber(line, gen.mark());`
`3258`	`3264`	`gen.dupX2(); //target,thunk,thunk,target`
`3259`	`3265`	`gen.invokeInterface(ObjExpr.ILOOKUP_THUNK_TYPE, Method.getMethod("Object get(Object)")); //target,thunk,result`
`3260`	`3266`	`gen.dupX2(); //result,target,thunk,result`
`@@ -3626,15 +3632,16 @@ public Object eval() {`
`3626`	`3632`	`}`
`3627`	`3633`
`3628`	`3634`	`public void emit(C context, ObjExpr objx, GeneratorAdapter gen){`
`3629`		`- gen.visitLineNumber(line, gen.mark());`
`3630`	`3635`	`if(isProtocol)`
`3631`	`3636`	`{`
	`3637`	`+ gen.visitLineNumber(line, gen.mark());`
`3632`	`3638`	`emitProto(context,objx,gen);`
`3633`	`3639`	`}`
`3634`	`3640`
`3635`	`3641`	`else`
`3636`	`3642`	`{`
`3637`	`3643`	`fexpr.emit(C.EXPRESSION, objx, gen);`
	`3644`	`+ gen.visitLineNumber(line, gen.mark());`
`3638`	`3645`	`gen.checkCast(IFN_TYPE);`
`3639`	`3646`	`emitArgsAndCall(0, context,objx,gen);`
`3640`	`3647`	`}`
`@@ -3704,6 +3711,7 @@ void emitArgsAndCall(int firstArgToEmit, C context, ObjExpr objx, GeneratorAdapt`
`3704`	`3711`	`}`
`3705`	`3712`	`MethodExpr.emitArgsAsArray(restArgs, objx, gen);`
`3706`	`3713`	`}`
	`3714`	`+ gen.visitLineNumber(line, gen.mark());`
`3707`	`3715`
`3708`	`3716`	`if(context == C.RETURN)`
`3709`	`3717`	`{`