[patch] let gjavah accept -source 1.[567]

[patch] let gjavah accept -source 1.[567]

[Matthias Klose]

[-- Attachment #1: Type: text/plain, Size: 156 bytes --]

Currently gjavah only accepts -source 1.4 and lower, and errors out for any
other value. Would it be reasonable to accept higher versions too?

  Matthias


[-- Attachment #2: gjdoc.diff --]
[-- Type: text/x-diff, Size: 1009 bytes --]

Index: classpath/tools/gnu/classpath/tools/gjdoc/Main.java
===================================================================
--- classpath/tools/gnu/classpath/tools/gjdoc/Main.java	(Revision 194604)
+++ classpath/tools/gnu/classpath/tools/gjdoc/Main.java	(Arbeitskopie)
@@ -1339,10 +1310,13 @@
             option_source = args[0];
             if (!"1.2".equals(option_source)
                 && !"1.3".equals(option_source)
-                && !"1.4".equals(option_source)) {
+                && !"1.4".equals(option_source)
+                && !"1.5".equals(option_source)
+                && !"1.6".equals(option_source)
+                && !"1.7".equals(option_source)) {
 
               throw new RuntimeException("Only he following values are currently"
-                                         + " supported for option -source: 1.2, 1.3, 1.4.");
+                                         + " supported for option -source: 1.2, 1.3, 1.4, 1.5, 1.6, 1.7.");
             }
           }
         });

Re: [cp-patches] [patch] let gjavah accept -source 1.[567]

[Andrew Hughes]

----- Original Message -----
> Currently gjavah only accepts -source 1.4 and lower, and errors out
> for any
> other value. Would it be reasonable to accept higher versions too?
> 
>   Matthias
> 
> 

Does source even matter to gjavah?  What do higher versions require?
-- 
Andrew :)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: 248BDC07 (https://keys.indymedia.org/)
Fingerprint = EC5A 1F5E C0AD 1D15 8F1F  8F91 3B96 A578 248B DC07

Re: [cp-patches] [patch] let gjavah accept -source 1.[567]

[Mark Wielaard]

On Wed, Dec 19, 2012 at 02:01:10PM +0100, Matthias Klose wrote:
> Currently gjavah only accepts -source 1.4 and lower, and errors out for any
> other value. Would it be reasonable to accept higher versions too?

I think that should be fine for gjavah, I cannot think of something
in the bytecode that would impact jni/cni header generation.

But your patch is for gjdoc. There I think there are source constructs
that might be a problem in newer versions. It should support some of
the new 1.5 source level features, but I am not sure if it handles
everything nor whether it handles any 1.6 and 1.7 extensions.

> Index: classpath/tools/gnu/classpath/tools/gjdoc/Main.java
> ===================================================================
> --- classpath/tools/gnu/classpath/tools/gjdoc/Main.java	(Revision 194604)
> +++ classpath/tools/gnu/classpath/tools/gjdoc/Main.java	(Arbeitskopie)
> @@ -1339,10 +1310,13 @@
>              option_source = args[0];
>              if (!"1.2".equals(option_source)
>                  && !"1.3".equals(option_source)
> -                && !"1.4".equals(option_source)) {
> +                && !"1.4".equals(option_source)
> +                && !"1.5".equals(option_source)
> +                && !"1.6".equals(option_source)
> +                && !"1.7".equals(option_source)) {

If you really meant gjdoc I think it would be OK to try to accept it,
but maybe with a warning message that it is untested?

Cheers,

Mark

Re: [cp-patches] [patch] let gjavah accept -source 1.[567]

[Andrew Hughes]

----- Original Message -----
> On Wed, Dec 19, 2012 at 02:01:10PM +0100, Matthias Klose wrote:
> > Currently gjavah only accepts -source 1.4 and lower, and errors out
> > for any
> > other value. Would it be reasonable to accept higher versions too?
> 
> I think that should be fine for gjavah, I cannot think of something
> in the bytecode that would impact jni/cni header generation.
> 
> But your patch is for gjdoc. There I think there are source
> constructs
> that might be a problem in newer versions. It should support some of
> the new 1.5 source level features, but I am not sure if it handles
> everything nor whether it handles any 1.6 and 1.7 extensions.
> 
> > Index: classpath/tools/gnu/classpath/tools/gjdoc/Main.java
> > ===================================================================
> > --- classpath/tools/gnu/classpath/tools/gjdoc/Main.java	(Revision
> > 194604)
> > +++ classpath/tools/gnu/classpath/tools/gjdoc/Main.java
> > 	(Arbeitskopie)
> > @@ -1339,10 +1310,13 @@
> >              option_source = args[0];
> >              if (!"1.2".equals(option_source)
> >                  && !"1.3".equals(option_source)
> > -                && !"1.4".equals(option_source)) {
> > +                && !"1.4".equals(option_source)
> > +                && !"1.5".equals(option_source)
> > +                && !"1.6".equals(option_source)
> > +                && !"1.7".equals(option_source)) {
> 
> If you really meant gjdoc I think it would be OK to try to accept it,
> but maybe with a warning message that it is untested?
> 
> Cheers,
> 
> Mark
> 
> 
> 

The question would make a lot more sense for gjdoc than gjavah :-)

As is, it does support 1.5 to a degree (enough to compile the Classpath
docs anyway; we should try OpenJDK at some point).  There are no source
changes in 1.6 and minimal ones in 1.7 so these should be easy to support,
if they aren't already.

I'm generally in favour of removing unnecessary version checks as they
only mask the real bugs (e.g. when the parser hits a construct it can't handle)
and would have to be allowed to actually implement the newer versions anyway.
I think this is more a case of neglected code (given there were 1.5 updates)
rather than a design choice.

Note that I have a JAPI run of gjdoc vs. javadoc's API which points out some
of the missing API (mainly, if not all, 1.5 stuff AFAICS) and I intend to look
at implementing this soon.

Thanks,
-- 
Andrew :)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: 248BDC07 (https://keys.indymedia.org/)
Fingerprint = EC5A 1F5E C0AD 1D15 8F1F  8F91 3B96 A578 248B DC07

Re: [cp-patches] [patch] let gjavah accept -source 1.[567]

[Matthias Klose]

[-- Attachment #1: Type: text/plain, Size: 1666 bytes --]

Am 19.12.2012 18:37, schrieb Mark Wielaard:
> On Wed, Dec 19, 2012 at 02:01:10PM +0100, Matthias Klose wrote:
>> Currently gjavah only accepts -source 1.4 and lower, and errors out for any
>> other value. Would it be reasonable to accept higher versions too?
> 
> I think that should be fine for gjavah, I cannot think of something
> in the bytecode that would impact jni/cni header generation.
> 
> But your patch is for gjdoc. There I think there are source constructs
> that might be a problem in newer versions. It should support some of
> the new 1.5 source level features, but I am not sure if it handles
> everything nor whether it handles any 1.6 and 1.7 extensions.
> 
>> Index: classpath/tools/gnu/classpath/tools/gjdoc/Main.java
>> ===================================================================
>> --- classpath/tools/gnu/classpath/tools/gjdoc/Main.java	(Revision 194604)
>> +++ classpath/tools/gnu/classpath/tools/gjdoc/Main.java	(Arbeitskopie)
>> @@ -1339,10 +1310,13 @@
>>              option_source = args[0];
>>              if (!"1.2".equals(option_source)
>>                  && !"1.3".equals(option_source)
>> -                && !"1.4".equals(option_source)) {
>> +                && !"1.4".equals(option_source)
>> +                && !"1.5".equals(option_source)
>> +                && !"1.6".equals(option_source)
>> +                && !"1.7".equals(option_source)) {
> 
> If you really meant gjdoc I think it would be OK to try to accept it,
> but maybe with a warning message that it is untested?

yes, I meant gjdoc. Here is an updated patch.

  Matthias

	* tools/gnu/classpath/tools/gjdoc/Main.java: Accept -source 1.5, 1.6, 1.7.



[-- Attachment #2: libjava-gjdoc.diff --]
[-- Type: text/x-diff, Size: 1215 bytes --]

# DP: Let gjdoc accept -source 1.5|1.6|1.7. Addresses: #678945.

--- a/src/libjava/classpath/tools/gnu/classpath/tools/gjdoc/Main.java
+++ b/src/libjava/classpath/tools/gnu/classpath/tools/gjdoc/Main.java
@@ -1337,12 +1337,17 @@
           void process(String[] args)
           {
             option_source = args[0];
-            if (!"1.2".equals(option_source)
+            if ("1.5".equals(option_source)
+                || "1.6".equals(option_source)
+                || "1.7".equals(option_source)) {
+              System.err.println("WARNING: support for option -source " + option_source + " is experimental");
+            }
+            else if (!"1.2".equals(option_source)
                 && !"1.3".equals(option_source)
                 && !"1.4".equals(option_source)) {
 
-              throw new RuntimeException("Only he following values are currently"
-                                         + " supported for option -source: 1.2, 1.3, 1.4.");
+              throw new RuntimeException("Only the following values are currently"
+                                         + " supported for option -source: 1.2, 1.3, 1.4; experimental: 1.5, 1.6, 1.7.");
             }
           }
         });

Re: [cp-patches] [patch] let gjavah accept -source 1.[567]

[Mark Wielaard]

On Sun, 2013-01-06 at 18:00 +0100, Matthias Klose wrote:
> Am 19.12.2012 18:37, schrieb Mark Wielaard:
> > If you really meant gjdoc I think it would be OK to try to accept it,
> > but maybe with a warning message that it is untested?
> 
> yes, I meant gjdoc. Here is an updated patch.
> 
>   Matthias
> 
> 	* tools/gnu/classpath/tools/gjdoc/Main.java: Accept -source 1.5, 1.6, 1.7.

That looks reasonable to me.

Thanks,

Mark

Re: [cp-patches] [patch] let gjavah accept -source 1.[567]

[Mark Wielaard]

On Mon, 2013-01-07 at 09:54 +0100, Mark Wielaard wrote:
> On Sun, 2013-01-06 at 18:00 +0100, Matthias Klose wrote:
> > Am 19.12.2012 18:37, schrieb Mark Wielaard:
> > > If you really meant gjdoc I think it would be OK to try to accept it,
> > > but maybe with a warning message that it is untested?
> > 
> > yes, I meant gjdoc. Here is an updated patch.
> > 
> >   Matthias
> > 
> > 	* tools/gnu/classpath/tools/gjdoc/Main.java: Accept -source 1.5, 1.6, 1.7.
> 
> That looks reasonable to me.

As requested on irc, I have pushed this patch to the git repo for
Matthias.

Cheers,

Mark

Re: [cp-patches] [patch] let gjavah accept -source 1.[567]

[Andrew Hughes]

----- Original Message -----
> On Mon, 2013-01-07 at 09:54 +0100, Mark Wielaard wrote:
> > On Sun, 2013-01-06 at 18:00 +0100, Matthias Klose wrote:
> > > Am 19.12.2012 18:37, schrieb Mark Wielaard:
> > > > If you really meant gjdoc I think it would be OK to try to
> > > > accept it,
> > > > but maybe with a warning message that it is untested?
> > > 
> > > yes, I meant gjdoc. Here is an updated patch.
> > > 
> > >   Matthias
> > > 
> > > 	* tools/gnu/classpath/tools/gjdoc/Main.java: Accept -source 1.5,
> > > 	1.6, 1.7.
> > 
> > That looks reasonable to me.
> 
> As requested on irc, I have pushed this patch to the git repo for
> Matthias.
> 

Thanks.  Beat me to it :)

> Cheers,
> 
> Mark
> 
> 
> 

-- 
Andrew :)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: 248BDC07 (https://keys.indymedia.org/)
Fingerprint = EC5A 1F5E C0AD 1D15 8F1F  8F91 3B96 A578 248B DC07