Skip to content

Conversation

@jonpryor
Copy link
Contributor

Context: https://bugzilla.xamarin.com/show_bug.cgi?id=45203
Context: https://bugzilla.xamarin.com/show_bug.cgi?id=44263#c8

(Sorry; private bugs.)

The getType() method isn't special; any Java library could contain
such a method:

// Java
public class Example {
    public int[] getType() {return null;}
}

(It's a wonder that we haven't hit such a thing before!)

Unfortunately, should such a method (1) exist, and (2) be bound as
GetType() instead of as a Type property -- in the above example,
we use an int[] return type because if a Java method has an array
return type, we won't turn it into a property -- the resulting code
wouldn't compile:

// error CS0019: Operator `==' cannot be applied to operands of type `int[]' and `System.Type'
if (GetType () == ThresholdType) ...

To fix this, qualify all use of the GetType() method so that we
explicitly use System.Object.GetType():

if ((object) this).GetType () == ThresholdType) ...

This has no performance-impact, IL-wise, as the C# compiler is able to
statically determine that we want non-virtual invocation of the
System.Object.GetType() method. No runtime cast is performed.

@atsushieno
Copy link
Contributor

No, we have been seeing this quite a lot. It should be renamed by metadata fixup.

@atsushieno
Copy link
Contributor

By dealing this as a "supported" form, any future changes to generator will have to be taken care very carefully to not cause this kind of issue which prevents productive developments.

An alternative approach is to detect "getType()" and raise an error.

Context: https://bugzilla.xamarin.com/show_bug.cgi?id=45203
Context: https://bugzilla.xamarin.com/show_bug.cgi?id=44263#c8

(Sorry; private bugs.)

The `getType()` method isn't special; any Java library could contain
such a method:

	// Java
	public class Example {
		public int[] getType() {return null;}
	}

(It's a wonder that we haven't hit such a thing before!)

Unfortunately, should such a method (1) exist, and (2) be bound as
`GetType()` instead of as a `Type` property -- in the above example,
we use an `int[]` return type because if a Java method has an array
return type, we won't turn it into a property -- the resulting code
wouldn't compile:

	// error CS0019: Operator `==' cannot be applied to operands of type `int[]' and `System.Type'
	if (GetType () == ThresholdType) ...

To fix this, *qualify* all use of the `GetType()` method so that we
explicitly use `System.Object.GetType()`:

	if ((object) this).GetType () == ThresholdType) ...

This has no performance-impact, IL-wise, as the C# compiler is able to
statically determine that we want non-virtual invocation of the
`System.Object.GetType()` method. No runtime cast is performed.
@jonpryor jonpryor force-pushed the jonp-generator-GetType branch from 3e6f401 to a3cf866 Compare October 12, 2016 15:06
@jonpryor
Copy link
Contributor Author

No, we have been seeing this quite a lot.

That suggests that we should do this fix.

It should be renamed by metadata fixup.

Why?

One of the (unobtainable?) goals for generator is that it always work: we give it a .jar file, and we get an assembly, no user intervention required. The API might not be good, the naming might be terrible, but ideally, we'd at least generate something, without requiring that the developer deal with a million errors.

We're clearly far from this goal. Checking for getType() and erroring would get us further from this goal.

By dealing this as a "supported" form, any future changes to generator will have to be taken care very carefully to not cause this kind of issue which prevents productive developments.

Once a test is in place -- and this PR includes a test -- I don't see why we would need to be "very careful" when using GetType() in the future. Additionally, the "alternative" to GetType() is trivial -- ((object) this).GetType().

@atsushieno
Copy link
Contributor

Okay okay, I'm not going to take care of any bugs that is related to this issue, so this can go in.

@atsushieno atsushieno merged commit 572b119 into dotnet:master Oct 12, 2016
@github-actions github-actions bot locked and limited conversation to collaborators Apr 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants