cubing / twsearch

🔍 Twizzle Search — a program to find algs and scrambles for twisty puzzles
GNU General Public License v3.0
24 stars 8 forks source link

Add `-Werror` to the list of `CXXFLAGS` #40

Closed jfly closed 7 months ago

jfly commented 7 months ago

How strict do we want to be in this repo?

Before

$ make build
...
g++ -I./src/cpp/cityhash/src -c -O3 -Wextra -Wall -pedantic -std=c++17 -g -Wsign-compare -DTWSEARCH_VERSION=v0.6.4-50-gdc1ec4e -DUSE_PTHREADS -DHAVE_FFSLL src/cpp/coset.cpp -o build/cpp/coset.o
src/cpp/coset.cpp: In function ‘int cosetcallback(setval, const std::vector<int>&, int, int)’:
src/cpp/coset.cpp:111:11: warning: array subscript 1 is outside array bounds of ‘setval [1]’ [-Warray-bounds]
  111 |     domove(pd, pos, moves[i], *(&pos + 1));
      |     ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/cpp/coset.cpp:98:26: note: at offset 8 into object ‘pos’ of size 8
   98 | int cosetcallback(setval pos, const vector<int> &moves, int d, int id) {
      |                   ~~~~~~~^~~
...
g++ -O3 -Wextra -Wall -pedantic -std=c++17 -g -Wsign-compare -o build/bin/twsearch build/cpp/antipode.o build/cpp/calcsymm.o build/cpp/canon.o build/cpp/cmdlineops.o build/cpp/filtermoves.o build/cpp/findalgo.o build/cpp/generatingset.o build/cpp/god.o build/cpp/index.o build/cpp/parsemoves.o build/cpp/prunetable.o build/cpp/puzdef.o build/cpp/readksolve.o build/cpp/solve.o build/cpp/test.o build/cpp/threads.o build/cpp/twsearch.o build/cpp/util.o build/cpp/workchunks.o build/cpp/rotations.o build/cpp/orderedgs.o build/cpp/ffi/ffi_api.o build/cpp/ffi/wasm_api.o build/cpp/city.o build/cpp/coset.o build/cpp/descsets.o build/cpp/ordertree.o build/cpp/unrotate.o build/cpp/shorten.o -lpthread

After

$ make build
...
src/cpp/coset.cpp: In function ‘int cosetcallback(setval, const std::vector<int>&, int, int)’:
src/cpp/coset.cpp:111:11: error: array subscript 1 is outside array bounds of ‘setval [1]’ [-Werror=array-bounds]
  111 |     domove(pd, pos, moves[i], *(&pos + 1));
      |     ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/cpp/coset.cpp:98:26: note: at offset 8 into object ‘pos’ of size 8
   98 | int cosetcallback(setval pos, const vector<int> &moves, int d, int id) {
      |                   ~~~~~~~^~~
cc1plus: all warnings being treated as errors
make: *** [Makefile:110: build/cpp/coset.o] Error 1
jfly commented 7 months ago

Strange. CI failed against cpp-windows, but not plain old cpp. Maybe we use a different version of gcc than I do.

I also see CI littered with a bunch of these scary looking logs:

g++ -I./src/cpp/cityhash/src -c -O3 -Wextra -Werror -Wall -pedantic -std=c++17 -g -Wsign-compare -DTWSEARCH_VERSION= -DUSE_PTHREADS -DHAVE_FFSLL src/cpp/util.cpp -o build/cpp/util.o
fatal: No names found, cannot describe anything.

I'll look into this later today.

lgarron commented 7 months ago

How strict do we want to be in this repo?

I'm generally a fan of fairly strict code checking, but unlike some other ecosystems it's not straightforward to pin the checks in CI. As you note, even getting consistent behaviour across OSes is a task.

I also see CI littered with a bunch of these scary looking logs:

g++ -I./src/cpp/cityhash/src -c -O3 -Wextra -Werror -Wall -pedantic -std=c++17 -g -Wsign-compare -DTWSEARCH_VERSION= -DUSE_PTHREADS -DHAVE_FFSLL src/cpp/util.cpp -o build/cpp/util.o
fatal: No names found, cannot describe anything.

I'll look into this later today.

That would be this: https://github.com/cubing/twsearch/issues/9

jfly commented 7 months ago

Closing this in favor of https://github.com/cubing/twsearch/commit/89ead3c3748bcbc3348fc2e877167e25185d56c0.

We can decide some other time if we want to go as extreme as -Werror.