JDK 15 RFR of JDK-8240130: Improve and update discussion of visitor evolution warnings

Jonathan Gibbons jonathan.gibbons at oracle.com
Fri Mar 13 19:14:52 UTC 2020 

+1


On 03/13/2020 12:13 PM, Joe Darcy wrote:
> On 3/13/2020 11:38 AM, Jonathan Gibbons wrote:
>>
>> Generally OK, although you may have removed a bit too much text (i.e. 
>> the @param tags) in
>> src/java.compiler/share/classes/javax/lang/model/util/AbstractElementVisitor8.java
>>
> Thanks for catching this; will fix before pushing.
>
>
>> I note inconsistent use of a blank line after `@apiNote`. I'm not 
>> sure if that is intentional.
>>
> Before pushing, I'll re-flow the paragraphs and delete and a spacer 
> line between @apiNote and the subsequent text.
>
> -Joe
>
>> -- Jon
>>
>>
>> On 03/13/2020 11:16 AM, Joe Darcy wrote:
>>>
>>> Hello,
>>>
>>> Following an off-list suggestion from Jon, I've re-worked the change 
>>> to remove most of the WARNING paragraphs and replace them with @see 
>>> links:
>>>
>>> http://cr.openjdk.java.net/~darcy/8240130.2
>>>
>>> Each root visitor/scanner has a slightly different wording; the 
>>> element kind visitor is not strictly correct without a specialized 
>>> wording. Therefore, visitors other than the AbstractFooVisitor6 have 
>>> warning paragraphs.
>>>
>>> Thanks Jon for the helpful suggestion.
>>>
>>> -Joe
>>>
>>> On 3/10/2020 10:09 PM, Joe Darcy wrote:
>>>>
>>>> Hello,
>>>>
>>>> Updated and expanded version of the webrev:
>>>>
>>>> http://cr.openjdk.java.net/~darcy/8240130.1
>>>>
>>>> A few notes on reviewing:
>>>>
>>>>     * The delta for each FooVisitor$N utility visitor family should 
>>>> be the same. I confirmed this visually by aligning up the udiffs of 
>>>> each family and switching between tabs.
>>>>
>>>>     * To highlight the text differences, I did *not* reflow the 
>>>> paragraphs in this webrev. I'll do that before a push.
>>>>
>>>>     * Some visitors had a tip
>>>>
>>>>> Note that annotating methods in concrete
>>>>> subclasses with {@link java.lang.Override @Override} will help
>>>>>  ensure that methods are overridden as intended.
>>>>
>>>>     While this may have been useful back in JDK 6, I don't think it 
>>>> is needed in JDK 15 and I'm proposing to remove these notes.
>>>>
>>>>     * The wording in TypeKindVisitor$N was updated to explicitly 
>>>> mention the possibility of new enum values being added, aligning 
>>>> with the wording already in use in ElementKindVisitor$N.
>>>>
>>>> Thanks,
>>>>
>>>> -Joe
>>>>
>>>>
>>>> On 3/5/2020 12:18 PM, Jonathan Gibbons wrote:
>>>>>
>>>>> Looks good to me.
>>>>>
>>>>> Consider using a ':' instead of ',' on this line in the first file:
>>>>>
>>>>> 66 * visitUnknown}, behavior that will be overridden in concrete
>>>>> -- Jon
>>>>>
>>>>> On 2/26/20 10:00 PM, Joe Darcy wrote:
>>>>>> Hello,
>>>>>>
>>>>>> Please review the documentation changes for
>>>>>>
>>>>>>     JDK-8240130: Improve and update discussion of visitor 
>>>>>> evolution warnings
>>>>>> http://cr.openjdk.java.net/~darcy/8240130.0/
>>>>>>
>>>>>> Patch below. Some discussion of and rationale for the changes, 
>>>>>> the original javax.lang.model visitor interfaces and concrete 
>>>>>> visitor types had various warning about the unusual compatibility 
>>>>>> policies those types would use. Unlike other post-Java SE 6 era 
>>>>>> types, it was anticipated methods would be added to interfaces. 
>>>>>> Prior to default methods in JDK 8, such additions were a highly 
>>>>>> unusual occurrence for public types in the JDK. After default 
>>>>>> methods were used commonly in jDK 8, such additions are of less 
>>>>>> concern. The risk of a true source compatibility is also low for 
>>>>>> a new "visitFoo" method since it will have a parameter of type 
>>>>>> Foo, which is new to the release, and thus not have an 
>>>>>> overloading conflict with a method in an existing visitor outside 
>>>>>> of the JDK.
>>>>>>
>>>>>> The warnings are thus toned down and adjusted to note that the 
>>>>>> envisioned additions have already occurred for prior language 
>>>>>> additions. For example, the current policy is only to deprecate 
>>>>>> the constructors of obsolete visitor classes rather than the 
>>>>>> whole class. The "directly or indirectly call" visitUnknown is to 
>>>>>> accurately describe, but not limit to, the current policy of the 
>>>>>> visitFoo methods in affected concrete visitors calling the 
>>>>>> visitFoo default method on the *interface*, which in turn calls 
>>>>>> visitUnknown.
>>>>>>
>>>>>> The warning text is pulled into an apiNote to avoid getting 
>>>>>> inherit-doc-ed for any subclasses. Once the shape of the changes 
>>>>>> for the interface and visitor classes are agree to, I'll 
>>>>>> replicated the changes for the other types.
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> -Joe
>>>>>>
>>>>>> --- 
>>>>>> old/src/java.compiler/share/classes/javax/lang/model/element/ElementVisitor.java 
>>>>>> 2020-02-26 13:13:00.848880601 -0800
>>>>>> +++ 
>>>>>> new/src/java.compiler/share/classes/javax/lang/model/element/ElementVisitor.java 
>>>>>> 2020-02-26 13:13:00.512880601 -0800
>>>>>> @@ -40,10 +40,17 @@
>>>>>>   * is {@code null}; see documentation of the implementing class for
>>>>>>   * details.
>>>>>>   *
>>>>>> - * <p> <b>WARNING:</b> It is possible that methods will be added to
>>>>>> + * @apiNote
>>>>>> + *
>>>>>> + * WARNING: It is possible that methods will be added to
>>>>>>   * this interface to accommodate new, currently unknown, language
>>>>>>   * structures added to future versions of the Java™ 
>>>>>> programming
>>>>>> - * language.  Therefore, visitor classes directly implementing this
>>>>>> + * language.
>>>>>> + *
>>>>>> + * Such additions have already occurred to support language 
>>>>>> features
>>>>>> + * added after this API was introduced.
>>>>>> + *
>>>>>> + * Visitor classes directly implementing this
>>>>>>   * interface may be source incompatible with future versions of the
>>>>>>   * platform.  To avoid this source incompatibility, visitor
>>>>>>   * implementations are encouraged to instead extend the appropriate
>>>>>> @@ -52,21 +59,15 @@
>>>>>>   * parameters, return type, etc. rather than one of the abstract
>>>>>>   * classes.
>>>>>>   *
>>>>>> - * <p>Note that methods to accommodate new language constructs 
>>>>>> could
>>>>>> - * be added in a source <em>compatible</em> way if they were 
>>>>>> added as
>>>>>> - * <em>default methods</em>.  However, default methods are only
>>>>>> - * available on Java SE 8 and higher releases and the {@code
>>>>>> - * javax.lang.model.*} packages bundled in Java SE 8 were 
>>>>>> required to
>>>>>> - * also be runnable on Java SE 7.  Therefore, default methods
>>>>>> - * were <em>not</em> used when extending {@code javax.lang.model.*}
>>>>>> - * to cover Java SE 8 language features.  However, default methods
>>>>>> - * are used in subsequent revisions of the {@code 
>>>>>> javax.lang.model.*}
>>>>>> - * packages that are only required to run on Java SE 8 and higher
>>>>>> - * platform versions.
>>>>>> - *
>>>>>> - * @apiNote
>>>>>> + * <p>Methods to accommodate new language constructs are 
>>>>>> expected to
>>>>>> + * be added as default methods to provide strong source 
>>>>>> compatibility,
>>>>>> + * as done for {@link visitModule visitModule}. The 
>>>>>> implementations of
>>>>>> + * the default methods will in turn call {@link visitUnknown
>>>>>> + * visitUnknown}, behavior that will be overridden in concrete
>>>>>> + * visitors supporting the source version with the new language
>>>>>> + * construct.
>>>>>>   *
>>>>>> - * There are several families of classes implementing this visitor
>>>>>> + * <p>There are several families of classes implementing this 
>>>>>> visitor
>>>>>>   * interface in the {@linkplain javax.lang.model.util util
>>>>>>   * package}. The families follow a naming pattern along the 
>>>>>> lines of
>>>>>>   * {@code FooVisitor}<i>N</i> where <i>N</i> indicates the
>>>>>> --- 
>>>>>> old/src/java.compiler/share/classes/javax/lang/model/util/AbstractElementVisitor6.java 
>>>>>> 2020-02-26 13:13:01.560880601 -0800
>>>>>> +++ 
>>>>>> new/src/java.compiler/share/classes/javax/lang/model/util/AbstractElementVisitor6.java 
>>>>>> 2020-02-26 13:13:01.196880601 -0800
>>>>>> @@ -36,7 +36,9 @@
>>>>>>   * appropriate for the {@link SourceVersion#RELEASE_6 RELEASE_6}
>>>>>>   * source version.
>>>>>>   *
>>>>>> - * <p> <b>WARNING:</b> The {@code ElementVisitor} interface
>>>>>> + * @apiNote
>>>>>> + *
>>>>>> + * <strong>WARNING:</strong> The {@code ElementVisitor} interface
>>>>>>   * implemented by this class may have methods added to it in the
>>>>>>   * future to accommodate new, currently unknown, language 
>>>>>> structures
>>>>>>   * added to future versions of the Java™ programming 
>>>>>> language.
>>>>>> @@ -46,12 +48,12 @@
>>>>>>   * methods with names beginning with {@code "visit"}.
>>>>>>   *
>>>>>>   * <p>When such a new visit method is added, the default
>>>>>> - * implementation in this class will be to call the {@link
>>>>>> + * implementation in this class will be to directly or 
>>>>>> indirectly call the {@link
>>>>>>   * #visitUnknown visitUnknown} method.  A new abstract element 
>>>>>> visitor
>>>>>>   * class will also be introduced to correspond to the new language
>>>>>>   * level; this visitor will have different default behavior for the
>>>>>> - * visit method in question.  When the new visitor is 
>>>>>> introduced, all
>>>>>> - * or portions of this visitor may be deprecated.
>>>>>> + * visit method in question.  When a new visitor is introduced,
>>>>>> + * portions of this visitor class may be deprecated, including 
>>>>>> its constructors.
>>>>>>   *
>>>>>>   * @param <R> the return type of this visitor's methods. Use {@link
>>>>>>   *            Void} for visitors that do not need to return 
>>>>>> results.
>>>>>>
>>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20200313/e34b6bd9/attachment.htm>

More information about the compiler-dev mailing list