Bug 326512

Comment 1 Frederic Fusier 2010-09-29 07:44:20 EDT

Created attachment 179835 [details]
Proposed patch

I have also fixed other compiler warnings (deprecated, write access to enclosing field) and also some builder warnings...

Comment 2 Frederic Fusier 2010-09-29 07:47:08 EDT

Dani, I'm not granted to write changes into org.eclipse.test project :-(
Could you please release the patch for me?
TIA

Comment 3 Dani Megert 2010-09-29 08:29:45 EDT

Created attachment 179837 [details]
Improved fix

Frédéric,

I think
test= (Test)suiteMethod.invoke(null, (Object)new Class[0]); // static method
must be
test= (Test)suiteMethod.invoke(null, (Object[])new Class[0]); // static method

I fixed that and also updated the bundle version.

Comment 4 Dani Megert 2010-09-29 08:31:27 EDT

Strange: I have commit rights to perf and also releng.tools but not to org.eclipse.test.

Kim, please commit the patch.

Comment 5 Frederic Fusier 2010-09-29 08:58:17 EDT

(In reply to comment #3)
> Created an attachment (id=179837) [details]
> Improved fix
> 
> Frédéric,
> 
> I think
> test= (Test)suiteMethod.invoke(null, (Object)new Class[0]); // static method
> must be
> test= (Test)suiteMethod.invoke(null, (Object[])new Class[0]); // static method
> 
> I fixed that and also updated the bundle version.

I do not think it makes any difference for the compiler as this a varargs  parameter. But I agree this is a more accurate cast :-)

Comment 6 Dani Megert 2010-09-29 09:00:37 EDT

> I do not think it makes any difference for the compiler as this a varargs 
> parameter.
I think it does: in one case it passes no arguments while in the other it passes 1 argument which is the empty array.

Comment 7 Kim Moir 2010-09-29 09:06:51 EDT

patch released

Comment 8 Dani Megert 2010-09-30 04:28:12 EDT

The compile warnings are gone in N20100929-2000. Hopefully, these changes aren't responsible that all tests DNFed.(In reply to comment #6)

> > I do not think it makes any difference for the compiler as this a varargs 
> > parameter.
> I think it does: in one case it passes no arguments while in the other it
> passes 1 argument which is the empty array.
Frédéric, do you agree?

Comment 9 Frederic Fusier 2010-09-30 04:50:47 EDT

(In reply to comment #8)
>(In reply to comment #6)
> 
> > > I do not think it makes any difference for the compiler as this a varargs 
> > > parameter.
> > I think it does: in one case it passes no arguments while in the other it
> > passes 1 argument which is the empty array.
> Frédéric, do you agree?

No, but I'm not a specialist of 1.5 syntax... IMO, casting either with (Object) or (Object[]) does not change the behavior at runtime. I'm pretty sure that it should be an empty array which is passed as an argument when calling the method in both cases...

An empty array of Class can be considered as an array of Object or as an Object. It would just matter for the compiler if it referenced this object afterward, but as it is not the case I still think both cast can be used. Note that my assumption is confirmed by what the warning message is suggesting when the cast is not present...

Anyway, as I said, I'm also happy by casting with (Object[]) which sounds more appropriate for an empty array of Class.

Comment 10 Dani Megert 2010-09-30 04:53:28 EDT

> No, but I'm not a specialist of 1.5 syntax... IMO, casting either with (Object)
> or (Object[]) does not change the behavior at runtime.
Well, it does ;-). Try this:


public class Test {

	/**
	 * @param args
	 */
	public static void main(String[] args) {
		foo("1", new String[0]);
		foo("2", (Object)new String[0]);
		foo("3", (Object[])new String[0]);
		foo("4", new String[] {"1", "2"});
		foo("5", (Object)new String[] {"1", "2"});
		foo("6", (Object[])new String[] {"1", "2"});

	}

	static void foo(Object s, Object... args) {
		System.out.print(s + ": ");
		for (int i = 0; i < args.length; i++) {
			System.out.print(args[i]);
			System.out.print(" ");
		}
		System.out.println();
	}
}

Comment 11 Frederic Fusier 2010-09-30 06:27:30 EDT

(In reply to comment #10)
Indeed! Thanks for the tip :-)

Comment 12 Kim Moir 2010-09-30 09:00:47 EDT

The test results are missing because the build scripts refer to a specific version of org.eclipse.test and the version changed in the manifest with the patches for this bug. This is very brittle and I will change this today.

Comment 13 Kim Moir 2010-09-30 10:18:50 EDT

Created attachment 179957 [details]
patch

patch for build scripts to load org.eclipse.test version from property file during test run

Comment 14 Kim Moir 2010-09-30 13:27:29 EDT

verified that the tests run with the patch in a test build I ran this morning.