Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[java-source-utils] Properly represent unresolved types #758

Merged
merged 1 commit into from
Dec 10, 2020

Conversation

jonpryor
Copy link
Member

@jonpryor jonpryor commented Dec 10, 2020

Context: #687 (comment)

Commit 69e1b80 stated:

We don't want to require that everything be resolvable -- it's painful, and
possibly impossible, e.g. w/ internal types -- so instead we should "demark"
the unresolvable types.

.params.txt output will use .* as a type prefix, e.g.

method(.*UnresolvableType foo, int bar);

docs.xml will output L.*UnresolvableType;.

…except this didn't actually happen:
java-source.utils.jar --output-javadoc didn't have an L prefix
and ; suffix for the JNI type representation. Additionally, it
would include generic type parameters!

Thus, a declaration such as:

static UnknownType<String, Integer> method(AnotherUnknownType<Class> p);

would result in:

<method name="method" jni-return=".*UnknownType&lt;String, Integer&gt;" jni-signature="(.*AnotherUnknownType&lt;Class&gt;).*UnkonwnType&lt;String, Integer&gt;">

which was not at all intended, and causes subsequent issued in PR #687
as the JNI signature parser would break when encountering ..

Update java-source-utils.jar & //*/@jni-signature so that
unresolved types are properly represented: //*/@jni-type and
//*/@jni-signature shouldn't contain generic type parameters, and
should also (somewhat) "conform" to JNI convention, with a leading L
and trailing ;, emitting:

<method name="method" jni-return="L.*UnknownType;" jni-signature="(L.*AnotherUnknownType;)L.*UnknownType;">

Additionally, update JavaType.java to cause it to have "orphaned"
Javadoc comments; this change was forgotten in c1bc5c8.

Context: dotnet#687 (comment)

Commit 69e1b80 stated:

> We don't want to *require* that everything be resolvable -- it's painful, and
> possibly impossible, e.g. w/ internal types -- so instead we should "demark"
> the unresolvable types.
>
> `.params.txt` output will use `.*` as a type prefix, e.g.
>
> 	method(.*UnresolvableType foo, int bar);
>
> `docs.xml` will output `L.*UnresolvableType;`.

…except this didn't actually happen:
`java-source.utils.jar --output-javadoc` didn't have an `L` prefix
and `;` suffix for the JNI type representation.  Additionally, it
would include generic type parameters!

Thus, a declaration such as:

	static UnknownType<String, Integer> method(AnotherUnknownType<Class> p);

would result in:

	<method name="method" jni-return=".*UnknownType&lt;String, Integer&gt;" jni-signature="(.*AnotherUnknownType&lt;Class&gt;).*UnkonwnType&lt;String, Integer&gt;">

which was not at all intended, and causes subsequent issued in PR dotnet#687
as the JNI signature parser would break when encountering `.`.

Update `java-source-utils.jar` & `//*/@jni-signature` so that
[unresolved types are properly represented][0]: `//*/@jni-type` and
`//*/@jni-signature` shouldn't contain generic type parameters, and
should also (somewhat) "conform" to JNI convention, with a leading `L`
and trailing `;`, emitting:

	<method name="method" jni-return="L.*UnknownType;" jni-signature="(L.*AnotherUnknownType;)L.*UnknownType;">

Additionally, update `JavaType.java` to cause it to have "orphaned"
Javadoc comments; this change was forgotten in c1bc5c8.

[0]: dotnet#687 (comment)
@jonpryor jonpryor merged commit 7c1ad51 into dotnet:master Dec 10, 2020
@jonpryor jonpryor deleted the jonp-jsu-unresolved-types branch December 10, 2020 21:51
@jpobst jpobst added this to the 11.1 (16.9 / 8.9) milestone Jan 5, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 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.

2 participants