Warnings -Wsign-compare inside an assert when building with Clang 11.1.0 on Windows #588

New issue

Closed

opened 2021-10-08 13:05:31 -05:00 by andrzejc · 11 comments

andrzejc commented 2021-10-08 13:05:31 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

The exact problem happens when assert() macro is active; the code location in release 0.8.0 is:

xxhsum.c:1540:38: warning: comparison of integers of different signs: 'const AlgoSelected' and 'unsigned long long' [-Wsign-compare]
assert(0 <= hashType && hashType <= XSUM_TABLE_ELT_SIZE(XSUM_algoName));

This is breaking build of projects using xxHash as in-source dependency, when -Werror flag is active.

The exact problem happens when assert() macro is active; the code location in release 0.8.0 is: xxhsum.c:1540:38: warning: comparison of integers of different signs: 'const AlgoSelected' and 'unsigned long long' [-Wsign-compare] assert(0 <= hashType && hashType <= XSUM_TABLE_ELT_SIZE(XSUM_algoName)); This is breaking build of projects using xxHash as in-source dependency, when -Werror flag is active.

t-mat commented 2021-10-08 15:24:55 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Hi @andrzejc, thanks for the report.

Firstly, please let me explain about our status.

At the time of releasing v0.8.0,

• We had -Wall -Wextra. But we didn't have -Werror in our CI.
• We didn't test against clang-11.

After the release of v0.8.0,

• We've changed strcture of xxhsum.c. But it still has same fuction and condition of assert().
• We've added test for clang-11.0 which always test with -Wall -Wextra -Werror.
  • As you know, -Wextra includes -Wsign-compare.

So far, we don't have any basic warnings/errors in our dev branch.

---

Unfortunately, I couldn't reproduce this issue with clang-11. I tried the following commands

cd
clang-11 --version
# Ubuntu clang version 11.0.0-2~ubuntu20.04.1
git clone https://github.com/Cyan4973/xxHash.git xxHash-issue-588
cd xxHash-issue-588
git checkout v0.8.0
git log -1
make clean
CFLAGS="-Wall -Wextra -Werror" CC=clang-11 V=1 make

So, could you provide reproducing procedure of this issue?

Also it'd be great if you can provide actual compiler options. It seems it's hard to track this issue without it.

Lastly, please try recent dev branch.

Hi @andrzejc, thanks for the report. Firstly, please let me explain about our status. At the time of releasing `v0.8.0`, - [We had `-Wall -Wextra`. But we didn't have `-Werror`](https://github.com/Cyan4973/xxHash/blob/v0.8.0/.travis.yml#L28) in our CI. - We didn't test against `clang-11`. After the release of `v0.8.0`, - We've changed strcture of `xxhsum.c`. But it still has [same fuction and condition of `assert()`](https://github.com/Cyan4973/xxHash/blob/9058687747c533a2fea7215fd72346e756da2034/cli/xxhsum.c#L587-L598). - We've added test for [`clang-11.0`](https://github.com/Cyan4973/xxHash/blob/9058687747c533a2fea7215fd72346e756da2034/.github/workflows/ci.yml#L88) which always test with [`-Wall -Wextra -Werror`](https://github.com/Cyan4973/xxHash/blob/9058687747c533a2fea7215fd72346e756da2034/.github/workflows/ci.yml#L137-L140). - As you know, [`-Wextra` includes `-Wsign-compare`](https://releases.llvm.org/11.0.0/tools/clang/docs/DiagnosticsReference.html#wextra). So far, we don't have any basic warnings/errors in our `dev` branch. ---- Unfortunately, I couldn't reproduce this issue with `clang-11`. I tried the following commands ```sh cd clang-11 --version # Ubuntu clang version 11.0.0-2~ubuntu20.04.1 git clone https://github.com/Cyan4973/xxHash.git xxHash-issue-588 cd xxHash-issue-588 git checkout v0.8.0 git log -1 make clean CFLAGS="-Wall -Wextra -Werror" CC=clang-11 V=1 make ``` So, could you provide reproducing procedure of this issue? Also it'd be great if you can provide actual compiler options. It seems it's hard to track this issue without it. Lastly, please try recent `dev` branch.

👍 1

andrzejc commented 2021-10-08 15:39:57 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Hi @t-mat ,
Thank you for quick reply.
This problem doesn't appear with clang-11 on Ubuntu. I was able to get it only on Windows with the official release of Clang from https://github.com/llvm/llvm-project/releases/download/llvmorg-11.1.0/LLVM-11.1.0-win64.exe package. This is also different than the Clang release shipped with mingw64. The way I use xxHash is as a wrap project with meson, so that it is built inline as a part of my project's build. I was asked by meson's maintainer to include link to the issue reported to xxHash in my PR silencing this warning in the meson's wrap project, so that they can remove the workaround if/when the issue is fixed - which is the reason I created this report. My guess is that on Windows, when Clang is using MSVC ABI enum types are signed, which triggers this warning. This is different to Linux Clang build, where it's emulating GCC ABI.

Hi @t-mat , Thank you for quick reply. This problem doesn't appear with clang-11 on Ubuntu. I was able to get it only on Windows with the official release of Clang from https://github.com/llvm/llvm-project/releases/download/llvmorg-11.1.0/LLVM-11.1.0-win64.exe package. This is also different than the Clang release shipped with mingw64. The way I use xxHash is as a wrap project with [meson](https://mesonbuild.com/Wrapdb-projects.html), so that it is built inline as a part of my project's build. I was asked by meson's maintainer to include link to the issue reported to xxHash in my PR silencing this warning in the meson's wrap project, so that they can remove the workaround if/when the issue is fixed - which is the reason I created this report. My guess is that on Windows, when Clang is using MSVC ABI enum types are signed, which triggers this warning. This is different to Linux Clang build, where it's emulating GCC ABI.

👍 1

t-mat commented 2021-10-08 16:39:18 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Thanks for the info. I've successfully reproduce this issue with LLVM-11.1.0-win64.exe on Microsoft Windows [Version 10.0.19042.1165].

For now, I just put the procedure to help other contributors can follow this issue.

Install LLVM-11.1.0-win64

ver
cd /d "%USERPROFILE%\Downloads"
curl -JLO https://github.com/llvm/llvm-project/releases/download/llvmorg-11.1.0/LLVM-11.1.0-win64.exe
start .\LLVM-11.1.0-win64.exe

v0.8.0

cd /d "%USERPROFILE%\Documents"
git clone https://github.com/Cyan4973/xxHash.git xxHash-issue-588-v0.8.0
cd xxHash-issue-588-v0.8.0
git checkout v0.8.0
git log -1

set "clang=C:\Program Files\LLVM\bin\clang.exe"
"%clang%" -Wall -Wextra -Werror xxhash.c xxhsum.c

In file included from xxhash.c:43:
./xxhash.h:3503:1: error: unused function 'XXH3_accumulate_512_scalar' [-Werror,-Wunused-function]
XXH3_accumulate_512_scalar(void* XXH_RESTRICT acc,
^
./xxhash.h:3521:1: error: unused function 'XXH3_scrambleAcc_scalar' [-Werror,-Wunused-function]
XXH3_scrambleAcc_scalar(void* XXH_RESTRICT acc, const void* XXH_RESTRICT secret)
^
./xxhash.h:3538:1: error: unused function 'XXH3_initCustomSecret_scalar' [-Werror,-Wunused-function]
XXH3_initCustomSecret_scalar(void* XXH_RESTRICT customSecret, xxh_u64 seed64)
^
3 errors generated.
xxhsum.c:1540:38: error: comparison of integers of different signs: 'const AlgoSelected' and 'unsigned long long'
      [-Werror,-Wsign-compare]
    assert(0 <= hashType && hashType <= XSUM_TABLE_ELT_SIZE(XSUM_algoName));
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
C:\Program Files (x86)\Windows Kits\10\Include\10.0.20348.0\ucrt\assert.h:40:17: note: expanded from macro 'assert'
            (!!(expression)) ||                                                              \
                ^~~~~~~~~~
xxhsum.c:1562:38: error: comparison of integers of different signs: 'const AlgoSelected' and 'unsigned long long'
      [-Werror,-Wsign-compare]
    assert(0 <= hashType && hashType <= XSUM_TABLE_ELT_SIZE(XSUM_algoName));
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
C:\Program Files (x86)\Windows Kits\10\Include\10.0.20348.0\ucrt\assert.h:40:17: note: expanded from macro 'assert'
            (!!(expression)) ||                                                              \
                ^~~~~~~~~~
2 errors generated.

dev

cd /d "%USERPROFILE%\Documents"
git clone https://github.com/Cyan4973/xxHash.git xxHash-issue-588-dev
cd xxHash-issue-588-dev
git log -1

set "clang=C:\Program Files\LLVM\bin\clang.exe"
"%clang%" -Wall -Wextra -Werror xxhash.c cli/xxhsum.c cli/xsum_os_specific.c cli/xsum_output.c cli/xsum_sanity_check.c

In file included from xxhash.c:43:
./xxhash.h:4160:1: error: unused function 'XXH3_accumulate_512_scalar' [-Werror,-Wunused-function]
XXH3_accumulate_512_scalar(void* XXH_RESTRICT acc,
^
./xxhash.h:4178:1: error: unused function 'XXH3_scrambleAcc_scalar' [-Werror,-Wunused-function]
XXH3_scrambleAcc_scalar(void* XXH_RESTRICT acc, const void* XXH_RESTRICT secret)
^
./xxhash.h:4195:1: error: unused function 'XXH3_initCustomSecret_scalar' [-Werror,-Wunused-function]
XXH3_initCustomSecret_scalar(void* XXH_RESTRICT customSecret, xxh_u64 seed64)
^
3 errors generated.
cli/xxhsum.c:592:38: error: comparison of integers of different signs: 'const AlgoSelected' and 'unsigned long long' [-Werror,-Wsign-compare]
    assert(0 <= hashType && hashType <= XSUM_TABLE_ELT_SIZE(XSUM_algoName));
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
C:\Program Files (x86)\Windows Kits\10\Include\10.0.20348.0\ucrt\assert.h:40:17: note: expanded from macro 'assert'
            (!!(expression)) ||                                                              \
                ^~~~~~~~~~
cli/xxhsum.c:614:38: error: comparison of integers of different signs: 'const AlgoSelected' and 'unsigned long long' [-Werror,-Wsign-compare]
    assert(0 <= hashType && hashType <= XSUM_TABLE_ELT_SIZE(XSUM_algoName));
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
C:\Program Files (x86)\Windows Kits\10\Include\10.0.20348.0\ucrt\assert.h:40:17: note: expanded from macro 'assert'
            (!!(expression)) ||                                                              \
                ^~~~~~~~~~
2 errors generated.

Thanks for the info. I've successfully reproduce this issue with `LLVM-11.1.0-win64.exe` on `Microsoft Windows [Version 10.0.19042.1165]`. For now, I just put the procedure to help other contributors can follow this issue. ## Install LLVM-11.1.0-win64 ```bat ver cd /d "%USERPROFILE%\Downloads" curl -JLO https://github.com/llvm/llvm-project/releases/download/llvmorg-11.1.0/LLVM-11.1.0-win64.exe start .\LLVM-11.1.0-win64.exe ``` ## v0.8.0 ```bat cd /d "%USERPROFILE%\Documents" git clone https://github.com/Cyan4973/xxHash.git xxHash-issue-588-v0.8.0 cd xxHash-issue-588-v0.8.0 git checkout v0.8.0 git log -1 set "clang=C:\Program Files\LLVM\bin\clang.exe" "%clang%" -Wall -Wextra -Werror xxhash.c xxhsum.c ``` ```txt In file included from xxhash.c:43: ./xxhash.h:3503:1: error: unused function 'XXH3_accumulate_512_scalar' [-Werror,-Wunused-function] XXH3_accumulate_512_scalar(void* XXH_RESTRICT acc, ^ ./xxhash.h:3521:1: error: unused function 'XXH3_scrambleAcc_scalar' [-Werror,-Wunused-function] XXH3_scrambleAcc_scalar(void* XXH_RESTRICT acc, const void* XXH_RESTRICT secret) ^ ./xxhash.h:3538:1: error: unused function 'XXH3_initCustomSecret_scalar' [-Werror,-Wunused-function] XXH3_initCustomSecret_scalar(void* XXH_RESTRICT customSecret, xxh_u64 seed64) ^ 3 errors generated. xxhsum.c:1540:38: error: comparison of integers of different signs: 'const AlgoSelected' and 'unsigned long long' [-Werror,-Wsign-compare] assert(0 <= hashType && hashType <= XSUM_TABLE_ELT_SIZE(XSUM_algoName)); ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C:\Program Files (x86)\Windows Kits\10\Include\10.0.20348.0\ucrt\assert.h:40:17: note: expanded from macro 'assert' (!!(expression)) || \ ^~~~~~~~~~ xxhsum.c:1562:38: error: comparison of integers of different signs: 'const AlgoSelected' and 'unsigned long long' [-Werror,-Wsign-compare] assert(0 <= hashType && hashType <= XSUM_TABLE_ELT_SIZE(XSUM_algoName)); ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C:\Program Files (x86)\Windows Kits\10\Include\10.0.20348.0\ucrt\assert.h:40:17: note: expanded from macro 'assert' (!!(expression)) || \ ^~~~~~~~~~ 2 errors generated. ``` ## dev ```bat cd /d "%USERPROFILE%\Documents" git clone https://github.com/Cyan4973/xxHash.git xxHash-issue-588-dev cd xxHash-issue-588-dev git log -1 set "clang=C:\Program Files\LLVM\bin\clang.exe" "%clang%" -Wall -Wextra -Werror xxhash.c cli/xxhsum.c cli/xsum_os_specific.c cli/xsum_output.c cli/xsum_sanity_check.c ``` ```txt In file included from xxhash.c:43: ./xxhash.h:4160:1: error: unused function 'XXH3_accumulate_512_scalar' [-Werror,-Wunused-function] XXH3_accumulate_512_scalar(void* XXH_RESTRICT acc, ^ ./xxhash.h:4178:1: error: unused function 'XXH3_scrambleAcc_scalar' [-Werror,-Wunused-function] XXH3_scrambleAcc_scalar(void* XXH_RESTRICT acc, const void* XXH_RESTRICT secret) ^ ./xxhash.h:4195:1: error: unused function 'XXH3_initCustomSecret_scalar' [-Werror,-Wunused-function] XXH3_initCustomSecret_scalar(void* XXH_RESTRICT customSecret, xxh_u64 seed64) ^ 3 errors generated. cli/xxhsum.c:592:38: error: comparison of integers of different signs: 'const AlgoSelected' and 'unsigned long long' [-Werror,-Wsign-compare] assert(0 <= hashType && hashType <= XSUM_TABLE_ELT_SIZE(XSUM_algoName)); ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C:\Program Files (x86)\Windows Kits\10\Include\10.0.20348.0\ucrt\assert.h:40:17: note: expanded from macro 'assert' (!!(expression)) || \ ^~~~~~~~~~ cli/xxhsum.c:614:38: error: comparison of integers of different signs: 'const AlgoSelected' and 'unsigned long long' [-Werror,-Wsign-compare] assert(0 <= hashType && hashType <= XSUM_TABLE_ELT_SIZE(XSUM_algoName)); ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C:\Program Files (x86)\Windows Kits\10\Include\10.0.20348.0\ucrt\assert.h:40:17: note: expanded from macro 'assert' (!!(expression)) || \ ^~~~~~~~~~ 2 errors generated. ```

👍 1

t-mat commented 2021-10-08 16:51:50 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

As for dev branch, obviously, the most straightforward fix is adding explicit cast from AlgoSelected to size_t.

        static void XSUM_printLine_BSD_internal(const char* filename,
                                                const void* canonicalHash, const AlgoSelected hashType,
                                                const char* algoString[],
                                                XSUM_displayHash_f f_displayHash)
        {
-           assert(0 <= hashType && hashType <= XSUM_TABLE_ELT_SIZE(XSUM_algoName));
+           assert(0 <= hashType && ((size_t) hashType) <= XSUM_TABLE_ELT_SIZE(XSUM_algoName));

        static void XSUM_printLine_GNU_internal(const char* filename,
                                       const void* canonicalHash, const AlgoSelected hashType,
                                       XSUM_displayHash_f f_displayHash)
        {
-           assert(0 <= hashType && hashType <= XSUM_TABLE_ELT_SIZE(XSUM_algoName));
+           assert(0 <= hashType && ((size_t) hashType) <= XSUM_TABLE_ELT_SIZE(XSUM_algoName));

As for `dev` branch, obviously, the most straightforward fix is adding explicit cast from `AlgoSelected` to `size_t`. ```c static void XSUM_printLine_BSD_internal(const char* filename, const void* canonicalHash, const AlgoSelected hashType, const char* algoString[], XSUM_displayHash_f f_displayHash) { - assert(0 <= hashType && hashType <= XSUM_TABLE_ELT_SIZE(XSUM_algoName)); + assert(0 <= hashType && ((size_t) hashType) <= XSUM_TABLE_ELT_SIZE(XSUM_algoName)); ``` ```c static void XSUM_printLine_GNU_internal(const char* filename, const void* canonicalHash, const AlgoSelected hashType, XSUM_displayHash_f f_displayHash) { - assert(0 <= hashType && hashType <= XSUM_TABLE_ELT_SIZE(XSUM_algoName)); + assert(0 <= hashType && ((size_t) hashType) <= XSUM_TABLE_ELT_SIZE(XSUM_algoName)); ```

t-mat commented 2021-10-08 17:15:07 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

As for -Wunused-function, we need to check what's happening.

/* test-clang.c */
/* "%clang%" -Wall -Wextra -Werror test-clang.c && .\a.exe */
#include <stdio.h>
int main(int argc, char** argv) {
    (void) argc; (void) argv;

    #if defined(__GNUC__)
    printf("__GNUC__\n");
    #endif

    #if defined(__clang__)
    printf("__clang__\n");
    #endif

    #if defined(_MSC_VER)
    printf("_MSC_VER\n");
    #endif
}

If we compile and run above program with clang.exe (Target: x86_64-pc-windows-msvc) it outputs the following

> "%clang%" --version
clang version 11.1.0
Target: x86_64-pc-windows-msvc
Thread model: posix
InstalledDir: C:\Program Files\LLVM\bin

> "%clang%" -Wall -Wextra -Werror test-clang.c && .\a.exe
__clang__
_MSC_VER

Of course this output is different from standard Linux version of clang.

$ clang-11 --version
Ubuntu clang version 11.0.0-2~ubuntu20.04.1
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

$ clang-11 ./test-clang.c && ./a.out
__GNUC__
__clang__

So, it seems we need some tweak for __clang__ which doesn't have __GNUC__.

// xxhash.h
	#if XXH_NO_INLINE_HINTS  /* disable inlining hints */
-	#  if defined(__GNUC__)
+	#  if defined(__GNUC__) || defined(__clang__)
	#    define XXH_FORCE_INLINE static __attribute__((unused))
	#  else
	#    define XXH_FORCE_INLINE static
	#  endif
	#  define XXH_NO_INLINE static
	/* enable inlining hints */
-	#elif defined(_MSC_VER)  /* Visual Studio */
-	#  define XXH_FORCE_INLINE static __forceinline
-	#  define XXH_NO_INLINE static __declspec(noinline)
-	#elif defined(__GNUC__)
-	#  define XXH_FORCE_INLINE static __inline__ __attribute__((always_inline, unused))
-	#  define XXH_NO_INLINE static __attribute__((noinline))
+	#elif defined(__GNUC__) || defined(__clang__)
+	#  define XXH_FORCE_INLINE static __inline__ __attribute__((always_inline, unused))
+	#  define XXH_NO_INLINE static __attribute__((noinline))
+	#elif defined(_MSC_VER)  /* Visual Studio */
+	#  define XXH_FORCE_INLINE static __forceinline
+	#  define XXH_NO_INLINE static __declspec(noinline)

As for `-Wunused-function`, we need to check what's happening. ```c /* test-clang.c */ /* "%clang%" -Wall -Wextra -Werror test-clang.c && .\a.exe */ #include <stdio.h> int main(int argc, char** argv) { (void) argc; (void) argv; #if defined(__GNUC__) printf("__GNUC__\n"); #endif #if defined(__clang__) printf("__clang__\n"); #endif #if defined(_MSC_VER) printf("_MSC_VER\n"); #endif } ``` If we compile and run above program with `clang.exe` (`Target: x86_64-pc-windows-msvc`) it outputs the following ``` > "%clang%" --version clang version 11.1.0 Target: x86_64-pc-windows-msvc Thread model: posix InstalledDir: C:\Program Files\LLVM\bin > "%clang%" -Wall -Wextra -Werror test-clang.c && .\a.exe __clang__ _MSC_VER ``` Of course this output is different from standard Linux version of `clang`. ``` $ clang-11 --version Ubuntu clang version 11.0.0-2~ubuntu20.04.1 Target: x86_64-pc-linux-gnu Thread model: posix InstalledDir: /usr/bin $ clang-11 ./test-clang.c && ./a.out __GNUC__ __clang__ ``` So, it seems we need some tweak for `__clang__` which doesn't have `__GNUC__`. ```c // xxhash.h #if XXH_NO_INLINE_HINTS /* disable inlining hints */ - # if defined(__GNUC__) + # if defined(__GNUC__) || defined(__clang__) # define XXH_FORCE_INLINE static __attribute__((unused)) # else # define XXH_FORCE_INLINE static # endif # define XXH_NO_INLINE static /* enable inlining hints */ - #elif defined(_MSC_VER) /* Visual Studio */ - # define XXH_FORCE_INLINE static __forceinline - # define XXH_NO_INLINE static __declspec(noinline) - #elif defined(__GNUC__) - # define XXH_FORCE_INLINE static __inline__ __attribute__((always_inline, unused)) - # define XXH_NO_INLINE static __attribute__((noinline)) + #elif defined(__GNUC__) || defined(__clang__) + # define XXH_FORCE_INLINE static __inline__ __attribute__((always_inline, unused)) + # define XXH_NO_INLINE static __attribute__((noinline)) + #elif defined(_MSC_VER) /* Visual Studio */ + # define XXH_FORCE_INLINE static __forceinline + # define XXH_NO_INLINE static __declspec(noinline) ```

t-mat commented 2021-10-08 17:19:33 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Since these warnings/errors/tweaks are hard to find without compiler's help, I think we must include clang for MSVC (Target: x86_64-pc-windows-msvc) in our CI.

Since these warnings/errors/tweaks are hard to find without compiler's help, I think we must include `clang` for MSVC (`Target: x86_64-pc-windows-msvc`) in our CI.

Cyan4973 commented 2021-10-08 17:26:47 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Since these warnings/errors/tweaks are hard to find without compiler's help, I think we must include clang for MSVC (Target: x86_64-pc-windows-msvc) in our CI.

Agreed

> Since these warnings/errors/tweaks are hard to find without compiler's help, I think we must include `clang` for MSVC (`Target: x86_64-pc-windows-msvc`) in our CI. Agreed

👍 1

Cyan4973 commented 2021-11-25 13:36:08 -06:00 (Migrated from github.com)

• Copy link
• Copy Markdown

It seems there is still a need to add the test target clang + msvc
to either github action (preferably) or Appveyor CI if that's the only working solution ?

It seems there is still a need to add the test target `clang + msvc` to either github action (preferably) or Appveyor CI if that's the only working solution ?

t-mat commented 2021-11-25 17:21:02 -06:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Sorry for the delay. I'm working on it. My plan is adding new matrix entries to windows-visualc-general of ci.yml.

Basically, the following cmake command with CMAKE_GENERATOR_TOOLSET enables clang-cl.

cd cmake_unofficial
mkdir build
cd build
cmake .. -DCMAKE_BUILD_TYPE=Release -A x64 -DCMAKE_GENERATOR_TOOLSET=ClangCL
cmake --build . --config Release

But this change also introduces a lot of unnecessary warnings/errors. I'm tweaking XXHASH_C_FLAGS to minimize it.

Sorry for the delay. I'm working on it. My plan is adding new matrix entries to `windows-visualc-general` of `ci.yml`. Basically, the following `cmake` command with `CMAKE_GENERATOR_TOOLSET` enables `clang-cl`. ```bat cd cmake_unofficial mkdir build cd build cmake .. -DCMAKE_BUILD_TYPE=Release -A x64 -DCMAKE_GENERATOR_TOOLSET=ClangCL cmake --build . --config Release ``` But this change also introduces a lot of unnecessary warnings/errors. I'm tweaking `XXHASH_C_FLAGS` to minimize it.

👍 1

Cyan4973 commented 2021-11-25 21:06:11 -06:00 (Migrated from github.com)

• Copy link
• Copy Markdown

I just tried to reproduce the -Wsign-compare issue on my local VM, using Visual Studio 2019 + clang-cl,
but failed to witness it.
I'm certain that assert() is enabled in this test, the main remaining difference I can see is
that my local MSVC version has installed clang-12.0, instead of clang-11.1 mentioned in this issue.
Not yet sure how to play with different clang versions within MSVC ...
edit : VS-install definitely doesn't allow installing multiple versions of clang, so it's currently v12.0 or nothing...
edit 2 : after following instructions from @t-mat , and installing clang-11.1 on its own (not as part of MSVC) and using it to compile xxhash, I can finally reproduce the reported warning. Gosh, there are so many ways to use and install variants of clang on the same system, and they also differ in the outcome of their warnings ....

I just tried to reproduce the `-Wsign-compare` issue on my local VM, using Visual Studio 2019 + `clang-cl`, but failed to witness it. I'm certain that `assert()` is enabled in this test, the main remaining difference I can see is that my local MSVC version has installed `clang-12.0`, instead of `clang-11.1` mentioned in this issue. Not yet sure how to play with different `clang` versions within MSVC ... _edit_ : VS-install definitely doesn't allow installing multiple versions of `clang`, so it's currently `v12.0` or nothing... _edit 2_ : after following instructions from @t-mat , and installing `clang-11.1` _on its own_ (not as part of MSVC) and using it to compile `xxhash`, I can finally reproduce the reported warning. Gosh, there are _so many ways_ to use and install variants of `clang` on the same system, and they also differ in the outcome of their warnings ....

👀 1

t-mat commented 2021-11-25 22:30:44 -06:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Gosh, there are so many ways to use and install variants of clang on the same system, and they also differ in the outcome of their warnings ....

Sounds like nightmare or episode from X-Files.

I'll make a free time to find out how we can (or can not) use clang-cl as a vanilla clang or vice versa.

Here are some notes for further investigation.

• Don't forget to play the X-Files theme.
• clang-13.0 reports same warnings/errors.
• According to "users manual of clang-cl", we can add -v (verbose) option to display actual options for clang. For example

cmake .. -DCMAKE_BUILD_TYPE=Release -A x64 -DXXHASH_C_FLAGS="-v" -DCMAKE_GENERATOR_TOOLSET=ClangCL
cmake --build . --config Release

• When we give -Wall to clang-cl, it translates -Wall to -Weverything.
• If we want to give -Wall to clang, we must use /clang:. For example

cmake .. -DCMAKE_BUILD_TYPE=Release -A x64 -DXXHASH_C_FLAGS="-v /clang:-Werror /clang:-Wextra /clang:-Wsign-compare" -DCMAKE_GENERATOR_TOOLSET=ClangCL
cmake --build . --config Release

• As @Cyan4973 observed, clang-cl doesn't report -Wsign-compare warning even if we give -v /clang:-Wsign-compare to clang-cl directly.

> Gosh, there are so many ways to use and install variants of clang on the same system, and they also differ in the outcome of their warnings .... Sounds like nightmare or episode from X-Files. I'll make a free time to find out how we can (or can not) use `clang-cl` as a *vanilla* `clang` or vice versa. Here are some notes for further investigation. - *Don't forget to play the X-Files theme.* - [`clang-13.0`](https://github.com/llvm/llvm-project/releases/tag/llvmorg-13.0.0) reports same warnings/errors. - According to ["users manual of `clang-cl`"](https://clang.llvm.org/docs/UsersManual.html#clang-cl), we can add `-v` (verbose) option to display actual options for `clang`. For example ``` cmake .. -DCMAKE_BUILD_TYPE=Release -A x64 -DXXHASH_C_FLAGS="-v" -DCMAKE_GENERATOR_TOOLSET=ClangCL cmake --build . --config Release ``` - When we give `-Wall` to `clang-cl`, it translates `-Wall` to `-Weverything`. - If we want to give `-Wall` to `clang`, we must use [`/clang:`](https://clang.llvm.org/docs/UsersManual.html#the-clang-option). For example ``` cmake .. -DCMAKE_BUILD_TYPE=Release -A x64 -DXXHASH_C_FLAGS="-v /clang:-Werror /clang:-Wextra /clang:-Wsign-compare" -DCMAKE_GENERATOR_TOOLSET=ClangCL cmake --build . --config Release ``` - As @Cyan4973 observed, `clang-cl` doesn't report `-Wsign-compare` warning even if we give `-v /clang:-Wsign-compare` to `clang-cl` directly.

👍 1 😆 1

Sign in to join this conversation.

No Branch/Tag specified

Branches Tags

dev

dependabot/github_actions/actions/upload-artifact-7.0.2

dependabot/github_actions/actions/download-artifact-8.0.2

dependabot/github_actions/actions/setup-node-7.1.0

archive/20261011T062114Z-rescue/fhgffy/fix/cmake-dispatch-off

dependabot/github_actions/msys2/setup-msys2-2.33.0

archive/20261011T062114Z-rescue/filtede98/fix-null-guards

ci/release-windows

fix/mingw-exe-symlink

gh-pages

release

ci/pin-actions

docs/refresh-readme

archive/20261011T062114Z-recovered/docs/refresh-readme

v0.8.4

v0.8.3

v0.8.2

v0.8.1

v0.8.0

v0.7.4

v0.7.3

v0.7.2

v0.7.1

v0.7.0

graphs

v0.6.5

v0.6.4

v0.6.3

v0.6.2

v0.6.1

v0.6.0

v0.5.1

v0.5.0

r42

r41

r40

r39

r38

r37

r36

r35

Labels

Clear labels   

announcement

  

bug

  

build

  

CI

Continuous Integration tests

  

dependencies

Pull requests that update a dependency file

  

documentation

  

duplicate

  

enhancement

  

experience feedback

  

feature request

  

github_actions

Pull requests that update GitHub Actions code

  

help wanted

  

install

  

invalid

  

Long-term

  

optimization

  

portability

  

question

  

target-specific

issue triggered in a specific environment (harware, compiler, OS)

  

test issue

  

wontfix

No labels

announcement

bug

build

CI

dependencies

documentation

duplicate

enhancement

experience feedback

feature request

github_actions

help wanted

install

invalid

Long-term

optimization

portability

question

target-specific

test issue

wontfix

Milestone

Clear milestone

No items

No milestone

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

gh-Cyan4973/xxHash#588

No description provided.