<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p>Hello,</p>
<p>Updated and expanded version of the webrev:</p>
<p>Â Â Â <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~darcy/8240130.1">http://cr.openjdk.java.net/~darcy/8240130.1</a><br>
</p>
<p>A few notes on reviewing:</p>
<p>Â Â Â * 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.<br>
</p>
<p>Â Â Â * To highlight the text differences, I did *not* reflow the
paragraphs in this webrev. I'll do that before a push.</p>
<p>Â Â Â * Some visitors had a tip</p>
<p>
<blockquote type="cite">Note that annotating methods in concrete<br>
subclasses with {@link java.lang.Override @Override} will help<br>
 ensure that methods are overridden as intended.</blockquote>
</p>
<p>Â Â Â 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.</p>
<p>Â Â Â * 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.</p>
<p>Thanks,<br>
</p>
<p>-Joe<br>
</p>
<br>
<div class="moz-cite-prefix">On 3/5/2020 12:18 PM, Jonathan Gibbons
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:0a7353aa-9c06-aa47-145c-765bf2069f89@oracle.com">
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
<p>Looks good to me.</p>
<p>Consider using a ':' instead of ',' on this line in the first
file:<span class="changed" style="color: blue;"></span><br
class="Apple-interchange-newline">
<span class="changed" style="color: blue;"></span></p>
<pre style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;"><span class="changed" style="color: blue;"> 66 * visitUnknown}, behavior that will be overridden in concrete
</span></pre>
<div class="moz-cite-prefix">-- Jon</div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix">On 2/26/20 10:00 PM, Joe Darcy wrote:<br>
</div>
<blockquote type="cite"
cite="mid:c5d47041-4ccd-f3e4-0f59-1f4cf941a864@oracle.com">Hello,
<br>
<br>
Please review the documentation changes for <br>
<br>
   JDK-8240130: Improve and update discussion of visitor
evolution warnings <br>
   <a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/~darcy/8240130.0/"
moz-do-not-send="true">http://cr.openjdk.java.net/~darcy/8240130.0/</a>
<br>
<br>
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. <br>
<br>
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. <br>
<br>
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. <br>
<br>
Thanks, <br>
<br>
-Joe <br>
<br>
---
old/src/java.compiler/share/classes/javax/lang/model/element/ElementVisitor.java
2020-02-26 13:13:00.848880601 -0800 <br>
+++
new/src/java.compiler/share/classes/javax/lang/model/element/ElementVisitor.java
2020-02-26 13:13:00.512880601 -0800 <br>
@@ -40,10 +40,17 @@ <br>
 * is {@code null}; see documentation of the implementing class
for <br>
 * details. <br>
 * <br>
- * <p> <b>WARNING:</b> It is possible that
methods will be added to <br>
+ * @apiNote <br>
+ * <br>
+ * WARNING: It is possible that methods will be added to <br>
 * this interface to accommodate new, currently unknown,
language <br>
 * structures added to future versions of the Java™
programming <br>
- * language. Therefore, visitor classes directly implementing
this <br>
+ * language. <br>
+ * <br>
+ * Such additions have already occurred to support language
features <br>
+ * added after this API was introduced. <br>
+ * <br>
+ * Visitor classes directly implementing this <br>
 * interface may be source incompatible with future versions of
the <br>
 * platform. To avoid this source incompatibility, visitor <br>
 * implementations are encouraged to instead extend the
appropriate <br>
@@ -52,21 +59,15 @@ <br>
 * parameters, return type, etc. rather than one of the
abstract <br>
 * classes. <br>
 * <br>
- * <p>Note that methods to accommodate new language
constructs could <br>
- * be added in a source <em>compatible</em> way if
they were added as <br>
- * <em>default methods</em>. However, default
methods are only <br>
- * available on Java SE 8 and higher releases and the {@code <br>
- * javax.lang.model.*} packages bundled in Java SE 8 were
required to <br>
- * also be runnable on Java SE 7. Therefore, default methods <br>
- * were <em>not</em> used when extending {@code
javax.lang.model.*} <br>
- * to cover Java SE 8 language features. However, default
methods <br>
- * are used in subsequent revisions of the {@code
javax.lang.model.*} <br>
- * packages that are only required to run on Java SE 8 and
higher <br>
- * platform versions. <br>
- * <br>
- * @apiNote <br>
+ * <p>Methods to accommodate new language constructs are
expected to <br>
+ * be added as default methods to provide strong source
compatibility, <br>
+ * as done for {@link visitModule visitModule}. The
implementations of <br>
+ * the default methods will in turn call {@link visitUnknown <br>
+ * visitUnknown}, behavior that will be overridden in concrete
<br>
+ * visitors supporting the source version with the new language
<br>
+ * construct. <br>
 * <br>
- * There are several families of classes implementing this
visitor <br>
+ * <p>There are several families of classes implementing
this visitor <br>
 * interface in the {@linkplain javax.lang.model.util util <br>
 * package}. The families follow a naming pattern along the
lines of <br>
 * {@code FooVisitor}<i>N</i> where
<i>N</i> indicates the <br>
---
old/src/java.compiler/share/classes/javax/lang/model/util/AbstractElementVisitor6.java
2020-02-26 13:13:01.560880601 -0800 <br>
+++
new/src/java.compiler/share/classes/javax/lang/model/util/AbstractElementVisitor6.java
2020-02-26 13:13:01.196880601 -0800 <br>
@@ -36,7 +36,9 @@ <br>
 * appropriate for the {@link SourceVersion#RELEASE_6
RELEASE_6} <br>
 * source version. <br>
 * <br>
- * <p> <b>WARNING:</b> The {@code
ElementVisitor} interface <br>
+ * @apiNote <br>
+ * <br>
+ * <strong>WARNING:</strong> The {@code
ElementVisitor} interface <br>
 * implemented by this class may have methods added to it in
the <br>
 * future to accommodate new, currently unknown, language
structures <br>
 * added to future versions of the Java™ programming
language. <br>
@@ -46,12 +48,12 @@ <br>
 * methods with names beginning with {@code "visit"}. <br>
 * <br>
 * <p>When such a new visit method is added, the default
<br>
- * implementation in this class will be to call the {@link <br>
+ * implementation in this class will be to directly or
indirectly call the {@link <br>
 * #visitUnknown visitUnknown} method. A new abstract element
visitor <br>
 * class will also be introduced to correspond to the new
language <br>
 * level; this visitor will have different default behavior for
the <br>
- * visit method in question. When the new visitor is
introduced, all <br>
- * or portions of this visitor may be deprecated. <br>
+ * visit method in question. When a new visitor is introduced,
<br>
+ * portions of this visitor class may be deprecated, including
its constructors. <br>
 * <br>
 * @param <R> the return type of this visitor's methods.
Use {@link <br>
 *           Void} for visitors that do not need to return
results. <br>
<br>
</blockquote>
</blockquote>
</body>
</html>