On 12/12/14 5:35 PM, joe darcy wrote:
Hi Phil,
On 12/12/2014 12:46 PM, Phil Race wrote:
Hi,
You did not provide a direct reference to the set of warnings that
were generated.
fortunately I found it here :-
https://bugs.openjdk.java.net/browse/JDK-8066622
Each "Suppress deprecations warnings in foo" bug is linked to a "Fix
deprecation warnings in foo" bug which lists the exact warnings.
OK .. direct link would have helped.
A couple of things I find 'unfortunate' are
1) In order to avoid a deprecation warning on one call/line of a 100
line method,
the entire method is subject to the annotation. Eg :-
dev/jdk/src/java.desktop/share/classes/javax/print/ServiceUI.java:226: warning:
[deprecation] show() in Dialog has been deprecated
Other deprecated uses could silently creep into such a body of code.
That is true, but today deprecations warnings can (and do) creep into
the entirely of the JDK without notice. Turning on the deprecation
lint warning in the build will prevent that for the vast majority of
code, which is why I want to get the remaining warning suppression
bugs quickly pushed into JDK 9 so the build warning can be enabled.
(This suppression effort was on hold until a small language change was
recently implemented in JDK 9 to eliminate deprecation warnings just
for importing a deprecated type.)
Maybe a digression, but why go to the trouble, why would one
legitimately import a
(deprecated) type and yet not use it ?
But the gist of my point is that with this approach more warnings can
still creep in.
Its unfortunate that the annotation system does not provide a way to
annotate the specific call
and so it is not apparent to the reader what its suppressing.
For the "fix the warning" companion bug to this bug, I would recommend
factoring out the deprecated method call into its own private method
to limit the scope of the @SuppressWarning annotation. For this
changeset, I didn't want to actually modify the contents or structure
of any methods I so didn't undertake that kind of transformation.
Adding a wrapper method seems artificial.
Personally I would prefer to find a solution in the annotation system or
find a replacement or de-deprecating
as Alan suggested might work in some cases, although Stuart Marks said
RMI has the same case.
2) Some significant fraction of all the warnings are for getPeer() :-
dev/jdk/src/java.desktop/share/classes/java/awt/Container.java:821:
warning: [deprecation] getPeer() in Component has been deprecated
Yes, I also noticed that a sizeable percentage of the warnings were
for uses of that one method.
The issue here is that the deprecation javadoc tag is unable to
distinguish deprecated for
external usage vs legitimate internal usage.
FYI, Stuart Mark / Dr. Deprecator gave an interesting talk at JavaOne
this year covering the nuances of deprecation in the JDK:
https://oracleus.activeevents.com/2014/connect/sessionDetail.ww?SESSION_ID=6377
There is no problem with code inside the desktop module
calling getPeer() which is defined in this same module. There may
not be many other APIs that
have this similar issue, but if there are it might be better to find
some way to make it clear
that we aren't suppressing warnings until we fix the code : rather we
really should not be
receiving a warning here anyway since there is nothing to fix.
Well, the @SuppressWarnings annotation can be used to convey that
information, perhaps supplemented by a comment or a wrapper method
around getPeer; something like
/**
* Package-access method somewhere in java.awt
*/
@SuppressWarnings("deprecation")
static java.awt Component privilegeOfPeerage(java.awt Component c) {
return c.getPeer();
}
I don't think that conveys that its OK to use. It just adds work to hide
it in a different way.
Perhaps "Component. getPeer()"
could acquire an annotation like "module-nodeprecation" which
automatically suppresses the
annotation processor warnings for all such cases. If javac doesn't
know about modules perhaps
we could utilise a javac flag that's used only by the JDK build to
indicate that an annotation
like that should apply.
At this point, I think that level of solution would be overkill
(especially given the JDK's historical lack of discipline around
deprecation warnings).
Well .. I think its worth more discussion than dismissing it out of hand.
Regarding the show() case above I came across a puzzle.
show() is first defined on Component, as is its 'replacement'
setVisible(boolean).
It turns out that what we have in Component is
public void setVisible(boolean b) {
show(b);
}
@Deprecated
public void show(boolean b) {
if (b) {
show();
} else {
hide();
}
@Deprecated
public void show() {
...
}
So I am puzzled why those uses within Component aren't suppressed in
your webrev ?
Is there some automatic suppression of the warnings within the class
that does
the deprecation ?
Yes, quoting from the JLS:
"A Java compiler must produce a deprecation warning when a type,
method, field, or constructor whose declaration is annotated with
|@Deprecated| is used (overridden, invoked, or referenced by name) in
a construct which is explicitly or implicitly declared, unless:
* [...]
* The use and declaration are both within the same outermost class. "
http://docs.oracle.com/javase/specs/jls/se8/html/jls-9.html#jls-9.6.4.6
OK. So that's not as well known as you'd think. It stumped Stuart.
I had to 'infer' this from the observed behaviour.
At this point I'll approve the changes although I would like full
consideration
of enhancements to the APS in the future.
Also I believe this should go to client. If it gets pushed by the end of
the day
or at least no later than the end of tomorrow it should be integrated by
23rd.
-phil.
Thanks for the review,
-Joe
If so then perhaps the module idea above can be considered
an extension of this. If that isn't what's happening, then what is ?
-phil.
On 12/9/2014 4:41 PM, joe darcy wrote:
Hello,
In support of JEP 212: Resolve Lint and Doclint Warnings
(http://openjdk.java.net/jeps/212), which is targeted to JDK 9,
please review the large but straightforward set of changes in the
webrev:
http://cr.openjdk.java.net/~darcy/8066621.0/
Some background of the approach being taken to address this part of
JEP 212 was discussed on core-libs:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-December/030085.html
Briefly, to allow the deprecation warnings to be dealt with and that
sole remaining lint warning category enabled in the build, a
two-step approach is being taken. The first step is to suppress the
deprecation warnings and the second step is for area-experts to
examine the specific uses of deprecated APIs in their code. This
webrev only attempts to cover the first step.
The webrev is based off of the JDK 9 "dev" forest rather than the
"client" forest. Since the change only involves copyright updates
and adding annotations, there would be no functional modification in
the changeset. Therefore, I would strongly prefer to push these
changes directly to dev rather than pushing them to client and
waiting for them to propagate to dev to expedite the time when the
build warning can be enabled. (If a warning is not enabled in the
build, new instances of the warning tend to creep into the code base.)
Thanks,
-Joe
