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
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.
-
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.
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
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
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
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
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/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 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`
>
&
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
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
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 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/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 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
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/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 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
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 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 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 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 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 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 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 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 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 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: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: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: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 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 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 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: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 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: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 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 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 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 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 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 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 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/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 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 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 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 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 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 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 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/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 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
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 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/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 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
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 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/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`
>
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
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
>
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
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
> 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
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
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
> 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
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
> 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
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
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
> 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
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.
-
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`.
--
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
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
---
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
83 matches
Mail list logo