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

Jonathan Gibbons jonathan.gibbons at oracle.com
Fri Mar 13 18:38:02 UTC 2020 

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

I note inconsistent use of a blank line after `@apiNote`. I'm not sure 
if that is intentional.

-- 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/3829c2da/attachment.htm>

More information about the compiler-dev mailing list