On Mon, 23 Jan 2023 19:34:42 GMT, Archie L. Cobbs wrote:
> [The CSR](https://bugs.openjdk.org/browse/JDK-825) also needs a review
> from a compiler-dev engineer as well if anyone is interested... thanks.
some comments on the CSR:
- I think the value for field `Compatibility Risk` could be
On Wed, 18 Jan 2023 22:27:08 GMT, Archie L. Cobbs wrote:
>> 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 gene
On Wed, 18 Jan 2023 22:27:08 GMT, Archie L. Cobbs wrote:
>> 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 gene
On Wed, 18 Jan 2023 22:27:08 GMT, Archie L. Cobbs wrote:
>> 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 gene
On Wed, 18 Jan 2023 22:27:08 GMT, Archie L. Cobbs wrote:
>> 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 gene
On Wed, 18 Jan 2023 22:27:08 GMT, Archie L. Cobbs wrote:
>> 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 gene
> 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
On Tue, 17 Jan 2023 23:59:04 GMT, Archie L. Cobbs wrote:
>> 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 gene
> 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
On Tue, 17 Jan 2023 15:53:57 GMT, Brian Goetz wrote:
> I agree with this conclusion. Also, even if a class is public, our question
> should be: is this class in a non-exported package? Because, if so, even a
> public class can be "implementation specific".
Agreed - I originally thought this wa
> 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
On Tue, 17 Jan 2023 15:53:57 GMT, Brian Goetz wrote:
> A more complete analysis would analyze the base class + possible subclass
> combinations, and could exclude some escapes which are harmless (i.e., the
> base class passes `this` to an overridable method, but no overrides make use
> of it,
On Tue, 17 Jan 2023 18:09:51 GMT, Vicente Romero wrote:
>> Archie L. Cobbs has updated the pull request incrementally with three
>> additional commits since the last revision:
>>
>> - Remove unused type variable on method visitScoped().
>> - Remove expression type filtering; it doesn't seem t
On Tue, 17 Jan 2023 16:15:33 GMT, Jan Lahoda wrote:
>> Archie L. Cobbs has updated the pull request incrementally with three
>> additional commits since the last revision:
>>
>> - Remove unused type variable on method visitScoped().
>> - Remove expression type filtering; it doesn't seem to be
On Mon, 16 Jan 2023 23:22:00 GMT, Archie L. Cobbs wrote:
>> 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 gene
On Mon, 16 Jan 2023 13:30:57 GMT, Jan Lahoda wrote:
>> Archie L. Cobbs has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Fix bug where field initializer warnings could be incorrectly suppressed.
>> - Consolidate all the unit tests that
On Mon, 16 Jan 2023 13:30:11 GMT, Jan Lahoda wrote:
>> Archie L. Cobbs has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Fix bug where field initializer warnings could be incorrectly suppressed.
>> - Consolidate all the unit tests that
On Fri, 13 Jan 2023 22:48:59 GMT, Archie L. Cobbs wrote:
>> 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 gene
On Mon, 16 Jan 2023 23:22:00 GMT, Archie L. Cobbs wrote:
>> 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 gene
On Mon, 16 Jan 2023 23:22:00 GMT, Archie L. Cobbs wrote:
>> 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 gene
On Tue, 17 Jan 2023 15:36:49 GMT, Archie L. Cobbs wrote:
> nstead, we could widen the maintenance boundary to the package/module
> boundary, and instead ask: Is X public, not final, not sealed, and does X
> have a non-private constructor?
>
> This is not a perfect drawing of that boundary, bec
On Tue, 17 Jan 2023 11:44:22 GMT, Maurizio Cimadamore
wrote:
> Yes, other clients might be able to access the hashmap, and observe a
> possibly uninitialized value - but this is in the same bucket as the factory
> example I gave last week, where a private constructor (called by public
> facto
On Tue, 17 Jan 2023 11:23:24 GMT, Maurizio Cimadamore
wrote:
>> Archie L. Cobbs has updated the pull request incrementally with three
>> additional commits since the last revision:
>>
>> - Remove unused type variable on method visitScoped().
>> - Remove expression type filtering; it doesn't
On Tue, 17 Jan 2023 11:23:07 GMT, Maurizio Cimadamore
wrote:
>> Archie L. Cobbs has updated the pull request incrementally with three
>> additional commits since the last revision:
>>
>> - Remove unused type variable on method visitScoped().
>> - Remove expression type filtering; it doesn't
On Mon, 16 Jan 2023 23:22:00 GMT, Archie L. Cobbs wrote:
>> 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 gene
On Mon, 16 Jan 2023 23:22:00 GMT, Archie L. Cobbs wrote:
>> 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 gene
On Mon, 16 Jan 2023 23:22:00 GMT, Archie L. Cobbs wrote:
>> 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 gene
On Sat, 14 Jan 2023 01:54:23 GMT, Archie L. Cobbs wrote:
>>> Ok - I thought false negative was the thing to absolutely avoid - and that
>>> was the no. 1 concern.
>>
>> You're right. I think at the time I reasoned that it would be unusual enough
>> for the type of an expression to start as an
> 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
On Mon, 16 Jan 2023 16:29:31 GMT, Archie L. Cobbs wrote:
> It looks like you might be counting the "here via invocation" lines as
> separate warnings. These are really part of the previous `possible 'this'
> escape` line, e.g.:
yes, I really did the simplest possible thing (e.g. just counting
On Mon, 16 Jan 2023 12:51:49 GMT, Maurizio Cimadamore
wrote:
>> The number of times around any single loop is bounded by the number of new
>> references that can possibly be created during the analysis of that loop.
>>
>> That number is at most 2 * (1 + 1 + 1 + 1 + N) where N is the number of
On Mon, 16 Jan 2023 11:53:40 GMT, Maurizio Cimadamore
wrote:
>> Archie L. Cobbs has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Fix bug where field initializer warnings could be incorrectly suppressed.
>> - Consolidate all the unit t
On Fri, 13 Jan 2023 21:28:51 GMT, Archie L. Cobbs wrote:
>> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ThisEscapeAnalyzer.java
>> line 1088:
>>
>>> 1086: private void visitLooped(T tree, Consumer
>>> visitor) {
>>> 1087: visitScoped(tree, false, t -> {
>>> 1088:
On Fri, 13 Jan 2023 22:48:59 GMT, Archie L. Cobbs wrote:
>> 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 gene
On Fri, 13 Jan 2023 00:57:14 GMT, Archie L. Cobbs wrote:
>> Ok - I thought false negative was the thing to absolutely avoid - and that
>> was the no. 1 concern. I think a possible approach to keep both the
>> filtering and the code more or less similar to what you have, is to save the
>> type
> 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
On Fri, 13 Jan 2023 21:33:02 GMT, Archie L. Cobbs wrote:
>> Sounds good - thanks.
>
> Ah. I just realized that we need to do it your way because of the following
> bug:
>
> Suppose you have a constructor and a field with initializer that both leak,
> but you have `@SuppressWarnings("this-escap
On Fri, 13 Jan 2023 16:20:41 GMT, Archie L. Cobbs wrote:
>> I'm OK either way we can revisit this later either as part of this PR or in
>> a future one. I let it to your consideration
>
> Sounds good - thanks.
Ah. I just realized that we need to do it your way because of the following bug:
Sup
On Fri, 13 Jan 2023 20:21:24 GMT, Vicente Romero wrote:
>> Archie L. Cobbs has updated the pull request incrementally with three
>> additional commits since the last revision:
>>
>> - Use more idiomatic test for java.lang.Object.
>> - Revert 27cb30129; the error was actually fixed in edf3c3f5
On Fri, 13 Jan 2023 20:16:25 GMT, Vicente Romero wrote:
>> Archie L. Cobbs has updated the pull request incrementally with three
>> additional commits since the last revision:
>>
>> - Use more idiomatic test for java.lang.Object.
>> - Revert 27cb30129; the error was actually fixed in edf3c3f5
On Fri, 13 Jan 2023 19:09:54 GMT, Archie L. Cobbs wrote:
>> 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 gene
On Fri, 13 Jan 2023 19:09:54 GMT, Archie L. Cobbs wrote:
>> 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 gene
On Fri, 13 Jan 2023 17:49:05 GMT, Archie L. Cobbs wrote:
>> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ThisEscapeAnalyzer.java
>> line 685:
>>
>>> 683:
>>> 684: @Override
>>> 685: public void visitDoLoop(JCDoWhileLoop tree) {
>>
>> I was thinking, code can also loop using
> 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
On Fri, 13 Jan 2023 17:35:08 GMT, Vicente Romero wrote:
>> 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 r
On Fri, 13 Jan 2023 04:04:36 GMT, Archie L. Cobbs wrote:
>> 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 gene
On Fri, 13 Jan 2023 16:12:50 GMT, Vicente Romero wrote:
>> Yes... I did it that way is so that it doesn't require any adaptation
>> if/when JDK-8194743 ever gets implemented. And it keeps the code a little
>> simpler in exchange for a little redundancy.
>>
>> I'm happy to fix this if you think
On Fri, 13 Jan 2023 16:06:04 GMT, Maurizio Cimadamore
wrote:
>>> Something seems to be up with the lint description for this-escape -
>>> compare this:
>>
>> Oops, will fix - thanks.
>
>> The decision was to go with drawing the "warning boundary" at the
>> compilation unit. The reasoning is t
On Fri, 13 Jan 2023 15:14:05 GMT, Archie L. Cobbs wrote:
>> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ThisEscapeAnalyzer.java
>> line 516:
>>
>>> 514: Name name = TreeInfo.name(invoke.meth);
>>> 515: if (name == names._super) {
>>> 516: scanInitializers
On Fri, 13 Jan 2023 15:08:59 GMT, Archie L. Cobbs wrote:
>>> I guess I was confused because, while subclasses are a particularly sneaky
>>> case where uninitialized values can show up, the above leak seems
>>> potentially dangerous as well...
>>
>> Yes - and this very question did come up in t
On Fri, 13 Jan 2023 12:42:24 GMT, Vicente Romero wrote:
>> 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 r
On Fri, 13 Jan 2023 15:08:43 GMT, Archie L. Cobbs wrote:
>> Something seems to be up with the lint description for this-escape - compare
>> this:
>>
>>
>> serial Warn about Serializable classes that do not have a
>> serialVersionUID field.
>> Also war
On Fri, 13 Jan 2023 11:08:33 GMT, Maurizio Cimadamore
wrote:
>> Perhaps my confusion might come from the name `this-escape` of the lint
>> warning - which seems overpromising in this respect. But I looked at the
>> description of the lint warning using `javac --help-lint` and I got this:
>>
>
On Fri, 13 Jan 2023 04:04:36 GMT, Archie L. Cobbs wrote:
>> 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 gene
On Fri, 13 Jan 2023 04:04:36 GMT, Archie L. Cobbs wrote:
>> 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 gene
On Fri, 13 Jan 2023 11:05:51 GMT, Maurizio Cimadamore
wrote:
>> So, to clarify, in this case:
>>
>>
>> import java.util.*;
>>
>> class B {
>> final Object ref;
>>
>> private B(Object ref) {
>> Foo.consume(this);
>> this.ref = ref;
>> }
>> }
>>
>>
>> Even
On Fri, 13 Jan 2023 10:58:33 GMT, Maurizio Cimadamore
wrote:
>> Caring about the proper initialization of any class in the _current_
>> compilation unit is an explicit non-goal.
>>
>> We only care about bugs where a superclass and subclass are in separate
>> compilation units.
>
> So, to clar
On Thu, 12 Jan 2023 21:04:09 GMT, Archie L. Cobbs wrote:
>> but what if `m` is a static method in a separate compilation unit? Should it
>> be able to observe a partially initialized Foo?
>
> Caring about the proper initialization of any class in the _current_
> compilation unit is an explicit
On Thu, 12 Jan 2023 21:47:28 GMT, Maurizio Cimadamore
wrote:
> Ok - I thought false negative was the thing to absolutely avoid - and that
> was the no. 1 concern.
You're right. I think at the time I reasoned that it would be unusual enough
for the type of an expression to start as an instanc
On Thu, 12 Jan 2023 19:12:27 GMT, Archie L. Cobbs wrote:
>> Uhm. Turns out I probably did not understand the filter correctly, and now
>> I'm more dubious about what it actually does. Say you have this hierarchy:
>>
>>
>> interface A { }
>> class B {
>> B() {
>> A a = (A)this;
>
> 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
On Thu, 12 Jan 2023 21:28:12 GMT, Archie L. Cobbs wrote:
>> My point is about who puts ThisRef in the set to begin with. It seems to me
>> that ThisRef is put there at the start of a method analysis. After which,
>> there's several code points where we say "if there's a direct ThisRef in the
>
On Thu, 12 Jan 2023 19:01:10 GMT, Maurizio Cimadamore
wrote:
>> The code you quoted has nothing specifically to do with method invocations.
>> This code is simply handing the evaluation of the expressions `this` and
>> `super`. For example, `this` could just be a parameter we're passing to
>>
On Thu, 12 Jan 2023 19:24:50 GMT, Archie L. Cobbs wrote:
>> This patch passes all tests:
>>
>>
>> diff --git
>> a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ThisEscapeAnalyzer.java
>>
>> b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ThisEscapeAnalyzer.java
>> index 9
On Thu, 12 Jan 2023 18:48:25 GMT, Maurizio Cimadamore
wrote:
>>> I guess what I'm thinking about:
>>
>> No leak is possible in that example.
>> * `new Foo()` creates an instance of `Foo` (not a subclass of `Foo`)
>> therefore `m()` is not overridden
>> * Any subclass of `Foo` that may exist in
On Thu, 12 Jan 2023 18:40:38 GMT, Maurizio Cimadamore
wrote:
>> This patch:
>>
>>
>> diff --git a/make/test/BuildMicrobenchmark.gmk
>> b/make/test/BuildMicrobenchmark.gmk
>> index 1c89328a388..7c3f0293edc 100644
>> --- a/make/test/BuildMicrobenchmark.gmk
>> +++ b/make/test/BuildMicrobenchmark
On Thu, 12 Jan 2023 17:40:36 GMT, Maurizio Cimadamore
wrote:
> But the filtering will end up dropping the expression Ref on the floor,
> right? (because B and A are unrelated).
Ah, I see what you mean.
Here's a more complete example:
public class CastLeak {
public CastLeak() {
(
On Thu, 12 Jan 2023 17:33:48 GMT, Archie L. Cobbs wrote:
>> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ThisEscapeAnalyzer.java
>> line 875:
>>
>>> 873: // Reference to this?
>>> 874: if (tree.name == names._this || tree.name == names._super) {
>>> 875: i
On Thu, 12 Jan 2023 17:29:22 GMT, Maurizio Cimadamore
wrote:
>> I put it there because of switch expressions and `yeild`... ?
>
> Well, yield can... yield a value - `case` doesn't. So I'm confused. Also
> because the variable is called `referenceExpressionNode` and `CASE` is not an
> expressio
On Thu, 12 Jan 2023 18:37:06 GMT, Archie L. Cobbs wrote:
>> I guess what I'm thinking about:
>>
>> class Foo {
>> private Foo() {
>> m(this);
>> }
>>
>> public void m() { ... } // overridable
>>
>> static Foo makeFoo() { return new Foo(); }
>> }
>
>> I guess wha
On Thu, 12 Jan 2023 18:18:38 GMT, Maurizio Cimadamore
wrote:
>> I can't seem to be able to run tests - I get failures in the build:
>>
>>
>> * For target support_test_micro_tools-classes__the.BUILD_INDIFY_batch:
>
> This patch:
>
>
> diff --git a/make/test/BuildMicrobenchmark.gmk
> b/make/t
On Thu, 12 Jan 2023 16:40:33 GMT, Maurizio Cimadamore
wrote:
> I guess what I'm thinking about:
No leak is possible in that example.
* `new Foo()` creates an instance of `Foo` (not a subclass of `Foo`) therefore
`m()` is not overridden
* Any subclass of `Foo` that may exist in the outside worl
On Thu, 12 Jan 2023 18:11:01 GMT, Maurizio Cimadamore
wrote:
>> Same comment as previous: I don't quite know what I'm doing and I'm loathe
>> to break what is already working. Do you have a suggested patch?
>
> I can't seem to be able to run tests - I get failures in the build:
>
>
> * For ta
On Thu, 12 Jan 2023 17:39:05 GMT, Vicente Romero wrote:
>> 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
On Thu, 12 Jan 2023 12:28:12 GMT, Maurizio Cimadamore
wrote:
> This might also be related with the fact that we deal with return values in
> different ways than with e.g. values returned from a nested scope (where we
> just pop, and then copy all pending expression to the outer depth).
Yes, a
On Thu, 12 Jan 2023 17:48:37 GMT, Archie L. Cobbs wrote:
>> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ThisEscapeAnalyzer.java
>> line 909:
>>
>>> 907:
>>> 908: // Check for implicit outer 'this' reference
>>> 909: if (this.types.hasOuterClass(currentClassT
On Thu, 12 Jan 2023 12:26:27 GMT, Maurizio Cimadamore
wrote:
> Do we really need a set for this?
There are surely other ways to model things. But I got myself really confused
trying to build more complicated models.
What I ended up with is this simple model that works:
* There is a set of `Re
On Thu, 12 Jan 2023 12:17:32 GMT, Maurizio Cimadamore
wrote:
> There is a concept of push/popScope and then there's a separate concept of
> call stack (which is just a list of diagnostic position up to the point). I
> wonder if this could be better modeled by using a single class e.g.
> Scope
On Thu, 12 Jan 2023 12:15:17 GMT, Maurizio Cimadamore
wrote:
>> 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 clarif
On Wed, 11 Jan 2023 03:30:03 GMT, Archie L. Cobbs wrote:
>> 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 gene
On Thu, 12 Jan 2023 17:13:55 GMT, Archie L. Cobbs wrote:
>> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ThisEscapeAnalyzer.java
>> line 411:
>>
>>> 409: final boolean referenceExpressionNode;
>>> 410: switch (tree.getTag()) {
>>> 411: case CASE:
>>
>> surpri
On Thu, 12 Jan 2023 11:09:35 GMT, Maurizio Cimadamore
wrote:
>> 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 clarif
On Thu, 12 Jan 2023 10:56:53 GMT, Maurizio Cimadamore
wrote:
>> 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 clarif
On Thu, 12 Jan 2023 10:48:49 GMT, Maurizio Cimadamore
wrote:
>> 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 clarif
On Thu, 12 Jan 2023 16:20:12 GMT, Archie L. Cobbs wrote:
>> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ThisEscapeAnalyzer.java
>> line 218:
>>
>>> 216: new TreeScanner() {
>>> 217:
>>> 218: private Lint lint = ThisEscapeAnalyzer.this.lint;
>>
>> On a first loo
On Thu, 12 Jan 2023 10:32:19 GMT, Maurizio Cimadamore
wrote:
> If we have a class with a private constructor and public static factory
> invoking said constructor, and the constructor makes this escape, isn't that
> an issue we should detect?
A static factory method will not create a subclass
On Thu, 12 Jan 2023 10:25:27 GMT, Maurizio Cimadamore
wrote:
>> 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 clarif
On Thu, 12 Jan 2023 10:18:27 GMT, Maurizio Cimadamore
wrote:
>> 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 clarif
On Thu, 12 Jan 2023 15:10:19 GMT, Maurizio Cimadamore
wrote:
> Interesting example - I thought you might have been referring to a case where
> the class being analyzed was itself an exception.
Yes - although that example doesn't compile (oops!). Just replace `catch
(RuntimeException e)` with
On Thu, 12 Jan 2023 13:01:44 GMT, Maurizio Cimadamore
wrote:
>> 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 clarif
On Thu, 12 Jan 2023 14:59:12 GMT, Archie L. Cobbs wrote:
>>> * On the Java stack
>>> (a) The current 'this' instance
>>> (b) A method parameter
>>> (c) A local variable
>>> (d) A temporary value that is part of the current expression being
>>> evaluated
>>> (e) The return value from a
On Thu, 12 Jan 2023 09:57:00 GMT, Maurizio Cimadamore
wrote:
> I'm not sure what you mean by (1f). You mean this can be embedded in an
> exception being thrown? Is that different from (2)?
Yes, this would be a different case from any other that you'd have to handle in
the code if you wanted t
On Wed, 11 Jan 2023 03:30:03 GMT, Archie L. Cobbs wrote:
>> 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 gene
On Wed, 11 Jan 2023 03:30:03 GMT, Archie L. Cobbs wrote:
>> 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 gene
On Wed, 11 Jan 2023 03:30:03 GMT, Archie L. Cobbs wrote:
>> 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 gene
On Thu, 12 Jan 2023 02:14:10 GMT, Archie L. Cobbs wrote:
>>>
>>> D'oh, you're right. But if you made `returnMe()` static or private then the
>>> argument would still hold.
>>>
>>> > And, if the method is static, same story - you are passing ref3 somewhere
>>> > else, and ref3 potentially cont
On Thu, 12 Jan 2023 00:15:08 GMT, Maurizio Cimadamore
wrote:
> So, for static methods, it could go down two ways: either we don't even look
> at referenced method bodies, give up and just declare "sorry, escaping". Or,
> if we look into method bodies, and see that the relationship between inne
On Wed, 11 Jan 2023 22:40:43 GMT, Archie L. Cobbs wrote:
>
> D'oh, you're right. But if you made `returnMe()` static or private then the
> argument would still hold.
>
> > And, if the method is static, same story - you are passing ref3 somewhere
> > else, and ref3 potentially contains this.
>
On Wed, 11 Jan 2023 21:51:45 GMT, Maurizio Cimadamore
wrote:
> So, in this example though you are calling an instance method before the
> object is initialized, which would seem to me like a leak
D'oh, you're right. But if you made `returnMe()` static or private then the
argument would still
On Wed, 11 Jan 2023 21:45:20 GMT, Maurizio Cimadamore
wrote:
>> Regarding the assignment graph approach, I think that would work if the
>> references are bouncing around strictly between variables, but what if the
>> chain includes any of the more complicated stuff that is currently being
>>
1 - 100 of 135 matches
Mail list logo
