> 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")`
Archie L. Cobbs has updated the pull request incrementally with two additional commits since the last revision: - Use the more appropriate Type comparison method Types.isSameType(). - Add some more comments to clarify how the analysis works. ------------- Changes: - all: https://git.openjdk.org/jdk/pull/11874/files - new: https://git.openjdk.org/jdk/pull/11874/files/d70d12f4..6e96a7d7 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=11874&range=06 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=11874&range=05-06 Stats: 31 lines in 2 files changed: 16 ins; 10 del; 5 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
