[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-2d-dev
Subject: Re: [OpenJDK 2D-Dev] JDK 9 RFR of JDK-8066621: Suppress deprecation warnings in java.desktop module
From: joe darcy <joe.darcy () oracle ! com>
Date: 2014-12-16 4:07:30
Message-ID: 548FB002.4030809 () oracle ! com
[Download RAW message or body]
On 12/15/2014 11:48 AM, Phil Race wrote:
>
>
> 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 ?
That is not the scenario that is problematic. If you
@SuppressWarnings("deprecation") at the top of a class, that declaration
annotation does *not* cover import statements. Additionally, a
@SuppressWarnings("deprecation") annotation cannot be applied to import
statements (this is a restriction since JDK 5).
Therefore, before the small language change to elide deprecation
warnings on import statements made in JDK 9 (JEP 211), you could have a
class where:
* A deprecated type was imported
* All the uses of the deprecated type in the code were @SuppressWarnings'ed
and still the class would generate deprecation warnings when it was
compiled. To get a warning-free compile, the import statement would need
to be removed and all the uses of the type replaced by fully qualified
names, which is an unreasonable transformation to get warning-free code!
>
> 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.
Conversely, the downside of annotating every call site is that every
call site is annotated...
All the @SuppressWarnings annotations are scoped to the declaration they
are applied to (type, method/constructor, field). If you want to reduce
the scope of warning suppression, a new scope can be introduced, such as
a new method to contain the problematic construct. (This is analogous to
introducing a new temporary variable with an @SuppressWarnings
annotation to constrain the suppression to the expression used to
initialize the variable, a technique that was used on occasion in the
warnings cleanup changes earlier in JDK 9.)
>
>> 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.
A comment could be added to clarify "Package-level method to allow
warning-free package-level use."
>
>>> 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.
Although it may be surprising, that aspect of deprecation goes back a
long ways.
>
> 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.
I've pushed the changes to the client repo.
I will be soon preparing two additional changesets for review to perform
similar deprecation warning suppression in platform-specific client code.
Thanks,
-Joe
[Attachment #3 (text/html)]
<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
On 12/15/2014 11:48 AM, Phil Race wrote:<br>
<blockquote cite="mid:548F3B15.1050300@oracle.com" type="cite">
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
<div class="moz-cite-prefix"><br>
<br>
On 12/12/14 5:35 PM, joe darcy wrote:<br>
</div>
<blockquote cite="mid:548B97D5.9050408@oracle.com" type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
Hi Phil,<br>
<br>
<div class="moz-cite-prefix">On 12/12/2014 12:46 PM, Phil Race
wrote:<br>
</div>
<blockquote cite="mid:548B5434.4020609@oracle.com" type="cite">Hi,
<br>
You did not provide a direct reference to the set of warnings
that were generated. <br>
fortunately I found it here :- <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8066622">https://bugs.openjdk.java.net/browse/JDK-8066622</a>
<br>
</blockquote>
<br>
Each "Suppress deprecations warnings in foo" bug is linked to a
"Fix deprecation warnings in foo" bug which lists the exact
warnings.<br>
<br>
</blockquote>
OK .. direct link would have helped.<br>
<br>
<br>
<blockquote cite="mid:548B97D5.9050408@oracle.com" type="cite">
<blockquote cite="mid:548B5434.4020609@oracle.com" type="cite">
<br>
A couple of things I find 'unfortunate' are <br>
1) In order to avoid a deprecation warning on one call/line of
a 100 line method, <br>
the entire method is subject to the annotation. Eg :- <br>
dev/jdk/src/java.desktop/share/classes/javax/print/ServiceUI.java:226:
warning: [deprecation] show() in Dialog has been deprecated <br>
<br>
Other deprecated uses could silently creep into such a body of
code. <br>
</blockquote>
</blockquote>
<blockquote cite="mid:548B97D5.9050408@oracle.com" type="cite">
<blockquote cite="mid:548B5434.4020609@oracle.com" type="cite">
</blockquote>
<br>
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.)<br>
</blockquote>
<br>
Maybe a digression, but why go to the trouble, why would one
legitimately import a<br>
(deprecated) type and yet not use it ?<br>
</blockquote>
<br>
That is not the scenario that is problematic. If you
@SuppressWarnings("deprecation") at the top of a class, that
declaration annotation does *not* cover import statements.
Additionally, a @SuppressWarnings("deprecation") annotation cannot
be applied to import statements (this is a restriction since JDK 5).<br>
<br>
Therefore, before the small language change to elide deprecation
warnings on import statements made in JDK 9 (JEP 211), you could
have a class where:<br>
<br>
* A deprecated type was imported<br>
* All the uses of the deprecated type in the code were
@SuppressWarnings'ed<br>
<br>
and still the class would generate deprecation warnings when it was
compiled. To get a warning-free compile, the import statement would
need to be removed and all the uses of the type replaced by fully
qualified names, which is an unreasonable transformation to get
warning-free code!<br>
<br>
<blockquote cite="mid:548F3B15.1050300@oracle.com" type="cite"> <br>
But the gist of my point is that with this approach more warnings
can still creep in.<br>
Its unfortunate that the annotation system does not provide a way
to annotate the specific call<br>
and so it is not apparent to the reader what its suppressing.<br>
</blockquote>
<br>
Conversely, the downside of annotating every call site is that every
call site is annotated...<br>
<br>
All the @SuppressWarnings annotations are scoped to the declaration
they are applied to (type, method/constructor, field). If you want
to reduce the scope of warning suppression, a new scope can be
introduced, such as a new method to contain the problematic
construct. (This is analogous to introducing a new temporary
variable with an @SuppressWarnings annotation to constrain the
suppression to the expression used to initialize the variable, a
technique that was used on occasion in the warnings cleanup changes
earlier in JDK 9.)<br>
<br>
<blockquote cite="mid:548F3B15.1050300@oracle.com" type="cite"> <br>
<blockquote cite="mid:548B97D5.9050408@oracle.com" type="cite">
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.<br>
</blockquote>
Adding a wrapper method seems artificial.<br>
Personally I would prefer to find a solution in the annotation
system or find a replacement or de-deprecating<br>
as Alan suggested might work in some cases, although Stuart Marks
said RMI has the same case.<br>
<br>
<br>
<blockquote cite="mid:548B97D5.9050408@oracle.com" type="cite">
<blockquote cite="mid:548B5434.4020609@oracle.com" type="cite">
<br>
2) Some significant fraction of all the warnings are for
getPeer() :- <br>
dev/jdk/src/java.desktop/share/classes/java/awt/Container.java:821:
warning: [deprecation] getPeer() in Component has been
deprecated <br>
</blockquote>
<br>
Yes, I also noticed that a sizeable percentage of the warnings
were for uses of that one method.<br>
</blockquote>
<br>
<blockquote cite="mid:548B97D5.9050408@oracle.com" type="cite"> <br>
<blockquote cite="mid:548B5434.4020609@oracle.com" type="cite">
<br>
The issue here is that the deprecation javadoc tag is unable
to distinguish deprecated for <br>
external usage vs legitimate internal usage.</blockquote>
<br>
FYI, Stuart Mark / Dr. Deprecator gave an interesting talk at
JavaOne this year covering the nuances of deprecation in the
JDK:<br>
<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="https://oracleus.activeevents.com/2014/connect/sessionDetail.ww?SESSION_ID=6377" \
>https://oracleus.activeevents.com/2014/connect/sessionDetail.ww?SESSION_ID=6377</a><br>
>
<br>
<blockquote cite="mid:548B5434.4020609@oracle.com" type="cite">
There is no problem with code inside the desktop module <br>
calling getPeer() which is defined in this same module. There
may not be many other APIs that <br>
have this similar issue, but if there are it might be better
to find some way to make it clear <br>
that we aren't suppressing warnings until we fix the code :
rather we really should not be <br>
receiving a warning here anyway since there is nothing to fix.</blockquote>
<br>
Well, the @SuppressWarnings annotation can be used to convey
that information, perhaps supplemented by a comment or a wrapper
method around getPeer; something like<br>
<br>
/**<br>
* Package-access method somewhere in java.awt<br>
*/<br>
@SuppressWarnings("deprecation")<br>
static java.awt Component privilegeOfPeerage(java.awt Component
c) {<br>
return c.getPeer();<br>
}<br>
<br>
</blockquote>
<br>
I don't think that conveys that its OK to use. It just adds work
to hide it in a different way.<br>
</blockquote>
<br>
A comment could be added to clarify "Package-level method to allow
warning-free package-level use."<br>
<br>
<blockquote cite="mid:548F3B15.1050300@oracle.com" type="cite"> <br>
<blockquote cite="mid:548B97D5.9050408@oracle.com" type="cite">
<blockquote cite="mid:548B5434.4020609@oracle.com" type="cite">
Perhaps "Component. getPeer()" <br>
could acquire an annotation like "module-nodeprecation" which
automatically suppresses the <br>
annotation processor warnings for all such cases. If javac
doesn't know about modules perhaps <br>
we could utilise a javac flag that's used only by the JDK
build to indicate that an annotation <br>
like that should apply. <br>
</blockquote>
<br>
At this point, I think that level of solution would be overkill
(especially given the JDK's historical lack of discipline around
deprecation warnings).<br>
<br>
</blockquote>
<br>
Well .. I think its worth more discussion than dismissing it out
of hand.<br>
<blockquote cite="mid:548B97D5.9050408@oracle.com" type="cite">
<blockquote cite="mid:548B5434.4020609@oracle.com" type="cite">
<br>
Regarding the show() case above I came across a puzzle. <br>
show() is first defined on Component, as is its 'replacement'
setVisible(boolean). <br>
It turns out that what we have in Component is <br>
<br>
public void setVisible(boolean b) { <br>
show(b); <br>
} <br>
<br>
@Deprecated <br>
public void show(boolean b) { <br>
if (b) { <br>
show(); <br>
} else { <br>
hide(); <br>
} <br>
<br>
@Deprecated <br>
public void show() { <br>
... <br>
} <br>
<br>
So I am puzzled why those uses within Component aren't
suppressed in your webrev ? <br>
Is there some automatic suppression of the warnings within the
class that does <br>
the deprecation ? </blockquote>
<br>
Yes, quoting from the JLS:<br>
<br>
"A Java compiler must produce a deprecation warning when a type,
method, field, or constructor whose declaration is annotated
with <code class="literal">@Deprecated</code> is used
(overridden, invoked, or referenced by name) in a construct
which is explicitly or implicitly declared, unless: <br>
* [...]<br>
* The use and declaration are both within the same outermost
class. "<br>
<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://docs.oracle.com/javase/specs/jls/se8/html/jls-9.html#jls-9.6.4.6">http://docs.oracle.com/javase/specs/jls/se8/html/jls-9.html#jls-9.6.4.6</a><br>
</blockquote>
<br>
OK. So that's not as well known as you'd think. It stumped Stuart.<br>
I had to 'infer' this from the observed behaviour.<br>
</blockquote>
<br>
Although it may be surprising, that aspect of deprecation goes back
a long ways.<br>
<br>
<blockquote cite="mid:548F3B15.1050300@oracle.com" type="cite"> <br>
At this point I'll approve the changes although I would like full
consideration<br>
of enhancements to the APS in the future.<br>
<br>
Also I believe this should go to client. If it gets pushed by the
end of the day<br>
or at least no later than the end of tomorrow it should be
integrated by 23rd.<br>
</blockquote>
<br>
I've pushed the changes to the client repo.<br>
<br>
I will be soon preparing two additional changesets for review to
perform similar deprecation warning suppression in platform-specific
client code.<br>
<br>
Thanks,<br>
<br>
-Joe<br>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic