jdk
jdk copied to clipboard
openjdk
8306039: ParameterizedType.getOwnerType() documentation is incomplete about null result
Clarify that ParameterizedType.getOwnerType() always return null in a few scenarios.
Progress
- [ ] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
- [x] Change must not contain extraneous whitespace
- [x] Commit message must refer to an issue
- [ ] Change requires CSR request JDK-8335467 to be approved
Issues
- JDK-8306039: ParameterizedType.getOwnerType() documentation is incomplete about null result (Bug - P4)
- JDK-8335467: ParameterizedType.getOwnerType() documentation is incomplete about null result (CSR)
Reviewing
Using git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/19977/head:pull/19977
$ git checkout pull/19977
Update a local copy of the PR:
$ git checkout pull/19977
$ git pull https://git.openjdk.org/jdk.git pull/19977/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 19977
View PR using the GUI difftool:
$ git pr show -t 19977
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/19977.diff
Webrev
:wave: Welcome back liach! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.
![bridgekeeper[bot] avatar](https://avatars.githubusercontent.com/u/41768318?v=4 "Published by a pub.dev verified publisher"){kind=small}
❗ This change is not yet ready to be integrated. See the Progress checklist in the description for automated requirements.
@liach The following label will be automatically applied to this pull request:
core-libs
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.
Webrevs
- 09: Full - Incremental (712d3ec5)
- 08: Full - Incremental (c237c2ad)
- 07: Full - Incremental (4f4d9b91)
- 06: Full - Incremental (1306153e)
- 05: Full - Incremental (eb0e2e3c)
- 04: Full - Incremental (35abd93c)
- 03: Full - Incremental (83feba99)
- 02: Full - Incremental (090eee17)
- 01: Full - Incremental (2d14b047)
- 00: Full (91f343b3)
@liach This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
@liach This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
@liach This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.
@liach This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
/label add compiler
cc'ing this to the javac list: this brings in a few definitions from the JLS to core reflection, for which the compiler developers know better.
Mailing list message from Alex Buckley on compiler-dev:
Hi Chen,
On 10/30/2024 11:51 AM, Chen Liang wrote:
2. Specify the underlying type for different AnnotatedType subinterfaces
The new "Interface Hierarchy" starts with: "Annotated use of types in the Java programming language is modeled with these subinterfaces".
Only _annotated_ uses? Even though AnnotatedType generally speaks about the *potentially* annotated use of a type" ?
I think your best bet when introducing the interface hierarchy is to explain that some types are rich in structure, and each part of the structure is itself a type that can potentially be annotated. For example, the parameterized type `Collection<? extends Number>` actually denotes three uses of types that can potentially be annotated individually: (1) the parameterized type as a whole (`@Foo Collection<? extends Number>`); (2) the wildcard type argument (`Collection<@Bar ? extends Number>`); and (3) the wildcard type bound (`Collection<? extends @Quux Number>`). The AnnotatedType interface grants access to the potentially annotated use of a type as a whole (`@Foo Collection<..>`). If that type is rich in structure, subinterfaces of AnnotatedType grant additional access to the potentially annotated uses of types in parts of that structure. [Note I have not said "structural type" at any point.]
With this in mind, you can see that e.g. AnnotatedParameterizedType doesn't model [the potentially annotated use of] parameterized types as such. AnnotatedType already models that: I can call getAnnotations() to get the @Foo in `@Foo Collection<..>`. What AnnotatedParameterizedType models is the potentially annotated use of types _serving as type arguments of a parameterized type_.
Similarly, AnnotatedWildcardType doesn't model wildcard type arguments as such -- you get the @Bar in `Collection<@Bar ? extends Number>` from the aforementioned AnnotatedParameterizedType, regardless of whether the type argument is a wildcard or not. What AnnotatedWildcardType models is the potentially annotated use of types _serving as upper or lower bounds of a wildcard type argument_. The @Quux in `<@Bar ? extends @Quux Number>`.
I recommend clarifying the interface hierarchy along those lines. Make liberal use of the same example type throughout, to demonstrate which part is which, and which use-of-a-type is which.
As for getType: before any interface hierarchy, you should explain what the underlying type of the annotated-type-as-a-whole is. An AnnotatedType that represents `@Foo int` yields the Class object for int as the underlying type. An AnnotatedType that represents `@Foo Collection<@Bar ? extends @Quux Number>` yields `Collection<? extends Number>` as the underlying type. By saying all this first, you can streamline the interface hierarchy to avoid mentioning getType/underlying types.
3. Define "inner member class" for `getOwnerType`, and refer to it in `AnnotatedType::getAnnotatedOwnerType`.
This definition and use are very nice to see.
Alex
Alex, thanks for your review!
Please review the APIdiff for the updated version at https://cr.openjdk.org/~liach/8343251-apidiff/java.base/java/lang/reflect/package-summary.html which includes the HTML rendering as well.
Notably, I gave up the interface hierarchy section and replaced it with a Java type or type argument to modeling interface mapping. This is more natural from the Java programmer point of view, and makes the documentation more concise. I have also included relevant examples, as you have recommended. The terms for AnnotatedXxType are all updated to be "use" instead of "type", and I distinguished type versus type argument as well.
Mailing list message from Alex Buckley on compiler-dev:
On 10/31/2024 12:40 PM, Chen Liang wrote:
Please review the APIdiff for the updated version at https://cr.openjdk.org/~liach/8343251-apidiff/java.base/java/lang/reflect/package-summary.html which includes the HTML rendering as well.
Notably, I gave up the interface hierarchy section and replaced it with a Java type or type argument to modeling interface mapping. This is more natural from the Java programmer point of view, and makes the documentation more concise. I have also included relevant examples, as you have recommended. The terms for AnnotatedXxType are all updated to be "use" instead of "type", and I distinguished type versus type argument as well.
Type has become difficult to read. Please lighten up the introductory paragraph. To do that, I recommend unifying the kinds of types/type arguments (primitive types, reference types, raw types, wildcard type arguments ...) with the mapping from types/type arguments to the interface hierarchy. One list, giving the kind of type + an example + the appropriate interface or subinterface.
Do the same in AnnotatedType. Also, drop the introduction of "current runtime" -- this is about the use of types/type arguments in programs.
Alex
Thanks for the recommendations. I have moved the trees into a table; it should be much more straightforward from the HTML documentation.
@liach This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
@liach This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.
/open
This attracts fewer reviews because the APIs are already widely used, but these improvements still clarify a lot of confusions and establish clear relations from the API models to the JLS.
I have updated the CR preview and did some minor updates; please review again.
@liach This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
@liach This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
@liach This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.