Re: RFR: 8258010: Debug build failure with clang-10 due to -Wdeprecated-copy [v5]
On Mon, 11 Jan 2021 02:42:09 GMT, Kim Barrett wrote: >> Hao Sun has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Define the copy assign operator of class DUIterator_Last as defaulted >> >> The copy assignment operator of class DUIterator_Last should also be >> defined as defaulted, i.e. =default, keeping consistent with the copy >> constructor. >> >> Besides, fix the NIT for the copy ctor definition of class >> DUIterator_Last. >> >> Change-Id: I2f9502f023443163910eea9469b72df5bf1e25e0 >> CustomizedGitHooks: yes > > Marked as reviewed by kbarrett (Reviewer). Thanks a lot for your review @kimbarrett @vnkozlov . - PR: https://git.openjdk.java.net/jdk/pull/1874
Integrated: 8259343: [macOS] Update JNI error handling in Cocoa code.
On Wed, 6 Jan 2021 21:14:06 GMT, Phil Race wrote: > Proposed updates to JNI error handling. This pull request has now been integrated. Changeset: d6a2105b Author:Phil Race URL: https://git.openjdk.java.net/jdk/commit/d6a2105b Stats: 75 lines in 8 files changed: 60 ins; 1 del; 14 mod 8259343: [macOS] Update JNI error handling in Cocoa code. Reviewed-by: erikj, serb - PR: https://git.openjdk.java.net/jdk/pull/1967
Re: RFR: 8259343: [macOS] Update JNI error handling in Cocoa code. [v3]
On Mon, 11 Jan 2021 19:27:12 GMT, Phil Race wrote: >> Proposed updates to JNI error handling. > > Phil Race has updated the pull request incrementally with one additional > commit since the last revision: > > 8259343: [macOS] Update JNI error handling in Cocoa code. Marked as reviewed by serb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1967
Re: RFR: 8259343: [macOS] Update JNI error handling in Cocoa code. [v3]
On Tue, 12 Jan 2021 17:21:53 GMT, Phil Race wrote:
>> src/java.desktop/macosx/native/libosxapp/JNIUtilities.h line 197:
>>
>>> 195: } \
>>> 196: if (getenv("JNU_NO_COCOA_EXCEPTION") == NULL) { \
>>> 197: [NSException raise:NSGenericException format:@"Java
>>> Exception"]; \
>>
>> How did you check that the logging in the NSApplication was swallowing? Both
>> macro will throw the NSException on the toolkit thread now, does it mean
>> that in both cases the logging in the NSApplication will be ignored/no
>> output?
>
> See the bug assigned to you that I filed last month :
> https://bugs.openjdk.java.net/browse/JDK-8258797
> This error should have been logged by that NSApplicationAWT code but was not
> (and I mean in JDK 16 as well before I started on this) and in JDK 17 it was
> seen only when adding the new logging.
I have found it down to the absence of NSApplication#reportException() method
and logging in it. Ok will update that code later in the separate update.
-
PR: https://git.openjdk.java.net/jdk/pull/1967
Re: RFR: 8258010: Debug build failure with clang-10 due to -Wdeprecated-copy [v5]
On Mon, 11 Jan 2021 02:14:17 GMT, Hao Sun wrote: >> 1. '-Wdeprecated-copy' >> As specified in C++11 [1], "the generation of the implicitly-defined >> copy constructor is deprecated if T has a user-defined destructor or >> user-defined copy assignment operator". The rationale behind is the >> well-known Rule of Three [2]. >> >> Introduced since gcc-9 [3] and clang-10 [4], flag '-Wdeprecated-copy' >> warns about the C++11 deprecation of implicitly declared copy >> constructor and assignment operator if one of them is user-provided. >> Defining an explicit copy constructor would suppress this warning. >> >> The main reason why debug build with gcc-9 or higher succeeds lies in >> the inconsistent warning behaviors between gcc and clang. See the >> reduced code example [5]. We suspect it might be return value >> optimization/copy elision [6] that drives gcc not to declare implicit >> copy constructor for this case. >> >> Note that flag '-Wdeprecated' in clang-8 and clang-9 would also raise >> warnings for deprecated defintions of copy constructors. However, >> '-Wdeprecated' is not enabled by '-Wall' or '-Wextra'. Hence, clang-8 >> and clang-9 are not affected. >> >> ~~2. '-Wimplicit-int-float-conversion'~~ >> ~~Making the conversion explicit would fix it.~~ >> >> ~~Flag '-Wimplicit-int-float-conversion' is first introduced in clang-10.~~ >> ~~Therefore clang-8 and clang-9 are not affected. The flag with similar~~ >> ~~functionality in gcc is '-Wfloat-conversion', but it is not enabled by~~ >> ~~'-Wall' or '-Wextra'. That's why this warning does not apprear when~~ >> ~~building with gcc.~~ >> >> [1] https://en.cppreference.com/w/cpp/language/copy_constructor >> [2] https://en.cppreference.com/w/cpp/language/rule_of_three >> [3] https://www.gnu.org/software/gcc/gcc-9/changes.html >> [4] https://releases.llvm.org/10.0.0/tools/clang/docs/ReleaseNotes.html >> [5] https://godbolt.org/z/err4jM >> [6] https://en.wikipedia.org/wiki/Copy_elision#Return_value_optimization >> >> >> Note that we have tested with this patch, debug build succeeded with >> clang-10 on Linux X86-64/AArch64 machines. >> Note that '--with-extra-cxxflags=-Wno-implicit-int-float-conversion' should >> be added when configuration. It's another issue (See JDK-8259288) > > Hao Sun has updated the pull request incrementally with one additional commit > since the last revision: > > Define the copy assign operator of class DUIterator_Last as defaulted > > The copy assignment operator of class DUIterator_Last should also be > defined as defaulted, i.e. =default, keeping consistent with the copy > constructor. > > Besides, fix the NIT for the copy ctor definition of class > DUIterator_Last. > > Change-Id: I2f9502f023443163910eea9469b72df5bf1e25e0 > CustomizedGitHooks: yes Looks good. - Marked as reviewed by kvn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1874
Re: RFR: 8259343: [macOS] Update JNI error handling in Cocoa code. [v3]
On Tue, 12 Jan 2021 02:31:56 GMT, Sergey Bylokhov wrote:
>> Phil Race has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> 8259343: [macOS] Update JNI error handling in Cocoa code.
>
> src/java.desktop/macosx/native/libosxapp/JNIUtilities.h line 197:
>
>> 195: } \
>> 196: if (getenv("JNU_NO_COCOA_EXCEPTION") == NULL) { \
>> 197: [NSException raise:NSGenericException format:@"Java
>> Exception"]; \
>
> How did you check that the logging in the NSApplication was swallowing? Both
> macro will throw the NSException on the toolkit thread now, does it mean that
> in both cases the logging in the NSApplication will be ignored/no output?
See the bug assigned to you that I filed last month :
https://bugs.openjdk.java.net/browse/JDK-8258797
This error should have been logged by that NSApplicationAWT code but was not
(and I mean in JDK 16 as well before I started on this) and in JDK 17 it was
seen only when adding the new logging.
-
PR: https://git.openjdk.java.net/jdk/pull/1967
Re: RFR: 8259343: [macOS] Update JNI error handling in Cocoa code. [v3]
On Tue, 12 Jan 2021 02:38:11 GMT, Sergey Bylokhov wrote: >>> Not sure that the check for ExceptionOccurred is needed, >> It may not be needed in practice but if the code path is never taken no harm >> ... >> >> in all other places where we check the ref to methods/field we only check >> the return value, and if it is null then return immediately assuming that an >> exception is rased already, for example : >>> >>> https://github.com/openjdk/jdk/blob/b72de3c5fc99f365e9fb25114ddd28eceddfa6e8/src/java.desktop/windows/native/libawt/windows/awt_Button.cpp#L357 >>> >>> Note that the exception in the static initializer is fatal for the >>> application. >> >> Nothing new here. > > The new thing here is that you check the result of the ExceptionOccurred if > NULL is returned from the GetMethodID/etc, we usually never do it. If such > checks are necessary then I'll create a separate bug to update other similar > use cases. Here I don't have immediate control over why this is being called. So we could have null but no pending exception depending on what the caller did. It won't be executed except in the rare case there's a problem so it certainly isn't causing overhead so what is the problem ? I don't see a need to change other code unless there's a really good reason. - PR: https://git.openjdk.java.net/jdk/pull/1967
RFR: 8226810: Failed to launch JVM because of NullPointerException occurred on System.props
Hi, Please review this small change which enables the GB18030 charset to be built into java.base Thanks - Commit messages: - 8226810: Failed to launch JVM because of NullPointerException occurred on System.props Changes: https://git.openjdk.java.net/jdk/pull/2053/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2053=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8226810 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/2053.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2053/head:pull/2053 PR: https://git.openjdk.java.net/jdk/pull/2053
Re: RFR: 8259485: Document need for short paths when building on Windows
On Tue, 12 Jan 2021 06:46:33 GMT, liach wrote: > Though this content seems simple, it would be extremely frustrating to > newcomers, especially when the build stalls at "cannot find valid visual > studio installation" for no clear reason in logs at all. Marked as reviewed by erikj (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2043
RE: [11u] RFR: 8256810: Incremental rebuild broken on Macosx
>> Sure. Is a comment stating that this was backported what you have in mind? > Preferrably comment + link "backportet by" to your backport issue. I'll do that once it's in. Roland.
