On Thu, 13 Apr 2023 12:07:00 GMT, Jan Lahoda wrote:
>> Note that currently, a `switch` over an enum will already trigger
>> initialisation of the enum class because of the need to compute the relevant
>> mapping array by the synthetic helper class, see [JDK‑7176515] for details.
>> - https://gi
On Fri, 7 Apr 2023 17:56:30 GMT, Alan Bateman wrote:
>> Archie L. Cobbs has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Apply Javadoc improvements suggested in review.
>
> src/java.base/share/classes/jav
avadoc so subclass implementers are made aware
>
> This patch takes the second, more conservative approach.
Archie L. Cobbs has updated the pull request incrementally with one additional
commit since the last revision:
Address review comments.
-
Changes:
- al
On Fri, 7 Apr 2023 16:09:06 GMT, Alan Bateman wrote:
> Perhaps the issue summary / PR title should be more like this?
I'm not so sure. What you suggest describes this fix for this bug, but it
doesn't really describe the bug itself - instead, the bug argues that the
reentrant behavior is wrong
On Thu, 6 Apr 2023 22:36:33 GMT, Archie L. Cobbs wrote:
> IO stream classes like `FileOutputStream` can have assocated NIO channels.
>
> When `close()` is invoked on one of these classes, it in turn invokes
> `close()` on the associated channel (if any). But when the associated
avadoc so subclass implementers are made aware
>
> This patch takes the second, more conservative approach.
Archie L. Cobbs has updated the pull request incrementally with one additional
commit since the last revision:
Apply Javadoc improvements suggested in review.
-
IO stream classes like `FileOutputStream` can have assocated NIO channels.
When `close()` is invoked on one of these classes, it in turn invokes `close()`
on the associated channel (if any). But when the associated channel's `close()`
method is invoked, it in turn invokes `close()` on the associ
On Sat, 18 Mar 2023 17:46:00 GMT, Archie L. Cobbs wrote:
> The fix for JDK-8015831, which added the new `this-escape` lint warning,
> caused the build of the `bootcycle-images` make target to fail.
>
> This commit adds the additional `@SuppressWarnings("this-escape")`
&
The fix for JDK-8015831, which added the new `this-escape` lint warning, caused
the build of the `bootcycle-images` make target to fail.
This commit adds the additional `@SuppressWarnings("this-escape")` annotations
needed to fix it.
-
Commit messages:
- Add @SuppressWarnings("thi
On Thu, 23 Feb 2023 09:04:23 GMT, Justin Lu wrote:
> This PR converts Unicode sequences to UTF-8 native in .properties file.
> (Excluding the Unicode space and tab sequence). The conversion was done using
> native2ascii.
>
> In addition, the build logic is adjusted to support reading in the
>
On Sun, 19 Feb 2023 23:52:52 GMT, Archie L. Cobbs wrote:
> This bug relates to the "potentially ambiguous overload" warning which is
> enabled by `-Xlint:overloads`.
>
> The warning detects certain ambiguities that can cause problems for lambdas.
> For exampl
On Sat, 25 Feb 2023 13:38:05 GMT, Vicente Romero wrote:
>> Archie L. Cobbs has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Do some refactoring & cleanup suggested in reviews.
>
> src/jdk.compiler/shar
On Sat, 25 Feb 2023 04:05:29 GMT, Vicente Romero wrote:
>> Archie L. Cobbs has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Fix typo in one comment and clarify another.
>
> src/jdk.compiler/share/cl
gt; void foo(Consumer c);
> }
>
> public interface I1 extends SuperIface {
> void foo(IntConsumer c);// warning was generated here
> }
>
> public interface I2 extends SuperIface {
> void foo(IntConsumer c);// no warning was generated here
> }
&g
On Fri, 24 Feb 2023 22:24:10 GMT, Vicente Romero wrote:
>> Archie L. Cobbs has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Fix typo in one comment and clarify another.
>
> src/jdk.compiler/share/cl
gt; void foo(Consumer c);
> }
>
> public interface I1 extends SuperIface {
> void foo(IntConsumer c);// warning was generated here
> }
>
> public interface I2 extends SuperIface {
> void foo(IntConsumer c);// no warning was generated here
> }
&g
On Fri, 24 Feb 2023 19:49:18 GMT, Vicente Romero wrote:
>>> I think the code should be split a bit using helper methods
>>
>> OK - will do.
>>
>>> I'm a bit worried that overuse of streams in this code could imply some
>>> performance hit
>>
>> I asked basically the same question ([in a separ
On Fri, 24 Feb 2023 19:00:51 GMT, Vicente Romero wrote:
> I think the code should be split a bit using helper methods
OK - will do.
> I'm a bit worried that overuse of streams in this code could imply some
> performance hit
I asked basically the same question ([in a separate
thread](https://
On Fri, 24 Feb 2023 18:01:56 GMT, Vicente Romero wrote:
>> Archie L. Cobbs has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Remove placeholder flag UNUSED_1; just leave a comment instead.
>
> src/jdk.compil
gt; void foo(Consumer c);
> }
>
> public interface I1 extends SuperIface {
> void foo(IntConsumer c);// warning was generated here
> }
>
> public interface I2 extends SuperIface {
> void foo(IntConsumer c);// no warning was generated here
> }
&g
On Fri, 24 Feb 2023 11:49:55 GMT, Vicente Romero wrote:
> I think a CSR is needed as we will be generating more warnings and projects
> compiling with -Werror could see compilation errors
Will do - thanks.
> src/jdk.compiler/share/classes/com/sun/tools/javac/code/Flags.java line 280:
>
>> 278
gt; void foo(Consumer c);
> }
>
> public interface I1 extends SuperIface {
> void foo(IntConsumer c);// warning was generated here
> }
>
> public interface I2 extends SuperIface {
> void foo(IntConsumer c);// no warning was generated here
> }
&g
gt; void foo(Consumer c);
> }
>
> public interface I1 extends SuperIface {
> void foo(IntConsumer c);// warning was generated here
> }
>
> public interface I2 extends SuperIface {
> void foo(IntConsumer c);// no warning was generated here
> }
&g
On Mon, 20 Feb 2023 23:53:05 GMT, SWinxy wrote:
> In the `AWTEventMulticaster` class(es), which interfaces are causing the
> ambiguity?
Ah - my apologies. Those got added during development but were not needed once
all the bugs were worked out.
I've removed them and that will also eliminate `
This bug relates to the "potentially ambiguous overload" warning which is
enabled by `-Xlint:overloads`.
The warning detects certain ambiguities that can cause problems for lambdas.
For example, consider the interface `Spliterator.OfInt`, which declares these
two methods:
void forEachRemaining
re/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`
>
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.
>&g
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
re/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`
>
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 becau
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; t
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; t
re/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
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 meth
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 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 meth
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 fiel
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 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
re/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`
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/j
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: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 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 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: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 som
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().
>>
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().
>>
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().
>>
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().
>>
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().
>>
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().
>>
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().
>>
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 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 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 15:59:29 GMT, Maurizio Cimadamore
wrote:
> What I'm interested in though is what incremental improvement is brought by
> the more complex analysis you have in this PR?
It's a good question. Here are some thoughts...
One meta-goal is that this analysis be conservative. In o
On Wed, 11 Jan 2023 18:44:20 GMT, Archie L. Cobbs wrote:
>> Also, looking at the loop test more closely, it seems to me that what the
>> compiler needs to do is to prove that there can be possible paths by which a
>> `this` can land into ref4.
>>
>> If we build
On Wed, 11 Jan 2023 16:14:24 GMT, Maurizio Cimadamore
wrote:
>> True - probably 3 * 3 can be achieved if this:
>>
>>
>> ThisEscapeLoop ref21 = ref14;
>>
>> Is replaced with
>>
>>
>> ThisEscapeLoop ref21 = this;
>>
>>
>> In which case the inner loop won't converge immediately (a
On Tue, 10 Jan 2023 23:38:14 GMT, Maurizio Cimadamore
wrote:
>> OK I'm glad you pointed that out because I'm a little unclear on the best
>> way to do this bit.
>>
>> Just to confirm, you are saying that this:
>>
>> `if (erasure(type).equalsIgnoreMetadata(outerType)) {`
>>
>> should be repla
re/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`
>
On Wed, 11 Jan 2023 00:04:14 GMT, Maurizio Cimadamore
wrote:
>> Yes, because the 'this' reference can bounce around through different
>> variables in scope each time around the loop. So we have to repeat the loop
>> until all 'this' references have "flooded" into all the nooks and crannies.
>>
On Tue, 10 Jan 2023 23:45:59 GMT, Maurizio Cimadamore
wrote:
>> It's slightly different from that.
>>
>> Considering any of the various things in scope that can point to an object
>> (these are: the current 'this' instance, the current outer 'this' instance,
>> method parameter/variable, meth
re/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`
On Mon, 9 Jan 2023 14:23:47 GMT, Maurizio Cimadamore
wrote:
>> Archie L. Cobbs has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Fix incorrect @bug numbers in unit tests.
>
> src/jdk.compiler/share/cl
On Mon, 9 Jan 2023 15:03:10 GMT, Maurizio Cimadamore
wrote:
>> Archie L. Cobbs has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Fix incorrect @bug numbers in unit tests.
>
> src/jdk.compiler/share/cl
On Mon, 9 Jan 2023 06:37:22 GMT, David Holmes wrote:
> All your new files need a copyright and GPL header.
Sorry if I'm being blind but I'm not seeing it. Which file(s) are you referring
to?
The `@test /nodynamiccopyright/` files don't get one per
[this](https://openjdk.org/groups/compiler/te
re/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`
&g
re/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`
>
&
On Sat, 7 Jan 2023 18:03:04 GMT, Alan Bateman wrote:
> I don't think the implementation notes should be included as part of the
> adding the lint warning as I think it creates an attractive nuisance.
I agree with you - not only for that reason, but also because as others have
pointed out the a
re/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`
>
On Fri, 6 Jan 2023 15:38:31 GMT, Alan Bateman wrote:
>>> The associated JBS issue has been dormant for 6+ years and this is a very
>>> intrusive change affecting many, many files. Has the resurrection of this
>>> project previously been discussed somewhere?
>>
>> Hi @dholmes-ora,
>>
>> The wo
re/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`
>
&
On Fri, 6 Jan 2023 04:48:27 GMT, David Holmes wrote:
> The associated JBS issue has been dormant for 6+ years and this is a very
> intrusive change affecting many, many files. Has the resurrection of this
> project previously been discussed somewhere?
Hi @dholmes-ora,
The work to add this war
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
84 matches
Mail list logo
