From: Gergely K. <ger...@ma...> - 2010-02-28 20:31:40
|
Hi, I uploaded a pretty big patch with various ObjC - Java API improvements. http://xmlvm-reviews.appspot.com/18002 This patch implements many different features: - String handling improvements - Collection framework improvements - Basic network handling - Java Threading, wait / notify, synchronization support - XSL changes implemented for JVM and DEX backend - Change long to int64_t ...etc. The main purpose of this patchset is to start the review and the merging process, and to avoid duplicate work if possible. The patchset is against r911. We are prepared to rebase it against the latest trunk, and also to split it up to smaller, self contained changes during the review process. There are still some features in our internal tree that are not ready to be submitted: - Some additions to the Android compatibility layer - AVAudioRecorder stubs - AudioRecorder and Player based on AudioQueue API -> allows to record into / play from streams -> required by J2ME API, also supports AMR record and playback - UIKit extensions to support J2ME Canvas port (needs some cleanup) Best Regards, Gergely -- Kis Gergely MattaKis Consulting Email: ger...@ma... Web: http://www.mattakis.com Phone: +36 70 408 1723 Fax: +36 27 998 622 |
From: Tor L. <tm...@ik...> - 2010-02-28 21:28:00
|
> I uploaded a pretty big patch with various ObjC - Java API improvements. Yay, great! I love your patch set already! I think xmlvm has great potential and it works very surprisingly nicely already. It's good that you made these patches public now, as I think most people starting to look into xmlvm to generate Objective-C from Java bytecode and will right away notice the same deficiencies as you did, and attempt fix them. (Well, at least I did... but not to the extent you have done.) Duplicating work is a waste of effort, and noticing one has wasted time on something that others have already solved in a better way makes a potential contributor frustrated... > We are prepared to rebase it against the latest trunk, and also to split it > up to smaller, self contained changes during the review process. Although I obviously can't speak for the xmlvm developers I think that is what one would expect, yes. I am just a newbie to xmlvm, just found it last week, and I am considering using it for some small scale spare time hacking ("hacking" in the non-evil sense) for the iPhone. (Not sure what if anything will come out of it and how much time I will have on it.) Anyway, looking at your patch set, I see several pointless diff chunks that just add or remove comments, like this: * For more information, visit the XMLVM Home Page at http://www.xmlvm.org */ +/** + * + * + */ + #import "xmlvm.h" Probably these should be dropped to make a review easier. Are these comments supposed to have some semantic meaning, as I see them added in several places. Here BTW is a patch for something I think you don't cover: native methods. Index: xmlvm/src/xmlvm2objc/xsl/xmlvm2objc.xsl =================================================================== --- xmlvm/src/xmlvm2objc/xsl/xmlvm2objc.xsl (revision 940) +++ xmlvm/src/xmlvm2objc/xsl/xmlvm2objc.xsl (working copy) @@ -374,10 +374,55 @@ </xsl:when> <xsl:when test="@isNative = 'true'"> <xsl:text>{ - NSException* ex = [[NSException alloc] initWithName:@"Native method not implemented" reason:nil userInfo:nil]; - @throw ex; -} + extern </xsl:text> + <xsl:call-template name="emitType"> + <xsl:with-param name="type" select="vm:signature/vm:return/@type"/> + </xsl:call-template> + <xsl:text> </xsl:text> + <xsl:value-of select="vm:fixname(../@package)"/> + <xsl:text>_</xsl:text> + <xsl:value-of select="../@name"/> + <xsl:text>_</xsl:text> + <xsl:call-template name="emitMethodName"> + <xsl:with-param name="name" select="@name"/> + <xsl:with-param name="class-type" select="concat(../@package, '.', ../@name)"/> + </xsl:call-template> + <xsl:call-template name="appendSignature"/> + <xsl:text> (</xsl:text> + <xsl:for-each select="vm:signature/vm:parameter"> + <xsl:call-template name="emitType"> + <xsl:with-param name="type" select="@type"/> + </xsl:call-template> + <xsl:if test="position() < count(../vm:parameter)"> + <xsl:text>, </xsl:text> + </xsl:if> + </xsl:for-each> + <xsl:text>); + </xsl:text> + <xsl:if test="vm:signature/vm:return/@type != 'void'"> + <xsl:text>return </xsl:text> + </xsl:if> + <xsl:value-of select="vm:fixname(../@package)"/> + <xsl:text>_</xsl:text> + <xsl:value-of select="../@name"/> + <xsl:text>_</xsl:text> + <xsl:call-template name="emitMethodName"> + <xsl:with-param name="name" select="@name"/> + <xsl:with-param name="class-type" select="concat(../@package, '.', ../@name)"/> + </xsl:call-template> + <xsl:call-template name="appendSignature"/> + <xsl:text> (</xsl:text> + <xsl:for-each select="vm:signature/vm:parameter"> + <xsl:text>n</xsl:text> + <xsl:value-of select="position()"/> + <xsl:if test="position() < count(../vm:parameter)"> + <xsl:text>, </xsl:text> + </xsl:if> + </xsl:for-each> + <xsl:text>); + } + </xsl:text> </xsl:when> <xsl:otherwise> With this patch, native method definitions in a Java class like: private static native int foo1(int a, float b); private static native void foo2(); get translated into: + (int) foo1___int_float :(int)n1 :(float)n2 { extern int fi_iki_tml_test1_foo1___int_float (int, float); return fi_iki_tml_test1_foo1___int_float (n1, n2); } + (void) foo2__ { extern void fi_iki_tml_test1_foo2__ (); fi_iki_tml_test1_foo2__ (); } I.e. a native method turns into an Objective-C stub that calls the actual extern function with "normal" C linkage. I think supporting native methods is essential to make it possible to actually use Java code for JDK compatibility classes (possibly even suitably licensed existing code), with just the minimal amount of code in Objective-C or plain C. --tml |
From: Panayotis K. <pan...@pa...> - 2010-03-01 07:43:32
|
>> I uploaded a pretty big patch with various ObjC - Java API >> improvements. > > Duplicating work is a waste of effort, and noticing one has wasted > time on something that others have already solved in a better way > makes a potential contributor frustrated... I totally agree. Although I understand that the reviewing is done solely on voluntary base, and time is sparse, there should be a solution to minimize duplications. I myself have also prepared (and sent) a similar (duplicate) patch, which makes things really frustrating :( |
From: Gergely K. <ger...@ma...> - 2010-02-28 22:03:36
|
Hi, 2010/2/28 Tor Lillqvist <tm...@ik...> > * For more information, visit the XMLVM Home Page at http://www.xmlvm.org > */ > +/** > + * > + * > + */ > + > #import "xmlvm.h" > > Probably these should be dropped to make a review easier. Are these > comments supposed to have some semantic meaning, as I see them added > in several places. > > Yes, there were several more of this in the internal tree, this is really just junk. I wanted to get the patch out there, so you can start looking at it, and some of these were left in by mistake. I will of course remove them from the next iteration. Best Regards, Gergely PS: I like your native method patch. -- Kis Gergely MattaKis Consulting Email: ger...@ma... Web: http://www.mattakis.com Phone: +36 70 408 1723 Fax: +36 27 998 622 |
From: Panayotis K. <pan...@pa...> - 2010-03-01 07:38:22
|
> Hi, > > 2010/2/28 Tor Lillqvist <tm...@ik...> > * For more information, visit the XMLVM Home Page at http://www.xmlvm.org > */ > +/** > + * > + * > + */ > + > #import "xmlvm.h" > > Probably these should be dropped to make a review easier. Are these > comments supposed to have some semantic meaning, as I see them added > in several places. > > Yes, there were several more of this in the internal tree, this is > really just junk. I wanted to get the patch out there, so you can > start looking at it, and some of these were left in by mistake. I > will of course remove them from the next iteration. > > Best Regards, > Gergely > I think the reason for this is some Windows \n\r taking place, which is removed by the patch ;) |