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.
>> -
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
the Javadoc 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:
- all: ht
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
the Javadoc 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.
-
Chang
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
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
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
; 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
> }
>
> public interface I3 extends SuperIface {
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
; 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
> }
>
> public interface I3 extends SuperIface {
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
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
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
; 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
> }
>
> public interface I3 extends SuperIface {
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:
>
>>
; 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
> }
>
> public interface I3 extends SuperIface {
; 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
> }
>
> public interface I3 extends SuperIface {
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
/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/ja
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
/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/jav
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
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
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
/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/lang
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 m
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
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
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 m
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
/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/tool
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
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
>> +++
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
>
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
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,
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
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.
>
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
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
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
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
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
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
/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/jav
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,
/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/to
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
/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/j
/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/ja
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
/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/langtool
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
/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/
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
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
82 matches
Mail list logo
