Code Monkey home page Code Monkey logo

Comments (20)

nadako avatar nadako commented on August 18, 2024

currently we have a filter for c#/java that sorts this stuff out. i think I even separated it from gencommon at some point.

it transforms try/catch so whatever is inherited from the base class is caught by native catches, and others are handled by is checks in a catch-em-all. of course throw also already wraps if needed.

from genjvm.

nadako avatar nadako commented on August 18, 2024

See tryCatchWrapper.ml

from genjvm.

Simn avatar Simn commented on August 18, 2024

I would like to avoid these filters. It shouldn't be necessary to transform it like that because we can check locally (at TThrow and TTry) what values we throw/catch and wrap accordingly.

from genjvm.

Simn avatar Simn commented on August 18, 2024

Plus that filter doesn't work quite right:

class Main {
	static public function main() {
		try {
			throw "string";
		} catch(e:String) {
			trace(e);
		} catch(e:java.lang.Throwable) {
			trace(e);
		}
	}
}

Generates:

try 
{
	try 
	{
		throw haxe.lang.HaxeException.wrap("string");
	}
	catch (java.lang.Throwable e)
	{
		haxe.lang.Exceptions.setException(e);
		haxe.Log.trace.__hx_invoke2_o(0.0, e, 0.0, new haxe.lang.DynamicObject(new java.lang.String[]{"className", "fileName", "methodName"}, new java.lang.Object[]{"Main", "src/Main.hx", "main"}, new java.lang.String[]{"lineNumber"}, new double[]{((double) (((double) (8) )) )}));
	}
	
	catch (java.lang.Throwable catchallException)
	{
		haxe.lang.Exceptions.setException(catchallException);
		java.lang.Object realException = ( (( catchallException instanceof haxe.lang.HaxeException )) ? (((haxe.lang.HaxeException) (catchallException) ).obj) : (catchallException) );
		if (( realException instanceof java.lang.String )) 
		{
			java.lang.String e1 = haxe.lang.Runtime.toString(realException);
			haxe.Log.trace.__hx_invoke2_o(0.0, e1, 0.0, new haxe.lang.DynamicObject(new java.lang.String[]{"className", "fileName", "methodName"}, new java.lang.Object[]{"Main", "src/Main.hx", "main"}, new java.lang.String[]{"lineNumber"}, new double[]{((double) (((double) (6) )) )}));
		}
		else
		{
			throw catchallException;
		}
		
	}			
}
catch (java.lang.Throwable typedException)
{
	throw haxe.lang.HaxeException.wrap(typedException);
}

Fails:

src\haxe\root\Main.java:53: error: exception Throwable has already been caught
                        catch (java.lang.Throwable catchallException)
                        ^
1 error

Even worse: If we catch java.lang.Exception instead it enters the wrong catch block.

from genjvm.

nadako avatar nadako commented on August 18, 2024

I don't follow. Doesn't that filter do exactly that?

E.g it transforms:

try throw "hello" catch (e:SomeJavaThrowable) trace("a") catch (e:String) trace("b");

into

try throw new HaxeException("hello")
catch (e:SomeJavaThrowable) trace("a")
catch (e:Throwable) {
 var actual = if (e is HaxeException) e.actual else e;
 if (actual is String) {
  trace("b")
 } else {
  throw e;
 }
}

from genjvm.

nadako avatar nadako commented on August 18, 2024

oops, you were faster :) i guess there are some bugs, but don't we still need a filter like that?

from genjvm.

nadako avatar nadako commented on August 18, 2024

Even worse: If we catch java.lang.Exception instead it enters the wrong catch block.

I think it's used to error there...

from genjvm.

Simn avatar Simn commented on August 18, 2024

Well, we need the functionality, but I don't want to add these extra expression passes if there's no good reason.

Maybe we can expose it in a way that we can just call its functionality from when we find a TThrow or TTry. But even then, we shouldn't have to work so much with the Haxe typed AST, especially if the native typing semantics are relevant.

from genjvm.

nadako avatar nadako commented on August 18, 2024

I see, well yes, we can do this on the generator level of course, if that's easy.

Note the thing that I keep forgetting: when processing throw, we want to do new HaxeException(value) if we are sure that value is not Throwable, but if the type is Dynamic - we need to call HaxeException.wrap(value) that does a run-time check for throwable to prevent double-wrapping.

from genjvm.

Simn avatar Simn commented on August 18, 2024

Yep makes sense, thanks!

I think the correct algorithm for the TTry catches would be something like this:

  1. Create list of the native (!) exception types of all caught types.
  2. While none of the tail exception types are assignable to the head exception type:
    1. Promote head exception type to a real catch.
    2. Pop head exception type.
  3. If there are exceptions left:
    1. Generate a catch (java.lang.Throwable).
    2. Create instanceof chain for remaining exceptions.
    3. Rethrow in the final else case.

Basically, we start with a list of types that we would have to instanceof-check (worst case), then check which of these we can promote to a catch without shadowing the others.

from genjvm.

Simn avatar Simn commented on August 18, 2024

The only issue with this is the "assignable" check. I suppose we have to do that on Haxe types after all because otherwise we cannot unify... But we still have to consider the native semantics, in particular java.lang.Throwable and java.lang.Exception subsuming all exception types.

from genjvm.

nadako avatar nadako commented on August 18, 2024

Well you don't need to unify there really? Just follow abstracts (if not yet), then if class: go up in the inheritance check to reach Throwable. if not class or no Throwable in the inheritance chain - it's "not assignable"

from genjvm.

Simn avatar Simn commented on August 18, 2024

I've done the initial work, but this is going to require some optimization/cleanup still.

	static function raise<T>(f:Void -> String) {
		return try {
			f();
		} catch(e:NativeExceptionChild) {
			'NativeExceptionChild: ${e.getMessage()}';
		} catch(e:NativeExceptionBase) {
			'NativeExceptionBase: ${e.getMessage()}';
		} catch(e:String) {
			'String: $e';
		} catch(e:NativeExceptionOther) {
			'NativeExceptionOther: ${e.getMessage()}';
		} catch(e:Int) {
			'Int: $e';
 		} catch(e:java.lang.Throwable) {
			'Throwable ${e.getMessage()}';
		} catch(e:Dynamic) {
			'Dynamic: $e';
		}
	}

Decompiled generated jvm:

   public static <T> String raise(MethodHandle f) {
      try {
         return f.invoke();
      } catch (NativeExceptionChild var2) {
         return Jvm.stringConcat("NativeExceptionChild: ", var2.getMessage());
      } catch (NativeExceptionBase var3) {
         return Jvm.stringConcat("NativeExceptionBase: ", var3.getMessage());
      } catch (Throwable var4) {
         Object var10000 = var4 instanceof Exception ? ((Exception)var4).value : (Object)var4;
         if ((var4 instanceof Exception ? ((Exception)var4).value : (Object)var4) instanceof String) {
            String e2 = (String)var10000;
            return Jvm.stringConcat("String: ", e2);
         } else if (var10000 instanceof NativeExceptionOther) {
            NativeExceptionOther e3 = (NativeExceptionOther)var10000;
            return Jvm.stringConcat("NativeExceptionOther: ", e3.getMessage());
         } else if (var10000 instanceof Integer) {
            int e4 = Jvm.toInt(var10000);
            return Jvm.stringConcat("Int: ", Jvm.toString(e4));
         } else if (var10000 instanceof Throwable) {
            Throwable e5 = (Throwable)var10000;
            return Jvm.stringConcat("Throwable ", e5.getMessage());
         } else if (var10000 instanceof Object) {
            Object e6 = var10000;
            return Jvm.stringConcat("Dynamic: ", Std.string(e6));
         } else {
            throw Exception.wrap(var10000);
         }
      }
   }

This shows the algorithm I outlined in the previous comment: It generates real catch expressions while it can, then defers to an instanceof chain. It's going to respect the lexical order this way and we can easily optimize for some cases, such as having a distinct exception type for String.

One TODO is that I'm currently re-wrapping the caught exception. That wasn't really intentional, I just didn't realize that I still need the original expression so I ignored it.

The failing genjava example I posted now generates this:

      try {
         throw Exception.wrap("string");
      } catch (Throwable var2) {
         Object var10000 = var2 instanceof Exception ? ((Exception)var2).value : (Object)var2;
         Object var10001;
         DynamicObject var10002;
         MethodHandle var4;
         if ((var2 instanceof Exception ? ((Exception)var2).value : (Object)var2) instanceof String) {
            String e = (String)var10000;
            var4 = Log.trace;
            var10001 = (Object)e;
            var10002 = new DynamicObject();
            var10002._hx_setField("fileName", "src/Main.hx");
            var10002._hx_setField("lineNumber", 6);
            var10002._hx_setField("className", "Main");
            var10002._hx_setField("methodName", "main");
            var4.invoke(var10001, var10002);
         } else {
            if (!(var10000 instanceof Throwable)) {
               throw Exception.wrap(var10000);
            }

            Throwable e1 = (Throwable)var10000;
            var4 = Log.trace;
            var10001 = (Object)e1;
            var10002 = new DynamicObject();
            var10002._hx_setField("fileName", "src/Main.hx");
            var10002._hx_setField("lineNumber", 8);
            var10002._hx_setField("className", "Main");
            var10002._hx_setField("methodName", "main");
            var4.invoke(var10001, var10002);
         }

      }

At first I thought that the var10000 instanceof Throwable check might be redundant, but that is not true because it checks the possibly unwrapped value here.

from genjvm.

Simn avatar Simn commented on August 18, 2024

Another thing I want to check is if this is really the best pattern:

        25: dup
        26: instanceof    #26                 // class java/lang/String
        29: ifeq          98
        32: checkcast     #26                 // class java/lang/String

I'm pretty sure the JIT doesn't care either way, but maybe this can be done better regardless.

from genjvm.

Simn avatar Simn commented on August 18, 2024

Something else to consider is the Exceptions attribute. The documentation says:

A method should throw an exception only if at least one of the following three criteria is met:

The exception is an instance of RuntimeException or one of its subclasses.

The exception is an instance of Error or one of its subclasses.

The exception is an instance of one of the exception classes specified in the exception_index_table just described, or one of their subclasses.

These requirements are not enforced in the Java Virtual Machine; they are enforced only at compile-time.

We can easily infer this for concrete occurrences of throw, but it gets trickier if we have to consider calls to other functions. This would require a preliminary inter-procedural analysis step to determine who throws what.

I have no idea how strict their "should" is and what happens if these rules are violated.

from genjvm.

Simn avatar Simn commented on August 18, 2024

Also related: Any invoke mechanism can cause a Throwable exception. That should just mean that whenever we invoke something unknown (through invokedynamic once we finally understood that), we add Throwable to the list of thrown exceptions.

from genjvm.

nadako avatar nadako commented on August 18, 2024

This would require a preliminary inter-procedural analysis step to determine who throws what.

I wonder if this can be also used for diagnostics like "you forgot to handle this exception" and "this exception is never thrown". But I guess this won't be ever accurate with all the virtual and dynamic calls...

from genjvm.

Simn avatar Simn commented on August 18, 2024

No more re-wrapping:

    public static <T> String raise(MethodHandle f) {
        try {
            return f.invoke();
        } catch (NativeExceptionChild var3) {
            return Jvm.stringConcat("caught NativeExceptionChild: ", var3.getMessage());
        } catch (NativeExceptionBase var4) {
            return Jvm.stringConcat("caught NativeExceptionBase: ", var4.getMessage());
        } catch (Throwable var5) {
            Object var10000 = var5;
            if (var5 instanceof Exception) {
                var10000 = ((Exception)var5).value;
            }

            if (var10000 instanceof String) {
                String e2 = (String)var10000;
                return Jvm.stringConcat("caught String: ", e2);
            } else if (var10000 instanceof NativeExceptionOther) {
                NativeExceptionOther e3 = (NativeExceptionOther)var10000;
                return Jvm.stringConcat("caught NativeExceptionOther: ", e3.getMessage());
            } else if (var10000 instanceof Integer) {
                int e4 = Jvm.toInt(var10000);
                return Jvm.stringConcat("caught Int: ", Jvm.toString(e4));
            } else if (var10000 instanceof Throwable) {
                Throwable e5 = (Throwable)var10000;
                return Jvm.stringConcat("caught Throwable: ", e5.getMessage());
            } else if (var10000 instanceof Object) {
                Object e6 = var10000;
                return Jvm.stringConcat("caught Dynamic: ", Std.string(e6));
            } else {
                throw var5;
            }
        }
    }

Reopening though so I don't forget checking that Exceptions attribute situation.

from genjvm.

Simn avatar Simn commented on August 18, 2024

I don't really remember implementing it but it looks like we generate the Exceptions attribute now. Should re-check after @RealyUniqueName's new exception handling is merged to make sure it's still there.

from genjvm.

Simn avatar Simn commented on August 18, 2024

Re-checked - still there.

from genjvm.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.