Re: RFR: 8308286 Fix clang warnings in linux code [v3]
On Tue, 6 Jun 2023 17:47:02 GMT, Weijun Wang wrote: >> This is rarely used in the code and is not the essence of the current >> changes. >> If you introduce such changes, then throughout the code. >> Moreover, this can lead to problems, such as, for example, here: >> https://bugs.openjdk.org/browse/JDK-8309225 > > I'm not a clang expect, I was just asking if modifying the current `#if > TARGET_OS_MAC` check into `#if defined(__APPLE__)` is also a solution. The > comment on lines 46-47 says the condition was copied from a macOS SDK file > and that's what the file is using now. I think you can create a separate ticket and pull request to discuss this issue. - PR Review Comment: https://git.openjdk.org/jdk/pull/14033#discussion_r1225842637
Re: RFR: 8308286 Fix clang warnings in linux code [v3]
On Tue, 6 Jun 2023 17:32:35 GMT, Artem Semenov wrote: >> I didn't ask to revert the change. It's >> `s/TARGET_OS_MAC/defined(__APPLE__)/`. > > This is rarely used in the code and is not the essence of the current changes. > If you introduce such changes, then throughout the code. > Moreover, this can lead to problems, such as, for example, here: > https://bugs.openjdk.org/browse/JDK-8309225 I'm not a clang expect, I was just asking if modifying the current `#if TARGET_OS_MAC` check into `#if defined(__APPLE__)` is also a solution. The comment on lines 46-47 says the condition was copied from a macOS SDK file and that's what the file is using now. - PR Review Comment: https://git.openjdk.org/jdk/pull/14033#discussion_r1220071779
Re: RFR: 8308286 Fix clang warnings in linux code [v3]
On Thu, 1 Jun 2023 15:04:09 GMT, Weijun Wang wrote: >> done > > I didn't ask to revert the change. It's `s/TARGET_OS_MAC/defined(__APPLE__)/`. This is rarely used in the code and is not the essence of the current changes. If you introduce such changes, then throughout the code. Moreover, this can lead to problems, such as, for example, here: https://bugs.openjdk.org/browse/JDK-8309225 - PR Review Comment: https://git.openjdk.org/jdk/pull/14033#discussion_r1220056527
Re: RFR: 8308286 Fix clang warnings in linux code [v3]
On Wed, 31 May 2023 13:52:39 GMT, Weijun Wang wrote: >> Artem Semenov has updated the pull request incrementally with one additional >> commit since the last revision: >> >> update > > src/java.security.jgss/share/native/libj2gss/gssapi.h line 47: > >> 45: >> 46: // Condition was copied from >> 47: // >> Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/gssapi/gssapi.h > > In the current version of the file above, it's written as > > #if defined(__APPLE__) && (defined(__ppc__) ||... > > Is it better? done - PR Review Comment: https://git.openjdk.org/jdk/pull/14033#discussion_r1213297788
Re: RFR: 8308286 Fix clang warnings in linux code [v3]
On Thu, 1 Jun 2023 15:02:16 GMT, Artem Semenov wrote: >> src/java.security.jgss/share/native/libj2gss/gssapi.h line 47: >> >>> 45: >>> 46: // Condition was copied from >>> 47: // >>> Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/gssapi/gssapi.h >> >> In the current version of the file above, it's written as >> >> #if defined(__APPLE__) && (defined(__ppc__) ||... >> >> Is it better? > > done I didn't ask to revert the change. It's `s/TARGET_OS_MAC/defined(__APPLE__)/`. - PR Review Comment: https://git.openjdk.org/jdk/pull/14033#discussion_r1213300305
Re: RFR: 8308286 Fix clang warnings in linux code [v3]
On Wed, 31 May 2023 13:37:06 GMT, Artem Semenov wrote: >> When using the clang compiler to build OpenJDk on Linux, we encounter >> various "warnings as errors". >> They can be fixed with small changes. > > Artem Semenov has updated the pull request incrementally with one additional > commit since the last revision: > > update src/java.security.jgss/share/native/libj2gss/gssapi.h line 47: > 45: > 46: // Condition was copied from > 47: // > Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/gssapi/gssapi.h In the current version of the file above, it's written as #if defined(__APPLE__) && (defined(__ppc__) ||... Is it better? - PR Review Comment: https://git.openjdk.org/jdk/pull/14033#discussion_r1211757380
Re: RFR: 8308286 Fix clang warnings in linux code [v3]
On Sun, 28 May 2023 03:57:40 GMT, Kim Barrett wrote:
>> Artem Semenov has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> update
>
> src/java.desktop/unix/native/libawt_xawt/awt/gtk2_interface.c line 1163:
>
>> 1161: #if defined(__clang__)
>> 1162: #pragma clang diagnostic push
>> 1163: #pragma clang diagnostic ignored "-Wparentheses"
>
> I think this warning is because of the several `if (init_result = ...)`?
> Better would be to just add the extra parens.
done
> src/jdk.hotspot.agent/linux/native/libsaproc/ps_core.c line 575:
>
>> 573: if (ps_pdread(ph, (char *)link_map_addr + LINK_MAP_LD_OFFSET,
>> 574:&lib_ld, sizeof(uintptr_t)) != PS_OK) {
>> 575: #else
>
> What problem is being "fixed" by these? I'm dubious that this is the best
> solution to whatever the problem
> is, but can't evaluate or suggest alternatives without knowing what it is.
done
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14033#discussion_r1211733228
PR Review Comment: https://git.openjdk.org/jdk/pull/14033#discussion_r1211732832
Re: RFR: 8308286 Fix clang warnings in linux code [v3]
> When using the clang compiler to build OpenJDk on Linux, we encounter various > "warnings as errors". > They can be fixed with small changes. Artem Semenov has updated the pull request incrementally with one additional commit since the last revision: update - Changes: - all: https://git.openjdk.org/jdk/pull/14033/files - new: https://git.openjdk.org/jdk/pull/14033/files/3f343c26..d5b70cb2 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=14033&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14033&range=01-02 Stats: 4 lines in 2 files changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/14033.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14033/head:pull/14033 PR: https://git.openjdk.org/jdk/pull/14033
