<!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>