RFR: 8295779: Xcode 14.0 fails to build jdk on m1 macos

2022-10-21 Thread Archie L . Cobbs
Building on MacOS 12.6 M1 with Xcode 14.0 fails due to C compiler unused parameter warnings: Creating support/modules_libs/java.desktop/libosx.dylib from 1 file(s) - Commit messages: - Fix build failure on MacOS 12.6 M1 with Xcode 14.0. Changes: https://git.openjdk.org/jdk/pull/10

Re: RFR: 8295779: Xcode 14.0 fails to build jdk on m1 macos

2022-10-21 Thread Archie L . Cobbs
On Wed, 19 Oct 2022 15:33:31 GMT, Archie L. Cobbs wrote: > Building on MacOS 12.6 M1 with Xcode 14.0 fails due to C compiler unused > parameter warnings: > > Creating support/modules_libs/java.desktop/libosx.dylib from 1 file(s) Thanks for doing that! PR title updated. -

Re: RFR: 8295779: Xcode 14.0 fails to build jdk on m1 macos

2022-10-24 Thread Archie L . Cobbs
On Wed, 19 Oct 2022 15:33:31 GMT, Archie L. Cobbs wrote: > Building on MacOS 12.6 M1 with Xcode 14.0 fails due to C compiler unused > parameter warnings: > > Creating support/modules_libs/java.desktop/libosx.dylib from 1 file(s) OK thanks. - PR: https://git.openjdk.

Withdrawn: 8295779: Xcode 14.0 fails to build jdk on m1 macos

2022-10-24 Thread Archie L . Cobbs
On Wed, 19 Oct 2022 15:33:31 GMT, Archie L. Cobbs wrote: > Building on MacOS 12.6 M1 with Xcode 14.0 fails due to C compiler unused > parameter warnings: > > Creating support/modules_libs/java.desktop/libosx.dylib from 1 file(s) This pull request has been closed without bein

Re: RFR: 8295779: Xcode 14.0 fails to build jdk on m1 macos

2022-11-10 Thread Archie L . Cobbs
On Thu, 10 Nov 2022 18:09:52 GMT, Vladimir Kempik wrote: > Well, you can just use `--disable-warnings-as-errors` configure argument > until this fixed Sure, but that's too blunt an instrument. It defeats the whole purpose of having warnings in the first place. If you're hacking on the code wi

Re: RFR: 8295779: Xcode 14.0 fails to build jdk on m1 macos

2022-11-10 Thread Archie L . Cobbs
On Wed, 19 Oct 2022 15:33:31 GMT, Archie L. Cobbs wrote: > Building on MacOS 12.6 M1 with Xcode 14.0 fails due to C compiler unused > parameter warnings: > > Creating support/modules_libs/java.desktop/libosx.dylib from 1 file(s) FWIW here are the patches I'm currently draggi

RFR: 8015831: Add lint check for calling overridable methods from a constructor

2023-01-05 Thread Archie L . Cobbs
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

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor

2023-01-06 Thread Archie L . Cobbs
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

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v2]

2023-01-06 Thread Archie L . Cobbs
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` > &

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor

2023-01-06 Thread Archie L . Cobbs
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: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v3]

2023-01-07 Thread Archie L . Cobbs
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` >

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor

2023-01-07 Thread Archie L . Cobbs
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: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v4]

2023-01-07 Thread Archie L . Cobbs
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` > &

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v5]

2023-01-07 Thread Archie L . Cobbs
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: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v5]

2023-01-09 Thread Archie L . Cobbs
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: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v5]

2023-01-10 Thread Archie L . Cobbs
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

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v5]

2023-01-10 Thread Archie L . Cobbs
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

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v6]

2023-01-10 Thread Archie L . Cobbs
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`

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v5]

2023-01-10 Thread Archie L . Cobbs
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: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v5]

2023-01-10 Thread Archie L . Cobbs
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. >>

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-10 Thread Archie L . Cobbs
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` >

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v5]

2023-01-10 Thread Archie L . Cobbs
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: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v5]

2023-01-11 Thread Archie L . Cobbs
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

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v5]

2023-01-11 Thread Archie L . Cobbs
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

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-11 Thread Archie L . Cobbs
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

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v5]

2023-01-11 Thread Archie L . Cobbs
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

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v5]

2023-01-11 Thread Archie L . Cobbs
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

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v5]

2023-01-12 Thread Archie L . Cobbs
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

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-12 Thread Archie L . Cobbs
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(). >>

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v5]

2023-01-12 Thread Archie L . Cobbs
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

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-12 Thread Archie L . Cobbs
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(). >>

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-12 Thread Archie L . Cobbs
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(). >>

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-12 Thread Archie L . Cobbs
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

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-12 Thread Archie L . Cobbs
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(). >>

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-12 Thread Archie L . Cobbs
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(). >>

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-12 Thread Archie L . Cobbs
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(). >>

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-12 Thread Archie L . Cobbs
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(). >>

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-12 Thread Archie L . Cobbs
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

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-12 Thread Archie L . Cobbs
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

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-12 Thread Archie L . Cobbs
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

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-12 Thread Archie L . Cobbs
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

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-12 Thread Archie L . Cobbs
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

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-12 Thread Archie L . Cobbs
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

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-12 Thread Archie L . Cobbs
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() { (

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-12 Thread Archie L . Cobbs
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

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-12 Thread Archie L . Cobbs
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

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-12 Thread Archie L . Cobbs
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

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-12 Thread Archie L . Cobbs
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 >>

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v8]

2023-01-12 Thread Archie L . Cobbs
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`

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-12 Thread Archie L . Cobbs
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: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-13 Thread Archie L . Cobbs
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: >> >

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-13 Thread Archie L . Cobbs
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

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v8]

2023-01-13 Thread Archie L . Cobbs
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

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-13 Thread Archie L . Cobbs
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

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v8]

2023-01-13 Thread Archie L . Cobbs
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

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v8]

2023-01-13 Thread Archie L . Cobbs
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

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v9]

2023-01-13 Thread Archie L . Cobbs
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

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v9]

2023-01-13 Thread Archie L . Cobbs
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: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v9]

2023-01-13 Thread Archie L . Cobbs
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

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v8]

2023-01-13 Thread Archie L . Cobbs
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

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v10]

2023-01-13 Thread Archie L . Cobbs
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` >

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-13 Thread Archie L . Cobbs
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: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v10]

2023-01-16 Thread Archie L . Cobbs
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

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v9]

2023-01-16 Thread Archie L . Cobbs
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

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v11]

2023-01-16 Thread Archie L . Cobbs
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` >

RFR: 8026369: javac potentially ambiguous overload warning needs an improved scheme

2023-02-19 Thread Archie L . Cobbs
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: RFR: 8301991: Convert l10n properties resource bundles to UTF-8 native

2023-03-15 Thread Archie L . Cobbs
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 >

RFR: 8182025: PropertyDescriptor ignores default methods from interfaces implemented by superclasses

2023-04-19 Thread Archie L . Cobbs
The `Introspector` class was never updated to include `default` methods inherited from interfaces. This patch attempts to fix that omission. - Commit messages: - Include default methods inherited from interfaces in bean introspection. Changes: https://git.openjdk.org/jdk/pull/1354

Re: RFR: 8071693: Introspector ignores default interface methods

2023-04-19 Thread Archie L . Cobbs
On Wed, 19 Apr 2023 21:29:05 GMT, Archie L. Cobbs wrote: > The `Introspector` class was never updated to include `default` methods > inherited from interfaces. > > This patch attempts to fix that omission. This one probably needs a CSR. - PR Comment: https://git.ope

Re: RFR: 8071693: Introspector ignores default interface methods [v2]

2023-04-19 Thread Archie L . Cobbs
> The `Introspector` class was never updated to include `default` methods > inherited from interfaces. > > This patch attempts to fix that omission. Archie L. Cobbs has updated the pull request incrementally with one additional commit since the last revision: Use Set.of() t

Re: RFR: 8071693: Introspector ignores default interface methods [v2]

2023-04-19 Thread Archie L . Cobbs
On Wed, 19 Apr 2023 21:40:14 GMT, Chen Liang wrote: >> Archie L. Cobbs has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Use Set.of() to initialize IGNORABLE_INTERFACES set. > > src/java.desktop/share/cl

Re: RFR: 8071693: Introspector ignores default interface methods [v2]

2023-04-20 Thread Archie L . Cobbs
On Thu, 20 Apr 2023 08:47:08 GMT, Andrey Turbanov wrote: >> Archie L. Cobbs has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Use Set.of() to initialize IGNORABLE_INTERFACES set. > > src/java.desktop/share/cl

Re: RFR: 8071693: Introspector ignores default interface methods [v3]

2023-04-20 Thread Archie L . Cobbs
> The `Introspector` class was never updated to include `default` methods > inherited from interfaces. > > This patch attempts to fix that omission. Archie L. Cobbs has updated the pull request incrementally with one additional commit since the last revision: Put braces around

Re: RFR: 8071693: Introspector ignores default interface methods [v2]

2023-04-20 Thread Archie L . Cobbs
On Thu, 20 Apr 2023 11:11:08 GMT, Alexey Ivanov wrote: >> Archie L. Cobbs has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Use Set.of() to initialize IGNORABLE_INTERFACES set. > > src/java.desktop/share/cl

Re: RFR: 8071693: Introspector ignores default interface methods [v4]

2023-04-20 Thread Archie L . Cobbs
> The `Introspector` class was never updated to include `default` methods > inherited from interfaces. > > This patch attempts to fix that omission. Archie L. Cobbs has updated the pull request incrementally with two additional commits since the last revision: - Verify static m

Re: RFR: 8071693: Introspector ignores default interface methods [v4]

2023-04-20 Thread Archie L . Cobbs
On Thu, 20 Apr 2023 19:29:46 GMT, Phil Race wrote: > The fix overall looks fine, but I'm not seeing why you think it needs a CSR. > It is just a bug fix. I'm not sure if it does or not. According to the [CSR FAQ](https://wiki.openjdk.org/display/csr/CSR+FAQs), _Behavioral compatibility involv

Re: RFR: 8071693: Introspector ignores default interface methods [v4]

2023-04-21 Thread Archie L . Cobbs
On Thu, 20 Apr 2023 14:22:47 GMT, Archie L. Cobbs wrote: >> The `Introspector` class was never updated to include `default` methods >> inherited from interfaces. >> >> This patch attempts to fix that omission. > > Archie L. Cobbs has updated the pull

Re: RFR: 8071693: Introspector ignores default interface methods [v5]

2023-04-21 Thread Archie L . Cobbs
> The `Introspector` class was never updated to include `default` methods > inherited from interfaces. > > This patch attempts to fix that omission. Archie L. Cobbs has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes t

Re: RFR: 8071693: Introspector ignores default interface methods [v2]

2023-04-21 Thread Archie L . Cobbs
On Fri, 21 Apr 2023 10:45:46 GMT, Andrey Turbanov wrote: >> given that serializable is commonly implemented, this may be a worthwhile >> optimisation. > > I think it should have some explanation comment, about why this interfaces > are ignored and how they were chosen. Fixed in ac90a10e238. -

Re: RFR: 8071693: Introspector ignores default interface methods [v4]

2023-04-21 Thread Archie L . Cobbs
On Fri, 21 Apr 2023 18:58:00 GMT, Alexey Ivanov wrote: > Is it expected that the test file doesn't compile with simple javac > DefaultMethodBeanPropertyTest.java? Definitely not. For example, many regression tests use classes that are not part of the standard JDK such as `toolbox.ToolBox`. --

Re: RFR: 8071693: Introspector ignores default interface methods [v2]

2023-04-21 Thread Archie L . Cobbs
On Fri, 21 Apr 2023 18:23:17 GMT, Alexey Ivanov wrote: >> OK, I'll fix. I've seen examples of both styles in the JDK so am never >> really sure. > > In client libs, we tend to use the braces all the time. > > By this comment, I meant adding braces to all the statements where you > omitted them

Re: RFR: 8071693: Introspector ignores default interface methods [v4]

2023-04-21 Thread Archie L . Cobbs
On Fri, 21 Apr 2023 20:21:03 GMT, Archie L. Cobbs wrote: > Let's make this CSR question easy. I'll remove the CSR and see if anybody > actually complains :) Ugh, it looks like the bot won't let me do that. A reviewer will have to remove the CSR ---

Re: RFR: 8071693: Introspector ignores default interface methods [v5]

2023-04-22 Thread Archie L . Cobbs
On Fri, 21 Apr 2023 22:19:28 GMT, Phil Race wrote: > I am submitting a test job to see if there are any unexpected failures from > this change. Thanks! - PR Comment: https://git.openjdk.org/jdk/pull/13544#issuecomment-1518699818