This PR adds a new lint warning category `this-escape`.

It also adds `@SuppressWarnings` annotations as needed to the JDK itself to 
allow the JDK to continue to compile with `-Xlint:all`.

A 'this' escape warning is generated for a constructor `A()` in a class `A` 
when the compiler detects that the following situation is _in theory possible:_
* Some subclass `B extends A` exists, and `B` is defined in a separate source 
file (i.e., compilation unit)
* Some constructor `B()` of `B` invokes `A()` as its superclass constructor
* During the execution of `A()`, some non-static method of `B.foo()` could get 
invoked, perhaps indirectly

In the above scenario, `B.foo()` would execute before `A()` has returned and 
before `B()` has performed any initialization. To the extent `B.foo()` accesses 
any fields of `B` - all of which are still uninitialized - it is likely to 
function incorrectly.

Note, when determining if a 'this' escape is possible, the compiler makes no 
assumptions about code outside of the current compilation unit. It doesn't look 
outside of the current source file to see what might actually happen when a 
method is invoked. It does follow method and constructors within the current 
compilation unit, and applies a simplified union-of-all-possible-branches data 
flow analysis to see where 'this' could end up.

>From my review, virtually all of the warnings generated in the various JDK 
>modules are valid warnings in the sense that a 'this' escape, as defined 
>above, is really and truly possible. However, that doesn't imply that any bugs 
>were found within the JDK - only that the possibility of a certain type of bug 
>exists if certain superclass constructors are used by someone, somewhere, 
>someday.

For several "popular" classes, this PR also adds `@implNote`'s to the offending 
constructors so that subclass implementors are made aware of the threat. For 
one example, `TreeMap(Map)` invokes `putAll()` and `put()`.

More details and a couple of motivating examples are given in an included [doc 
file](https://github.com/archiecobbs/jdk/blob/ThisEscape/src/java.base/share/classes/java/lang/doc-files/ThisEscape.html)
 that these `@implNote`'s link to. See also the recent thread on `amber-dev` 
for some background.

Ideally, over time the owners of the various modules would review their 
`@SuppressWarnings("this-escape")` annotations and determine which other 
constructors also warranted such an `@implNote`.

Because of all the`@SuppressWarnings` annotations, this PR touches a bunch of 
different JDK modules. My apologies for that. Adding these annotations was 
determined to be the more conservative approach, as compared to just excepting 
`this-escape` from various module builds globally.

**Patch Navigation Guide**

* Non-trivial compiler changes:
    * `src/jdk.compiler/share/classes/com/sun/tools/javac/code/Lint.java`
    * `src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java`
    * `src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java`
    * `src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeInfo.java`
    * 
`src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ThisEscapeAnalyzer.java`
    * 
`src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties`
    * 
`src/jdk.compiler/share/classes/com/sun/tools/javac/resources/javac.properties`

* Javadoc additions of `@implNote`:

    * `src/java.base/share/classes/java/io/PipedReader.java`
    * `src/java.base/share/classes/java/io/PipedWriter.java`
    * `src/java.base/share/classes/java/lang/Throwable.java`
    * `src/java.base/share/classes/java/util/ArrayDeque.java`
    * `src/java.base/share/classes/java/util/EnumMap.java`
    * `src/java.base/share/classes/java/util/HashSet.java`
    * `src/java.base/share/classes/java/util/Hashtable.java`
    * `src/java.base/share/classes/java/util/LinkedList.java`
    * `src/java.base/share/classes/java/util/TreeMap.java`
    * `src/java.base/share/classes/java/util/TreeSet.java`

* New unit tests
    * `test/langtools/tools/javac/warnings/ThisEscape/*.java`

* **Everything else** is just adding `@SuppressWarnings("this-escape")`

-------------

Commit messages:
 - Remove trailing whitespace.
 - Some documentation tweaks.
 - Treat method references like the equivalent lambda.
 - Fix bug where initializers could generate duplicate warnings.
 - Javadoc fix.
 - Add ThisEscape.html doc note and link the new @implNote's to it.
 - Add more @SuppressWarnings("this-escape") annotations.
 - Add more @SuppressWarnings("this-escape") annotations.
 - Add more @SuppressWarnings("this-escape") annotations.
 - Add more @SuppressWarnings("this-escape") annotations.
 - ... and 38 more: https://git.openjdk.org/jdk/compare/c6588d5b...9c162283

Changes: https://git.openjdk.org/jdk/pull/11874/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=11874&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8015831
  Stats: 4326 lines in 1285 files changed: 4259 ins; 3 del; 64 mod
  Patch: https://git.openjdk.org/jdk/pull/11874.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/11874/head:pull/11874

PR: https://git.openjdk.org/jdk/pull/11874

Reply via email to