<!DOCTYPE html><html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<p>Hi Archie,<br>
the idea is surely intriguing. Reminds me of teh way exception
analysis work, but applied to Lint warnings :-)</p>
<p>Logistically, I'd like to air some concerns:</p>
<p>* There have been other requests to alter and/or enhance Lint
warnings recently, and some changes in the area are likely
unavoidable because of null-restricted types (Valhalla). One
common request is to allow for "families" of logically-related
warnings (so that, on the command line, I can decide at which
level of granularity I want lint warnings to be enabled/disabled).
Something like this has a non-trivial chance to interact with your
proposal.<br>
</p>
<p>* Implementation-wise, I've always been unhappy about the way in
which Lint warnings are so deeply integrated inside the core javac
classes. Maybe for some of them (e.g. the file manager ones) not
much can be done... but for most of them I do wonder whether they
should all be done in a separate pass, after flow analysis is
complete -- and I also wonder if this should be some kind of
mechanism that should be open for extension (e.g. so that
developers can define their own analyses). Of course, the more
Lint warnings are a core part of javac, and the harder this is.</p>
<p>* Even if we never implement the proposal in full, I think there
are useful bits and pieces in your email - such as consolidating
the way in which lint warnings are reported. I believe these are
good changes to make, and I'd like to perhaps consider them
separately. It is likely that any improvement and consolidation in
this area will make any subsequent change to the way in which lint
warnings are reported easier to implement, so I support such
changes and refactorings.</p>
<p>* On a similar note, it would perhaps be better to separate the
javac changes required to enable the new analysis, from the JDK
changes to remove redundant suppressions. In other similar cases
where a new analysis has been added, we have been filing umbrella
issues for the various JDK subcomponents:</p>
<p><a class="moz-txt-link-freetext" href="https://bugs.openjdk.org/browse/JDK-8244681">https://bugs.openjdk.org/browse/JDK-8244681</a></p>
<p>Cheers<br>
Maurizio<br>
</p>
<div class="moz-cite-prefix">On 09/11/2024 22:50, Archie Cobbs
wrote:<br>
</div>
<blockquote type="cite" cite="mid:CANSoFxth8njxzfag-Rff5WqYe7Ykx=L3e7v1VJ_ArcW8cN_iMQ@mail.gmail.com">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div><b>Overview</b><br>
</div>
<div><br>
</div>
<div>This is a
proposal to
add the
ability for
the compiler
to detect and
report
unnecessary
warning
suppressions.</div>
<div><br>
</div>
<div>An
"unnecessary
warning
suppression"
is when one of
the following
happens:<br>
</div>
<div>
<ul>
<li><span style="font-family:arial,sans-serif">There is a </span><span style="font-family:monospace">@SuppressWarnings("foo")</span><font face="arial,sans-serif"> annotation, but if it hadn't been there, no </font><span style="font-family:monospace">foo </span><font face="arial,sans-serif">warning
would have
been generated
within the
annotation's
scope<br>
</font></li>
<li><span style="font-family:arial,sans-serif">The compiler is passed </span><span style="font-family:monospace">-Xlint:-foo</span>, but if it hadn't been,
no <span style="font-family:monospace">foo</span> warning would<font face="arial,sans-serif"> have been generated during the entire
compilation<br>
</font></li>
</ul>
</div>
<div><b>Motivation</b></div>
<div><br>
</div>
<div><span style="font-family:monospace">@SuppressWarnings</span> and <span style="font-family:monospace">-Xlint:-foo</span> are blunt instruments.
The latter is
maximally
blunt: it
covers the
entire
compilation.
The former is
somewhat
blunt,
especially
when the
warning occurs
at a specific
statement
other than a
variable
declaration
and so the
annotation has
to annotate
and cover the
entire
containing
method.<br>
</div>
<div><br>
</div>
<div>In
practice <span style="font-family:monospace">@SuppressWarnings</span> and <span style="font-family:monospace">-Xlint:-foo</span> are also very sticky:
once they get
added to a
source file or
a build
process, they
are rarely
removed,
because that
would require
an audit to
determine if
the original
problem is now
resolved (or
the compiler
behavior has
changed),
which is
tedious.<br>
</div>
<br>
<div>Sometimes <span style="font-family:monospace">@SuppressWarnings</span> annotations are
never needed
in the first
place: they're
added to the
code
proactively as
the code is
written
because the
developer
thinks they <i>might</i>
be needed. In
this
situation, the
compiler
provides the
same feedback
either way
(i.e. none),
so this type
of mistake is
almost never
caught.<br>
</div>
<div>
<div><br>
</div>
<div>As code
evolves over
time, newly
added bugs
that warnings
are designed
to catch can
escape
detection if
they happen to
appear within
the scope of a
<span style="font-family:monospace">@SuppressWarnings</span> or <span style="font-family:monospace">-Xlint:-foo</span> flag. That problem
can't be
solved
completely,
but it can be
minimized by
ensuring that
all <span style="font-family:monospace">@SuppressWarnings</span> annotations and <span style="font-family:monospace">-Xlint:-foo</span> flags that do exist are
actually
serving some
purpose.</div>
<div><br>
</div>
<div>More
generally,
there is the
natural and
healthy need
to
"declutter",
and also the
"peace of
mind" factor:
We want to
know we're
doing
everything we
reasonably can
to prevent
bugs... and
since the
compiler is
the thing that
generates the
warnings in
the first
place,
shouldn't it
also be able
to detect and
report when a
warning is
being
unnecessarily
suppressed?</div>
<div><br>
</div>
<div><b>Caveats</b></div>
<div><br>
</div>
<div>There are
real-world
concerns with
adding
something like
this. Lots of
people build
with <span style="font-family:monospace">-Xlint:all</span>. We don't want to
constrict the
compiler so
tightly that
it becomes
more
frustrating
than helpful
for people
trying to
build software
in the real
world. Warning
behavior can
differ not
only across
JDK versions
but also
across
operating
systems, so we
don't want to
force
over-complexification
of builds.</div>
<div><br>
</div>
<div>There is
a balance to
strike; the
functionality
should be easy
to disable.<br>
</div>
<br>
</div>
<div><b>Proposal</b><br>
</div>
<div><br>
</div>
<div>Add two
new lint
categories, as
described by
this <span style="font-family:monospace">--help-lint</span> output:<br>
</div>
<div><br>
</div>
<div><span style="font-family:monospace"> suppression Warn about
@SuppressWarnings
values that
don't actually
suppress any
warnings.<br>
suppression-option
Warn about
-Xlint:-key
options that
don't actually
suppress any
warnings
(requires
"options").<br>
</span></div>
<div><br>
</div>
<div>Notice
that for <span style="font-family:monospace">suppression-option</span> to work, you
also have to
enable <span style="font-family:monospace">options</span> (see below for discussion).</div>
<br>
<div>
<div>The
behavior in a
nutshell:</div>
<div>
<ul>
<li>When
warnable code
is detected,
the warning
"bubbles up"
until it hits
the first <span style="font-family:monospace">@SuppressWarning</span> annotation in
scope, or if
none, the <span style="font-family:monospace">-Xlint:-foo</span> option (if any).</li>
<li>If the
warning
doesn't hit
anything and
"escapes", the
warning is
emitted (this
is what
happens today)<br>
</li>
<li>Otherwise,
the warning
has hit a <i>suppression</i>
- either a <span style="font-family:monospace">@SuppressWarning</span> annotation or
global <span style="font-family:monospace">-Xlint:-foo</span> option - and so:</li>
<ul>
<li>It is
suppressed
(this is what
happens
today), and</li>
<li>NEW: That
suppression is
marked as <i>validated</i></li>
</ul>
<li>NEW: After
processing
each file, the
<span style="font-family:monospace">suppression</span> category warns about <span style="font-family:monospace">@SuppressWarning</span> annotations in
that file
containing
unvalidated
categories<br>
</li>
<li>NEW: After
processing the
entire
compilation,
the <span style="font-family:monospace">suppression-option</span> category warns
about
unvalidated <span style="font-family:monospace">-Xlint:-foo</span> options.</li>
</ul>
</div>
</div>
<div>Here's an
example using
<span style="font-family:monospace">rawtypes</span> to demonstrate the
proposed
behavior:</div>
<div><br>
</div>
<div style="margin-left:40px"><span style="font-family:monospace">@SuppressWarnings("rawtypes")
// annotation
#1<br>
</span></div>
<div style="margin-left:40px"><span style="font-family:monospace">public
class Test {</span></div>
<div style="margin-left:40px"><span style="font-family:monospace"><br>
</span></div>
<div style="margin-left:40px"><span style="font-family:monospace">
@SuppressWarnings("rawtypes")
// annotation
#2<br>
</span></div>
<div style="margin-left:40px"><span style="font-family:monospace"> public
Iterable obj =
null; //
"rawtypes"
warning here<br>
</span></div>
<div style="margin-left:40px"><span style="font-family:monospace">}</span></div>
<div><br>
</div>
<div>For a <span style="font-family:monospace">rawtypes</span> warning to be emitted, the
following must
be true:</div>
<div>
<ul>
<li><span style="font-family:monospace">-Xlint:rawtypes</span> must be enabled</li>
<li>Annotation
#1 and
annotation #2
must both NOT
be present</li>
</ul>
</div>
<div>This is
the same logic
that we
already have.<br>
</div>
<div><br>
</div>
<div>For a <span style="font-family:monospace">suppression</span> warning to be emitted
at outer
annotation #1
the following
must be true:</div>
<div>
<div>
<ul>
<li><span style="font-family:monospace">-Xlint:</span><span style="font-family:monospace">suppression</span> must be enabled</li>
<li>Annotation
#1 AND
annotation #2
must BOTH be
present</li>
</ul>
</div>
</div>
<div>Note that
in this case
either
annotation
could be
declared as
the
"unnecessary"
one, but when
nested
annotations
suppress the
same warning,
we will always
assume that
the innermost
annotation is
the "real" one
(it's the
first to
"catch" the
warning as it
bubbles up)
and any
containing
annotations
are therefore
the
"unnecessary"
ones.</div>
<div><br>
</div>
<div>As a
result, it
would never be
possible for a
<span style="font-family:monospace">suppression</span> warning to be emitted
at annotation
#2.<br>
</div>
<div><br>
</div>
<div>Also note
that the
category being
suppressed
does not
itself need to
be enabled:
the lint
categories <span style="font-family:monospace">rawtypes</span> and <span style="font-family:monospace">suppression</span> warn about two
different
things, and so
they are
enabled/disabled
independently
(*)<br>
</div>
<div><br>
</div>
<div>(*) This
might be
debatable. One
could argue
that if <span style="font-family:monospace">rawtypes</span> is not enabled, then all
activity
related to the
<span style="font-family:monospace">rawtypes</span> warning should be shut
down,
including
determining
whether there
is any
unnecessary
suppression of
it. This would
be a more
conservative
change, but it
would mean
that only the
warnings that
are actually
enabled could
be detected as
unnecessarily
suppressed,
which is a
less robust
check. In
addition, it
would mean
that for any
given lint
category, only
one of the <span style="font-family:monospace">suppression</span> or <span style="font-family:monospace">suppression-option</span> categories could
be applicable
at a time,
which seems
too limiting.<br>
</div>
<div><br>
</div>
<div>
<div>For a <span style="font-family:monospace">suppression-option</span> warning to be
emitted for
the above
example, the
following must
be true:</div>
<div>
<div>
<ul>
<li><span style="font-family:monospace">-Xlint:options</span> must be enabled</li>
<li><span style="font-family:monospace">-Xlint:</span><span style="font-family:monospace">suppression</span><span style="font-family:monospace">-option</span> must be enabled</li>
<li><span style="font-family:monospace">-Xlint:-rawtypes</span> must be specified
(i.e., it must
be actively
suppressed,
not just
disabled which
is the
default)<br>
</li>
<li>At least
one of
annotation #1
or annotation
#2 must be
present</li>
</ul>
</div>
</div>
</div>
<div>The
reason for
requiring <span style="font-family:monospace">options</span> is that the warning does in
fact relate to
a command line
option and so
it seems
appropriate
that it be
included. In
practice, <span style="font-family:monospace">options</span> appears to be already in
use as a
"catch-all"
when building
on multiple
operating
systems and/or
JDK versions,
etc., so this
will make for
a cleaner
upgrade path.<br>
</div>
<div><b><br>
</b></div>
<div><b>Gory
Details<br>
</b></div>
<br>
<div>Some lint
categories
don't support
<span style="font-family:monospace">@SuppressWarnings</span> annotation
scoping, e.g,
<span style="font-family:monospace">classfile</span>, <span style="font-family:monospace">output-file-clash</span>, <span style="font-family:monospace">path</span>, and <span style="font-family:monospace">text-blocks</span> (the latter because it
is calculated
by the scanner
before
annotation
symbols are
available).
Putting them
in a <span style="font-family:monospace">@SuppressWarnings</span> annotation is
always useless
(and will be
reported as
such).
However, they
are still
viable
candidates for
the <span style="font-family:monospace">suppression-option</span> warning.</div>
</div>
<div dir="ltr">
<div>
<div><b><br>
</b></div>
Some lint
categories
will be
omitted from
"suppression
tracking"
altogether:</div>
<div>
<ul>
<li><span style="font-family:monospace">path</span><br>
<span style="font-family:monospace"></span></li>
<li><span style="font-family:monospace">options</span></li>
<li><span style="font-family:monospace">suppression</span></li>
<li><span style="font-family:monospace">suppression-option</span><br>
</li>
</ul>
</div>
<div>The path
category is
omitted
because it is
used too early
in the
pipeline
(before
singletons are
created).</div>
<div><br>
</div>
<div>The <span style="font-family:monospace">options</span> category is omitted because
including it
would be
pointless:<br>
</div>
<div>
<ul>
<li>It doesn't
support <span style="font-family:monospace">@SuppressWarnings</span>, so <span style="font-family:monospace">suppressions</span> doesn't apply<br>
</li>
<li>If there's
<span style="font-family:monospace">-Xlint:-options</span>, then <span style="font-family:monospace">suppression-option</span> is also disabled<br>
</li>
</ul>
</div>
<div>What
about the
self-referential
nature of
suppressing <span style="font-family:monospace">suppression</span> itself? Consider this
example:</div>
<div><br>
</div>
<div style="margin-left:40px"><span style="font-family:monospace">@SuppressWarnings({
"rawtypes",
"suppression"
})</span></div>
<div style="margin-left:40px"><span style="font-family:monospace">public
class Test { }<br>
</span></div>
<div><br>
</div>
<div>There is
no <span style="font-family:monospace">rawtypes</span> warning in there, so the
suppression of
<span style="font-family:monospace">rawtypes</span> is indeed unnecessary and
would normally
result in a <span style="font-family:monospace">suppression</span> warning. But we also
are
suppressing
the <span style="font-family:monospace">suppression</span> warning itself, so the
end result is
that no
warning would
be generated.<br>
</div>
<div><br>
</div>
<div>OK what
about this?</div>
<div><br>
</div>
<div>
<div style="margin-left:40px"><span style="font-family:monospace">@SuppressWarnings("suppression")</span></div>
<div style="margin-left:40px"><span style="font-family:monospace">public
class Test { }<br>
</span></div>
<div><br>
</div>
<div>If <span style="font-family:monospace">suppression</span> were itself subject to
suppression
tracking, this
example would
lead to a
paradox.
Instead, we
exclude <span style="font-family:monospace">suppression</span> itself from suppression
tracking. So
that example
would generate
no warning.
Analogous
logic applies
to <span style="font-family:monospace">suppression-option</span> - it doesn't
apply to
itself.<br>
</div>
</div>
<div>
<div><br>
</div>
<div>Note that
<span style="font-family:monospace">@SuppressWarnings("suppression")</span> is
not totally
useless,
because it can
affect nested
annotations:</div>
<div><br>
</div>
<div>
<div style="margin-left:40px"><span style="font-family:monospace">@SuppressWarnings("suppression")
// this is NOT
unnecessary<br>
</span></div>
<div style="margin-left:40px"><span style="font-family:monospace">public
class Test {</span></div>
<div style="margin-left:40px"><span style="font-family:monospace"><br>
</span></div>
<div style="margin-left:40px"><span style="font-family:monospace"> </span><span style="font-family:monospace">// Suppression of "rawtypes" is
unnecessary -
but that won't
be reported<br>
</span></div>
<div style="margin-left:40px">
<div style="margin-left:40px"><span style="font-family:monospace"></span></div>
<div style="margin-left:40px"><span style="font-family:monospace">@SuppressWarnings("rawtypes")<br>
</span></div>
<div style="margin-left:40px"><span style="font-family:monospace">public int
x = 1;<br>
</span></div>
</div>
<div style="margin-left:40px"><span style="font-family:monospace">}<br>
</span></div>
<div><br>
</div>
</div>
<div>Making <span style="font-family:monospace">suppression-option</span> a separate
warning from <span style="font-family:monospace">suppression</span> seems a reasonably
obvious thing
to do but
there are also
some subtle
reasons for
doing that.<br>
</div>
<div>
<div>
<div><br>
</div>
<div>First,
any system
that does
incremental
builds (like
the JDK
itself) can
have a problem
if the <span style="font-family:monospace">suppression-option</span> warning is
applied to a
partial
compilation,
because what
if the file(s)
that generate
the warning
being
suppressed are
not part of
that
particular
build? Then
you would get
a false
positive. So
incremental
builds could
disable <span style="font-family:monospace">suppression-option</span> but still
safely leave <span style="font-family:monospace">suppression</span> enabled.<br>
</div>
</div>
<div><b><br>
</b></div>
</div>
<div>Also,
different
versions of
the JDK
support
different lint
flags and have
different
warning logic,
so that
warnings in
some versions
don't occur in
other
versions. When
the same
source needs
to be compiled
under multiple
JDK versions,
some <span style="font-family:monospace">-Xlint:-foo</span> flags may be necessary
in some
versions and
useless in
others. We
want to ensure
there's a
reasonably
simple way to
use the same
command line
flags when
compiling
under
different JDK
versions
without having
to disable
suppression
tracking
altogether.</div>
<div><br>
</div>
<div>Similarly, for
some warnings
the operating
system might
affect whether
warnings are
generated.</div>
</div>
<div><br>
</div>
<div><b>Prototype
Status</b><br>
</div>
<div><br>
</div>
<div>What
follows is
probably TMI
but I figured
I'd include a
full brain
dump while top
of mind...<br>
</div>
<div><br>
</div>
<div>I
originally
implemented
this just to
see how hard
it would be
and to play
around with
the idea; it
seems like the
experiment has
worked fairly
well.<br>
</div>
<div>
<div><br>
</div>
<div>Of course
the first
thing I wanted
to try was to
run it on the
JDK itself.
This revealed
400+
unnecessary <span style="font-family:monospace">@SuppressWarnings</span> annotations and
11 unnecessary
<span style="font-family:monospace">-Xlint:foo</span> flags detected. That
showed that
the issue
being
addressed is
not imaginary.</div>
<div><br>
</div>
<div>Of
course,
because most
of the JDK is
built with <span style="font-family:monospace">-Xlint:all </span>(or close to it), that
also meant
tracking down
and removing
all of the
unnecessary
suppressions;
I had to
semi-automate
the process. A
side-effect of
that effort is
<a href="https://github.com/openjdk/jdk/pulls?q=author%3Aarchiecobbs+is%3Apr+%22Remove+unnecessary%22+in%3Atitle+" target="_blank" moz-do-not-send="true">a series of separate PR's</a> to
remove
unnecessary <span style="font-family:monospace">@SuppressWarnings</span> annotations and <span style="font-family:monospace">-Xlint:-foo</span> flags. Of course, those
PR's can be
evaluated
independently
from this
proposal.<br>
<div>
<div><br>
</div>
<div>
<div>(You may
wonder: How
did all those
useless
suppressions
get in there?
See <a href="https://github.com/openjdk/jdk/pull/21853#issuecomment-2462566874" target="_blank" moz-do-not-send="true">this PR comment</a>.)<br>
<br>
</div>
<div>I played
around with a
couple of
different API
designs. The
API design is
key to
ensuring we
avoid various
annoying
inconsistencies
that can
easily occur;
a worst case
scenario is a
<span style="font-family:monospace">foo</span> warning that gets reported
somewhere, but
then when you
add the <span style="font-family:monospace">@SuppressWarnings("foo")</span> annotation
to suppress
it, the
annotation is
reported as
unnecessary -
a catch-22. So
I tried to
design &
document the
API to make it
easy for
compiler
developers to
avoid
inconsistencies
(regression
tests also
contribute to
this effort).</div>
<div><br>
</div>
<div>
<div>The key
challenges as
you might
guess are:</div>
<div>
<ul>
<li>Ensuring
warning
detection
logic is no
longer skipped
when a
category is
suppressed if
<span style="font-family:monospace">suppression</span> is enabled (easy)<br>
</li>
<li>Ensuring
that anywhere
a warning is
detected but
isn't reported
because the
category is
suppressed,
the
suppression is
still
validated
(harder)<br>
</li>
</ul>
</div>
<div>
<div>Summary
of internal
compiler
changes:<br>
</div>
<div>
<ul>
<li><span style="font-family:monospace">Lint</span> now keeps track of the current
symbol "in
scope" - this
is whatever
symbol was
last used for
<span style="font-family:monospace">Lint.augment()</span>. Validations are
tracked
against these
symbols, or
null for the
global scope.</li>
<li>A new
singleton <span style="font-family:monospace">LintSuppression</span> is responsible for
maintaining
this tracking
information on
a per-symbol
and
per-category
basis, and
generating
warnings as
needed when
the time
comes.</li>
<li>A new
method <span style="font-family:monospace">Lint.isActive()</span> answers the
question
"Should I
bother doing
some
non-trivial
calculation
that might or
might not
generate a
warning?" It
returns true
if the
category is
enabled OR if
it's
suppressed but
subject to
suppression
tracking and
the current
suppression in
scope has not
yet been
validated.
This is
entirely
optional and
usually not
needed. An
obvious
example:
before
invoking <span style="font-family:monospace">Check.checkSerialStructure()</span>.</li>
<li>A new
method <span style="font-family:monospace">Lint.validate()</span> means "If this lint
category is
currently
suppressed,
then validate
that
suppression".
In other
words, you are
saying that a
warning would
be generated
here.<br>
</li>
<li>A new
method <span style="font-family:monospace">Lint.emit()</span> simplifies the logic
when a lint
warning is
detected:</li>
<ul>
<li>If the
category is
enabled, it
logs the
message<br>
</li>
<li>If the
category is
suppressed, it
validates the
suppression<br>
</li>
</ul>
</ul>
<div>So code
that looked
like this:</div>
<div><br>
</div>
<div style="margin-left:40px"><span style="font-family:monospace">if
(lint.isEnabled(LintCategory.FOO))
{<br>
</span></div>
<div style="margin-left:40px"><span style="font-family:monospace">
log.warning(LintCategory.FOO,
pos,
SomeWarning</span><span style="font-family:monospace">(x, y)</span><span style="font-family:monospace">);</span></div>
<div style="margin-left:40px"><span style="font-family:monospace">}<br>
</span></div>
<div><br>
</div>
<div>can be
simplified to
this:</div>
<div><br>
</div>
<div>
<div style="margin-left:40px"><span style="font-family:monospace">lint.emit</span><span style="font-family:monospace">(log, LintCategory.FOO, pos,
SomeWarning(x,
y));</span></div>
<div><br>
</div>
</div>
<div>A minor
downside of
that
simplification
is that the <span style="font-family:monospace">Warning</span> object is constructed even
if the warning
is suppressed.
The upside is
that
suppression
validation
happens
automatically.
Since warnings
are relatively
rare, I felt
this was a
worthwhile
trade-off, but
it's not
forced on
people - you
can always do
this instead:<br>
</div>
</div>
<div><br>
</div>
<div>
<div style="margin-left:40px"><span style="font-family:monospace">if
(lint.validate(</span><span style="font-family:monospace">LintCategory.FOO</span><span style="font-family:monospace">).isEnabled(LintCategory.FOO)) {<br>
</span></div>
<div style="margin-left:40px"><span style="font-family:monospace">
log.warning(LintCategory.FOO,
pos,
SomeWarning</span><span style="font-family:monospace">(x, y)</span><span style="font-family:monospace">);</span></div>
<div style="margin-left:40px"><span style="font-family:monospace">}<br>
</span></div>
<div><br>
When we're
ready to
report on
unnecessary
suppressions
in a file, we
scan the file
for <span style="font-family:monospace">@SuppressWarnings</span> (and <span style="font-family:monospace">@Deprecated</span>) annotations, then look
at the
validatations
of the
corresponding
symbol
declarations,
and do the
"propagation"
step where all
the
validations
bubble up. Any
suppressions
that aren't
validated are
then reported
as
unnecessary. A
similar thing
happens at the
global scope
to generate
the <span style="font-family:monospace">suppression-option</span> warnings, using
validations
that escape
individual
source files,
at the end of
the overall
compilation.<br>
</div>
</div>
<br>
<div>There
were two
tricky
refactorings:
The <span style="font-family:monospace">overloads</span> warning reports when two
methods are
ambiguous when
called with
lambdas, but
the warning
itself has the
property that
a <span style="font-family:monospace">@SuppressWarnings("overloads")</span>
annotation on
<i>either</i>
of two such
methods
suffices to
suppress the
warning. So we
have to be
careful with
the logic,
e.g., if both
methods have
the
annotation, we
don't want to
randomly
validate one
of them and
then declare
the other as
unnecessary,
etc. To avoid
this, both
annotations
are validated
simultaneously.</div>
<div><br>
</div>
<div>The other
is the
"this-escape"
analyzer. When
a constructor
invokes <span style="font-family:monospace">this()</span> or a method, control flow
jumps to that
constructor or
method; when
it executes <span style="font-family:monospace">super()</span>, control flow jumps to all
the field
initializers
and non-static
initializer
blocks. This
jumping around
conflicts with
the AST
tree-based
scoping of <span style="font-family:monospace">@SuppressWarnings</span> annotations. We
apply "fixups"
so the
suppression
effect follows
the control
flow, not the
AST. This is
how it already
worked, but
the code had
to be updated
to validate
properly.<br>
</div>
<div><br>
</div>
<div>What
about <span style="font-family:monospace">DeferredLintHandler</span> and <span style="font-family:monospace">MandatoryWarningHandler</span>? These were
not really an
issue; all you
need is a
handle on the
correct <span style="font-family:monospace">Lint</span> instance and one is always
available.<br>
</div>
<div><br>
</div>
<div>The
prototype is
available
here: <a href="https://github.com/archiecobbs/jdk/tree/suppression" target="_blank" moz-do-not-send="true" class="moz-txt-link-freetext">https://github.com/archiecobbs/jdk/tree/suppression</a><br>
</div>
<div><br>
</div>
<div>This
prototype
patch is a
little
unwieldy
because it
includes:</div>
<div>
<ul>
<li>Compiler
changes to
support the
new lint
categories (in
the diff
starting with
<span style="font-family:monospace">Lint.java</span>)<br>
</li>
<li>Removal of
400+ <span style="font-family:monospace">@SuppressWarnings </span>annotations to
continue to
allow the use
of <span style="font-family:monospace">-Xlint:all</span> everywhere (<a href="https://github.com/archiecobbs/jdk/actions/runs/11728281642" target="_blank" moz-do-not-send="true">build logs</a>)<br>
</li>
<li>Several
build-related
cleanups,
e.g., adding <span style="font-family:monospace">-Xlint:-suppression-option</span> to
unbreak
incremental
builds</li>
<li>Temporary
build
workaround for
JDK-8340341</li>
</ul>
</div>
<div>
<div>I'm
interested in
any opinions
and/or folks
who have large
bodies of code
or specific
test cases
they would
like to run
this on.<br>
</div>
</div>
<div><br>
</div>
<div>Thanks,<br>
</div>
<div>-Archie<br>
</div>
<div><b><br>
</b></div>
<span class="gmail_signature_prefix">-- </span><br>
<div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature">Archie L. Cobbs<br>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
</body>
</html>