JDK 13 RFR of JDK-8224687: Add clarifying overrides of Element.asType to more specific subinterfaces
Joe Darcy
joe.darcy at oracle.com
Thu May 30 21:22:34 UTC 2019
Updated webrev:
webrev: http://cr.openjdk.java.net/~darcy/8224687.3/
More descriptive comments in the test, added more specific @see tags to
the overridden methods (in part of suppress unwanted propagation of the
all the @see entries from Element.asType), and gave guidance on the
range of types for a variable.
Diff of patches below, trimmed out some white space differences.
Thanks,
-Joe
27,29c29,40
< ---
old/src/java.compiler/share/classes/javax/lang/model/element/ExecutableElement.java
2019-05-29 17:14:43.047376000 -0700
< +++
new/src/java.compiler/share/classes/javax/lang/model/element/ExecutableElement.java
2019-05-29 17:14:42.667376000 -0700
< @@ -41,6 +41,15 @@
---
> TypeMirror asType();
>
> ---
old/src/java.compiler/share/classes/javax/lang/model/element/ExecutableElement.java
2019-05-30 14:15:59.625000999 -0700
> +++
new/src/java.compiler/share/classes/javax/lang/model/element/ExecutableElement.java
2019-05-30 14:15:59.261000999 -0700
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2005, 2013, Oracle and/or its affiliates. All
rights reserved.
> + * Copyright (c) 2005, 2019, Oracle and/or its affiliates. All
rights reserved.
> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> *
> * This code is free software; you can redistribute it and/or modify it
> @@ -41,6 +41,17 @@
36a48,49
> + *
> + * @see ExecutableType
45,46c58,59
< ---
old/src/java.compiler/share/classes/javax/lang/model/element/ModuleElement.java
2019-05-29 17:14:43.735376000 -0700
< +++
new/src/java.compiler/share/classes/javax/lang/model/element/ModuleElement.java
2019-05-29 17:14:43.375376000 -0700
---
> ---
old/src/java.compiler/share/classes/javax/lang/model/element/ModuleElement.java
2019-05-30 14:16:00.341000999 -0700
> +++
new/src/java.compiler/share/classes/javax/lang/model/element/ModuleElement.java
2019-05-30 14:15:59.957000999 -0700
55c68
< @@ -37,6 +38,12 @@
---
> @@ -37,6 +38,16 @@
60c73,74
< + * Returns a {@linkplain javax.lang.model.type.NoType
pseudo-type} for this module.
---
> + * Returns a {@linkplain javax.lang.model.type.NoType pseudo-type}
> + * for this module.
61a76,78
> + *
> + * @see javax.lang.model.type.NoType
> + * @see javax.lang.model.type.TypeKind#MODULE
68,69c85,93
< ---
old/src/java.compiler/share/classes/javax/lang/model/element/PackageElement.java
2019-05-29 17:14:44.439376000 -0700
< +++
new/src/java.compiler/share/classes/javax/lang/model/element/PackageElement.java
2019-05-29 17:14:44.071376000 -0700
---
> ---
old/src/java.compiler/share/classes/javax/lang/model/element/PackageElement.java
2019-05-30 14:16:01.049000999 -0700
> +++
new/src/java.compiler/share/classes/javax/lang/model/element/PackageElement.java
2019-05-30 14:16:00.657000999 -0700
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2005, 2017, Oracle and/or its affiliates. All
rights reserved.
> + * Copyright (c) 2005, 2019, Oracle and/or its affiliates. All
rights reserved.
> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> *
> * This code is free software; you can redistribute it and/or modify it
78c102
< @@ -38,6 +39,12 @@
---
> @@ -38,6 +39,16 @@
83c107,108
< + * Returns a {@linkplain javax.lang.model.type.NoType
pseudo-type} for this package.
---
> + * Returns a {@linkplain javax.lang.model.type.NoType pseudo-type}
> + * for this package.
84a110,112
> + *
> + * @see javax.lang.model.type.NoType
> + * @see javax.lang.model.type.TypeKind#PACKAGE
91,93c119,128
< ---
old/src/java.compiler/share/classes/javax/lang/model/element/TypeElement.java
2019-05-29 17:14:45.139376000 -0700
< +++
new/src/java.compiler/share/classes/javax/lang/model/element/TypeElement.java
2019-05-29 17:14:44.751376000 -0700
< @@ -60,6 +60,25 @@
---
> ---
old/src/java.compiler/share/classes/javax/lang/model/element/TypeElement.java
2019-05-30 14:16:01.745000999 -0700
> +++
new/src/java.compiler/share/classes/javax/lang/model/element/TypeElement.java
2019-05-30 14:16:01.373000999 -0700
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2005, 2017, Oracle and/or its affiliates. All
rights reserved.
> + * Copyright (c) 2005, 2019, Oracle and/or its affiliates. All
rights reserved.
> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> *
> * This code is free software; you can redistribute it and/or modify it
> @@ -60,6 +60,28 @@
110a146,148
> + *
> + * @see Types#asMemberOf(DeclaredType, Element)
> + * @see Types#getDeclaredType(TypeElement, TypeMirror...)
119,120c157,165
< ---
old/src/java.compiler/share/classes/javax/lang/model/element/TypeParameterElement.java
2019-05-29 17:14:45.811376000 -0700
< +++
new/src/java.compiler/share/classes/javax/lang/model/element/TypeParameterElement.java
2019-05-29 17:14:45.455376000 -0700
---
> ---
old/src/java.compiler/share/classes/javax/lang/model/element/TypeParameterElement.java
2019-05-30 14:16:02.509000999 -0700
> +++
new/src/java.compiler/share/classes/javax/lang/model/element/TypeParameterElement.java
2019-05-30 14:16:02.109000999 -0700
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2005, 2011, Oracle and/or its affiliates. All
rights reserved.
> + * Copyright (c) 2005, 2019, Oracle and/or its affiliates. All
rights reserved.
> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> *
> * This code is free software; you can redistribute it and/or modify it
137,139c182,191
< ---
old/src/java.compiler/share/classes/javax/lang/model/element/VariableElement.java
2019-05-29 17:14:46.475376000 -0700
< +++
new/src/java.compiler/share/classes/javax/lang/model/element/VariableElement.java
2019-05-29 17:14:46.119376000 -0700
< @@ -26,6 +26,7 @@
---
> ---
old/src/java.compiler/share/classes/javax/lang/model/element/VariableElement.java
2019-05-30 14:16:03.233000999 -0700
> +++
new/src/java.compiler/share/classes/javax/lang/model/element/VariableElement.java
2019-05-30 14:16:02.857000999 -0700
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2005, 2013, Oracle and/or its affiliates. All
rights reserved.
> + * Copyright (c) 2005, 2019, Oracle and/or its affiliates. All
rights reserved.
> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> *
> * This code is free software; you can redistribute it and/or modify it
> @@ -26,6 +26,8 @@
143a196
> +import javax.lang.model.type.TypeKind;
147c200
< @@ -38,6 +39,12 @@
---
> @@ -38,6 +40,19 @@
152a206,210
> + *
> + * Note that the types of variables range over {@linkplain
> + * TypeKind many kinds} of types, including primitive types,
> + * declared types, and array types, among others.
> + *
153a212,213
> + *
> + * @see TypeKind
161,162c221,222
< +++
new/test/langtools/tools/javac/processing/model/element/TestElementAsType.java
2019-05-29 17:14:46.795376000 -0700
< @@ -0,0 +1,139 @@
---
> +++
new/test/langtools/tools/javac/processing/model/element/TestElementAsType.java
2019-05-30 14:16:03.561000999 -0700
> @@ -0,0 +1,152 @@
273a334,342
> + /*
> + * For both of the maps below, a ElementKind value is mapped to
> + * one value related to an element's asType image. In some cases,
> + * the ElementKind -> (TypeMirror type, TypeKind) mapping is
> + * always the same, such as ElementKind.PACKAGE mapping to
> + * (NoType.class, TypeKind.PACKAGE). In other cases, such as for a
> + * field, there are many possible mappings and they are not
> + * attempted to be examined exhaustively by this test.
> + */
280a350,351
> + // For the field NestedClass.name that is tested, a
> + // declared type is used.
289a361,362
> + // For the field NestedClass.name that is tested, a
> + // declared type is used.
On 5/30/2019 12:04 PM, Joe Darcy wrote:
> Hi Jon,
>
> On 5/30/2019 10:23 AM, Jonathan Gibbons wrote:
>>
>>
>> On 05/30/2019 10:15 AM, Jonathan Gibbons wrote:
>>>
>>>
>>> On 05/29/2019 05:24 PM, Joe Darcy wrote:
>>>> Hello,
>>>>
>>>> Please review the webrev and CSR for:
>>>>
>>>> JDK-8224687: Add clarifying overrides of Element.asType to more
>>>> specific subinterfaces
>>>> webrev: http://cr.openjdk.java.net/~darcy/8224687.2/
>>>> CSR: https://bugs.openjdk.java.net/browse/JDK-8225027
>>>>
>>>> Thanks,
>>>>
>>>> -Joe
>>>>
>>>
>>> Joe,
>>>
>>> CSR Reviewed.
>>>
>>> In the new test, I don't understand lines 119, 128, regarding
>>> ElementKind.FIELD.
>>> Either the code is a copy-paste error, or the description elsewhere
>>> of DeclaredType
>>> and TypeKind.DECLARED is incorrect, since these only refer to
>>> declared types
>>> (class, interface, etc) and not fields.
>>>
>>> Surprising or maybe not, the test actually passes, suggesting that
>>> this is the
>>> current behavior even if it is unexpected. That being said, the
>>> output from the
>>> test would be easier to understand if it included the ElementKind
>>> and simple name
>>> of the element, as well as the typeMirror, typeMirror class and
>>> typeKind.
>>>
>>> -- Jon
>>
>> I guess what is happening is that the mapping to TypeMirror for
>> ElementKind.FIELD
>> varies, depending on the field being declared. If so, that deserves
>> mention in the
>> spec for VariableElement.asType, and also deserves a comment in the
>> test code.
>> I'll stop short of suggesting that the test code should have more
>> fields in it, such as
>> a field of primitive type, but if you could find an easy way to adapt
>> the test for that,
>> that would be good too.
>>
>
> Correct. I propose adding some additional documentation around the
> maps used by the test:
>
>> /*
>> * For both of the maps below, a ElementKind value is mapped to
>> * one value related to an element's asType image. In some cases,
>> * the ElementKind -> (TypeMirror type, TypeKind) mapping is
>> * always the same, such as ElementKind.PACKAGE mapping to
>> * (NoType.class, TypeKind.PACKAGE). In other cases, such as for a
>> * field, there are many possible mappings and they are not
>> * attempted to be examined exhaustively by this test.
>> */
>> private static final Map<ElementKind, Class<?>>
>> elementKindToTypeClass =
>> Map.of(ElementKind.CLASS, DeclaredType.class,
>> ElementKind.CONSTRUCTOR, ExecutableType.class,
>> ElementKind.METHOD, ExecutableType.class,
>> ElementKind.PACKAGE, NoType.class,
>> ElementKind.MODULE, NoType.class,
>> ElementKind.TYPE_PARAMETER, TypeVariable.class,
>> // For the field NestedClass.name that is tested, a
>> // declared type is used.
>> ElementKind.FIELD, DeclaredType.class);
>>
>> private static final Map<ElementKind, TypeKind>
>> elementKindToTypeKind =
>> Map.of(ElementKind.CLASS, TypeKind.DECLARED,
>> ElementKind.CONSTRUCTOR, TypeKind.EXECUTABLE,
>> ElementKind.METHOD, TypeKind.EXECUTABLE,
>> ElementKind.PACKAGE, TypeKind.PACKAGE,
>> ElementKind.MODULE, TypeKind.MODULE,
>> ElementKind.TYPE_PARAMETER, TypeKind.TYPEVAR,
>> // For the field NestedClass.name that is tested, a
>> // declared type is used.
>> ElementKind.FIELD, TypeKind.DECLARED);
>>
> For the cases where only one flavor of mapping is possible, such as
> packages and modules, the revised map explicitly mentions the mapping.
> I'll work on crafting an additional sentence along the lines of "Note
> that a variable can have many possible kinds of types."
>
> Thanks,
>
> -Joe
>
>
More information about the compiler-dev
mailing list