gf-core icon indicating copy to clipboard operation
gf-core copied to clipboard
verified publisher icon GrammaticalFramework

Java bindings to Majestic runtime

Open harisont opened this issue 4 years ago • 14 comments

Hi, I'm getting started with the Java bindings for the new runtime and so far it seems I got readPGF(path) and getAbstractName working.

Even though this is still very little, I'm opening this pull request so that maybe @krangelov can tell me how I should go about in terms of memory management in what I already implemented, so that I can continue along those lines (I have not noticed any strange behavior so far, but I'd be surprised if there were no memory issues, I don't really have a clue about that, and especially not in the context of the JNI).

harisont avatar Sep 24 '21 15:09 harisont

Looks good! There are three things that I noticed:

  • To release the memory correctly, implement the finalize for the PGF class. From the method call pgf_free_revision

  • If you get err.type == PGF_EXN_OTHER_ERROR then just clean up any allocated resources and return NULL. This error is used when the runtime of the host language, e.g. Java or Python, has detected an error and then we must abort the whole operation. If that happens the host runtime has already generated the exception and we don't want to generate a new one. This is probably wrong in Python as well. I will take a look.

  • You can't use (*env)->NewStringUTF(env,txt->text). There are two reasons. First using txt->text ignores the size field which means that the call will fail if the string contains the zero character. Second, Java is using a modified version of UTF-8 while we use the standard one. You should implement your own conversions to and from UTF-8. gu2j_string and j2gu_string should be good starting points. Java is using UTF-16 for storing text and NewStringUTF is converting to a modified UTF-8. Since we want the conversion to work in a different way, we should not use NewStringUTF to avoid double conversion. Instead use NewString, GetStringLength and GetStringChars.

On Fri, 24 Sept 2021 at 17:02, Arianna Masciolini @.***> wrote:

Hi, I'm getting started with the Java bindings for the new runtime and so far it seems I got readPGF(path) and getAbstractName working.

Even though this is still very little, I'm opening this pull request so that maybe @krangelov https://github.com/krangelov can tell me how I should go about in terms of memory management in what I already implemented, so that I can continue along those lines (I have not noticed any strange behavior so far, but I'd be surprised if there were no memory issues, I don't really have a clue about that, and especially not in the context of the JNI).

You can view, comment on, or merge this pull request online at:

https://github.com/GrammaticalFramework/gf-core/pull/133 Commit Summary

File Changes

Patch Links:

  • https://github.com/GrammaticalFramework/gf-core/pull/133.patch
  • https://github.com/GrammaticalFramework/gf-core/pull/133.diff

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GrammaticalFramework/gf-core/pull/133, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEYFSZG7LP2VFYPUI7NPEGDUDSHIJANCNFSM5EWEY4BQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

krangelov avatar Sep 25 '21 06:09 krangelov

Thank you! I might come back with further question as I make these changes but everything seems clear for now.

harisont avatar Sep 27 '21 08:09 harisont

I'm not sure how to do the last thing.

I understand why I should use NewString instead of NewStringUTF, but I do not know how to get from let's say txt->text to the corresponding string in the correct encoding.

If I am not mistaken, txt->text is standard UTF-8 to be converted to UTF-16, and I assume conversion is similar to that in gu2j_string. The thing is, I don't really understand how GuStrings and GuUCSs work, which makes me unsure about how a hypothetical utf82j_string should differ from gu2j_string itself.

harisont avatar Sep 27 '21 13:09 harisont

Push the code as it is and I can update it.

On Mon, 27 Sept 2021 at 15:48, Arianna Masciolini @.***> wrote:

I'm not sure how to do the last thing.

I understand why I should use NewString instead of NewStringUTF, but I do not know how to get from let's say txt->text to the corresponding string in the correct encoding.

If I am not mistaken, txt->text is standard UTF-8 to be converted to UTF-16, and I assume conversion is similar to that in gu2j_string. The thing is, I don't really understand how GuStrings and GuUCSs work, which makes me unsure about how a hypothetical utf82j_string should differ from gu2j_string itself.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GrammaticalFramework/gf-core/pull/133#issuecomment-927892871, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEYFSZGHJVOPIHHAEJHUPBLUEBY3PANCNFSM5EWEY4BQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

krangelov avatar Sep 27 '21 14:09 krangelov

Question for @krangelov:

I'm working on some abstract syntax methods of the PGF class, specifically getStartCat and getCategories, and looking at the old bindings I noticed that they return respectively a String and a list of List<String>. Given that there is already a class Type as part of the Java bindings, shouldn't these method return instead a Type and a list of Types (like in Haskell and Python)?

To me, that seems only logical, but at the same time I imagine that there is a reason for having done otherwise and wanted to know what your thoughts are before breaking backwards compatibility.

harisont avatar Sep 30 '21 13:09 harisont

Similarly, shouldn0t the input of getFunctionsByCat be a Type?

harisont avatar Sep 30 '21 14:09 harisont

Probably when I added getStartCat, there wasn't a type for classes. Features in the Java binding were developed on demand. I think getStartCat should return Type.

getCategories should return strings just like getFunctions which also returns only function names.

On Thu, 30 Sept 2021 at 15:43, Arianna Masciolini @.***> wrote:

Question for @krangelov https://github.com/krangelov:

I'm working on some abstract syntax methods of the PGF class, specifically getStartCat and getCategories, and looking at the old bindings I noticed that they return respectively a String and a list of List<String>. Given that there is already a class Type as part of the Java bindings, shouldn't these method return instead a Type and a list of Types (like in Haskell and Python)?

To me, that seems only logical, but at the same time I imagine that there is a reason for having done otherwise and wanted to know what your thoughts are before breaking backwards compatibility.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GrammaticalFramework/gf-core/pull/133#issuecomment-931335174, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEYFSZEBAAWARKFRM3VXEELUERSOLANCNFSM5EWEY4BQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

krangelov avatar Sep 30 '21 18:09 krangelov

On Thu, 30 Sept 2021 at 16:09, Arianna Masciolini @.***> wrote:

Similarly, shouldn0t the input of getFunctionsByCat be a Type?

No. getFunctionsByCat should take a string as input. If we filter by type then we must do type checking as well.

krangelov avatar Sep 30 '21 18:09 krangelov

What do you mean? I see this in the master repository:

/** Returns the type of the function with the given name.

  • @param fun The name of the function. */ public native Type getFunctionType(String fun);

On Wed, 20 Oct 2021 at 12:37, Arianna Masciolini @.***> wrote:

Probably when I added getStartCat, there wasn't a type for classes. Features in the Java binding were developed on demand. I think getStartCat should return Type. getCategories should return strings just like getFunctions which also returns only function names.

Maybe getStartCat should also return a string after all. getFunctionType also returns a string instead of a Type, now that I look at it...

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GrammaticalFramework/gf-core/pull/133#issuecomment-947541503, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEYFSZCCPW3FTLB7NYPKKCDUH2LXBANCNFSM5EWEY4BQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

krangelov avatar Oct 20 '21 12:10 krangelov

Yes, sorry. I had mixed up two things, in fact I deleted the comment immediately, but you still got the email. On Oct 20 2021, at 2:14 pm, Krasimir Angelov @.***> wrote:

What do you mean? I see this in the master repository: /** Returns the type of the function with the given name.

  • @param fun The name of the function. */ public native Type getFunctionType(String fun);

On Wed, 20 Oct 2021 at 12:37, Arianna Masciolini @.***> wrote:

Probably when I added getStartCat, there wasn't a type for classes. Features in the Java binding were developed on demand. I think getStartCat should return Type. getCategories should return strings just like getFunctions which also returns only function names.

Maybe getStartCat should also return a string after all. getFunctionType also returns a string instead of a Type, now that I look at it...

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GrammaticalFramework/gf-core/pull/133#issuecomment-947541503, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEYFSZCCPW3FTLB7NYPKKCDUH2LXBANCNFSM5EWEY4BQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub (https://github.com/GrammaticalFramework/gf-core/pull/133#issuecomment-947606169), or unsubscribe (https://github.com/notifications/unsubscribe-auth/AEJI3XYLCBJU7442H5MEPM3UH2XDZANCNFSM5EWEY4BQ).

harisont avatar Oct 20 '21 13:10 harisont

I'm having issues implementing the marshaller. In particular, I'm working on match_type.

I know that this makes the program crash, and initially I thought that the issue was related to casting the result of getObjectField to jobjectArray (or, in general, that it was an array problem), but that doesn't seem to be the case. In fact, I think the reason for the failure is that the fieldID I pass to getObjectField is NULL, even though I don't understand why, and even though I thought that, with a null field id, GetObjectField itself would fail in the first place.

I then also noticed that, similarly, trying to use the cat field also causes the program to crash.

Therefore I think that the problem is not specific to any field of Type objects, and I suspect it is somehow connected to the env pointer, which I don't know if I'm using correctly. To be honest, even if so far everything seems to work in the unmarshaller I'm in general unsure about what I should do with it in both marshalling and unmarshalling.

Can anyone help me with that?

harisont avatar Oct 26 '21 13:10 harisont

In both cases, I see that there is a missing semicolon. It should be:

"[Lorg/grammaticalframework/pgf/Hypo;" "Ljava/lang/String;"

I think that is the reason.

In general if you get 0 from GetFieldID then you should return back to Java. Java will then show an exception which will hopefully clarify the issue.

On Tue, 26 Oct 2021 at 15:06, Arianna Masciolini @.***> wrote:

I'm having issues implementing the marshaller. In particular, I'm working on match_type.

I know that this https://github.com/harisont/gf-core/blob/0fdcce39fcc91ed9968e22d5c8a8873f7f9c8837/src/runtime/java/jpgf.c#L161 makes the program crash, and initially I thought that the issue was related to casting the result of getObjectField to jobjectArray https://github.com/harisont/gf-core/blob/0fdcce39fcc91ed9968e22d5c8a8873f7f9c8837/src/runtime/java/jpgf.c#L160 (or, in general, that it was an array problem), but that doesn't seem to be the case. In fact, I think the reason for the failure is that the fieldID I pass to getObjectField https://github.com/harisont/gf-core/blob/0fdcce39fcc91ed9968e22d5c8a8873f7f9c8837/src/runtime/java/jpgf.c#L159 is NULL, even though I don't understand why, and even though I thought that, with a null field id, GetObjectField itself would fail in the first place.

I then also noticed that, similarly, trying to use the cat field also causes the program to crash https://github.com/harisont/gf-core/blob/0fdcce39fcc91ed9968e22d5c8a8873f7f9c8837/src/runtime/java/jpgf.c#L168-L170 .

Therefore I think that the problem is not specific to any field of Type objects, and I suspect it is somehow connected to the env pointer, which I don't know if I'm using correctly. To be honest, even if so far everything seems to work in the unmarshaller I'm in general unsure about what I should do with it in both marshalling and unmarshalling.

Can anyone help me with that?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GrammaticalFramework/gf-core/pull/133#issuecomment-951919490, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEYFSZADDUKPQ55V3TI4NE3UI2YU5ANCNFSM5EWEY4BQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

krangelov avatar Oct 26 '21 19:10 krangelov

Oh but of course! :facepalm: The example(s) in the JNI documentation had made me misunderstand what the semicolons'role in signatures is

harisont avatar Oct 27 '21 08:10 harisont

I also missed the semicolon the first time when I used the JNI. Something in their documentation should be improved.

On Wed, 27 Oct 2021 at 10:07, Arianna Masciolini @.***> wrote:

Oh but of course! 🤦 The example(s) in the JNI documentation had made me misunderstand what the semicolons'role in signatures is

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GrammaticalFramework/gf-core/pull/133#issuecomment-952645851, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEYFSZBTZV5E4Z6PDADEXETUI66MJANCNFSM5EWEY4BQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

krangelov avatar Oct 27 '21 11:10 krangelov