Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmake, ci: Bump clang version to 15 #159

Merged
merged 2 commits into from Apr 29, 2024
Merged

cmake, ci: Bump clang version to 15 #159

merged 2 commits into from Apr 29, 2024

Conversation

hebasto
Copy link
Owner

@hebasto hebasto commented Apr 21, 2024

@fanquake
Copy link

Is this being moved away from our minimum on purpose? Otherwise I don't see why this is needed, or how bitcoin#29859 is related to unreachable code warnings.

@hebasto
Copy link
Owner Author

hebasto commented Apr 24, 2024

Is this being moved away from our minimum on purpose?

These CI tasks are to test CMake-specific points of the build system rather than compiler support in general. And .github/workflows/cmake.yml won't go to the master branch.

Otherwise I don't see why this is needed

To keep CI config simpler.

how bitcoin#29859 is related to unreachable code warnings.

bitcoin#29859 is related to compiling with clang-15. And unreachable code warnings are observable on clang-14.

@vasild
Copy link

vasild commented Apr 25, 2024

FWIW on latest master @ 2a07c46 with clang 19 I see:

clang++-devel -std=c++20 -DHAVE_CONFIG_H -I. -I../src/config  -DDEBUG -DDEBUG_LOCKORDER -DDEBUG_LOCKCONTENTION -DRPC_DOC_CHECK -DABORT_ON_FAILED_ASSUME  -DHAVE_BUILD_INFO -DPROVIDE_FUZZ_MAIN_FUNCTION -I. -I./minisketch/include -I./secp256k1/include -I./univalue/include  -isystem /usr/local/include -DBOOST_MULTI_INDEX_DISABLE_SERIALIZATION -isystem /usr/local/include  -I/usr/local/include -g -O2 -O0 -g3 -ftrapv -Wstack-protector -fstack-protector-all -fcf-protection=full -fstack-clash-protection -Wall -Wextra -Wgnu -Wformat -Wformat-security -Wvla -Wshadow-field -Wthread-safety -Wloop-analysis -Wredundant-decls -Wunused-member-function -Wdate-time -Wconditional-uninitialized -Woverloaded-virtual -Wsuggest-override -Wimplicit-fallthrough -Wunreachable-code -Wdocumentation -Wno-unused-parameter -Wno-self-assign -Werror   -fPIE  -MT test/test_bitcoin-util_threadnames_tests.o -MD -MP -MF test/.deps/test_bitcoin-util_threadnames_tests.Tpo -c -o test/test_bitcoin-util_threadnames_tests.o `test -f 'test/util_threadnames_tests.cpp' || echo './'`test/util_threadnames_tests.cpp

test/util_threadnames_tests.cpp:62:35: error: code will never be executed [-Werror,-Wunreachable-code]
   62 |     std::set<std::string> names = RenameEnMasse(100);
      |                                   ^~~~~~~~~~~~~

@fanquake
Copy link

I'm assuming that's on FreeBSD? If so, I assume it's because we currently outright disable thread local.

@vasild
Copy link

vasild commented Apr 25, 2024

@fanquake, yes to both.

@hebasto
Copy link
Owner Author

hebasto commented Apr 25, 2024

Rebased to resolve conflicts.

@hebasto hebasto changed the title cmake, ci: Bump clang to avoid -Wunreachable-code warnings. cmake, ci: Bump clang version to 15 Apr 29, 2024
@hebasto
Copy link
Owner Author

hebasto commented Apr 29, 2024

I've updated PR title and description in the light of the merged bitcoin#29165.

@hebasto
Copy link
Owner Author

hebasto commented Apr 29, 2024

Bump clang version to 15.

Drop jobs with clang 14.
On Ubuntu 22.04, the default gcc is 11.2.0.
@m3dwards
Copy link

utACK 8fd40d4

@hebasto hebasto merged commit 4561f76 into cmake-staging Apr 29, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants