Re: RFR: 7044282: (reflect) Class.forName and Array.newInstance are inconsistent regarding multidimensional arrays

[Joel Borggrén-Franck]

Thanks for the reviews, fix has been pushed.

cheers
/Joel

On 2013-10-10, Joe Darcy wrote:
 Looks fine; thanks,
 
 -Joe
 
 On 10/10/2013 5:56 AM, Paul Sandoz wrote:
 On Oct 10, 2013, at 2:46 PM, Joel Borggren-Franck joel.fra...@oracle.com 
 wrote:
 
 Hi,
 
 Joe, Paul, agreed the test could be better. Improved it (without using
 streams) and also added a bug id tag to the old Arrays.newInstance test.
 
 Thanks for the comments.
 
 Webrev here: http://cr.openjdk.java.net/~jfranck/7044282/webrev.01/
 
 Looks OK.
 
 Paul.
 

Re: RFR: 7044282: (reflect) Class.forName and Array.newInstance are inconsistent regarding multidimensional arrays

[Paul Sandoz]

On Oct 9, 2013, at 8:33 PM, Joel Borggren-Franck joel.fra...@oracle.com wrote:

 Hi
 
 Please review this spec update and test for getting array classes and
 instances of more dimensions than the class file can express or the VM
 can handle.
 
 Array.newInstance have a test for arrays of more dimensions than 255,
 this patch adds a test for Class.forName as well.
 
 Also the javadoc for Array.newInstance are clarified.
 
 Bug: https://bugs.openjdk.java.net/browse/JDK-7044282
 Webrev: http://cr.openjdk.java.net/~jfranck/7044282/webrev.00/
 

Looks OK, i agree with Joe on the test refactoring.

FYI (because i have streams on my brain...).

String brackets254 = Stream.generate(() - [).limit(254).collect(joining());

or:

String name254 = Stream.generate(() - [).limit(254).collect(joining(, , 
Ljava.lang.String;));

or:

IntFunctionString nameN = (int n) -  Stream.generate(() - 
[).limit(n).collect(joining(, , Ljava.lang.String;));
String name254 = nameN.apply(254);
String name254 = nameN.apply(255);
String name254 = nameN.apply(256);
String name1 = nameN.apply(1);
String bigName = nameN.apply(Short.MAX_VALUE + 20);

Paul.

 cheers
 /Joel

Re: RFR: 7044282: (reflect) Class.forName and Array.newInstance are inconsistent regarding multidimensional arrays

[Joel Borggren-Franck]

Hi,

Joe, Paul, agreed the test could be better. Improved it (without using
streams) and also added a bug id tag to the old Arrays.newInstance test.

Thanks for the comments.

Webrev here: http://cr.openjdk.java.net/~jfranck/7044282/webrev.01/

cheres
/Joel

On 2013-10-09, Joel Borggren-Franck wrote:
 Hi
 
 Please review this spec update and test for getting array classes and
 instances of more dimensions than the class file can express or the VM
 can handle.
 
 Array.newInstance have a test for arrays of more dimensions than 255,
 this patch adds a test for Class.forName as well.
 
 Also the javadoc for Array.newInstance are clarified.
 
 Bug: https://bugs.openjdk.java.net/browse/JDK-7044282
 Webrev: http://cr.openjdk.java.net/~jfranck/7044282/webrev.00/
 
 cheers
 /Joel

Re: RFR: 7044282: (reflect) Class.forName and Array.newInstance are inconsistent regarding multidimensional arrays

[Paul Sandoz]

On Oct 10, 2013, at 2:46 PM, Joel Borggren-Franck joel.fra...@oracle.com 
wrote:

 Hi,
 
 Joe, Paul, agreed the test could be better. Improved it (without using
 streams) and also added a bug id tag to the old Arrays.newInstance test.
 
 Thanks for the comments.
 
 Webrev here: http://cr.openjdk.java.net/~jfranck/7044282/webrev.01/
 

Looks OK.

Paul.

Re: RFR: 7044282: (reflect) Class.forName and Array.newInstance are inconsistent regarding multidimensional arrays

[Joe Darcy]

Looks fine; thanks,

-Joe

On 10/10/2013 5:56 AM, Paul Sandoz wrote:

On Oct 10, 2013, at 2:46 PM, Joel Borggren-Franck joel.fra...@oracle.com 
wrote:


Hi,

Joe, Paul, agreed the test could be better. Improved it (without using
streams) and also added a bug id tag to the old Arrays.newInstance test.

Thanks for the comments.

Webrev here: http://cr.openjdk.java.net/~jfranck/7044282/webrev.01/


Looks OK.

Paul.

Re: RFR: 7044282: (reflect) Class.forName and Array.newInstance are inconsistent regarding multidimensional arrays

[Joseph Darcy]

Hi Joel,

The code changes look fine, but I'd like to see some refactoring to the 
tests. In particular, please put the logic in


  81 try {
  82 Class? c256 = Class.forName(name256);
  83 error++;
  84 System.err.println(ERROR: could create  + c256);
  85 } catch (ClassNotFoundException e) {
  86 ;// ok
  87 }

into a method that can be called like failingForName(name, clazz) (or 
whatever is appropriate).


Thanks,

-Joe

On 10/9/2013 11:33 AM, Joel Borggren-Franck wrote:

Hi

Please review this spec update and test for getting array classes and
instances of more dimensions than the class file can express or the VM
can handle.

Array.newInstance have a test for arrays of more dimensions than 255,
this patch adds a test for Class.forName as well.

Also the javadoc for Array.newInstance are clarified.

Bug: https://bugs.openjdk.java.net/browse/JDK-7044282
Webrev: http://cr.openjdk.java.net/~jfranck/7044282/webrev.00/

cheers
/Joel