> 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 16 additional commits since the last revision: - Fix bug where all but the last yeild statement were being ignored. - Add method RefSet.mapInto() and use to refactor/clean up. - Fix possible assertion failure when handling if statements. - Use Symbol methods isSubClass() and isEnclosedBy() instead of homebrew stuff. Suggested-by: mcimadamore - Add comment regarding limitations of expresison type filtering. - Add a few more DISABLED_WARNINGS to unbreak build. - Clean up handling of switch expressions a bit. - Remove unused method variant of analyzeTree(). - Avoid all caps in comments. - Clarify confusing comment. - ... and 6 more: https://git.openjdk.org/jdk/compare/6e96a7d7...edf3c3f5 ------------- Changes: - all: https://git.openjdk.org/jdk/pull/11874/files - new: https://git.openjdk.org/jdk/pull/11874/files/6e96a7d7..edf3c3f5 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=11874&range=07 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=11874&range=06-07 Stats: 540 lines in 8 files changed: 120 ins; 92 del; 328 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
