Build errors with GCC 5

[rotu]

afsctool fails to build with GCC 5. This seems to be due to some nonstandard language extensions. In particular __has_builtin (added in GCC 6).
And initializing an array with a const int length; const int array[length] = ...; (see, e.g., here)

/home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/shims/linux/super/g++-5 -DSUPPORT_PARALLEL -DZFSCTOOL_PROG_NAME="zfsctool" -I/home/linuxbrew/.linuxbrew/Cellar/zlib/1.2.11/include -I/home/linuxbrew/.linuxbrew/Cellar/google-sparsehash/2.0.4/include -I/tmp/afsctool-20220303-4478-1g0rh9p/afsctool-1.7.2 -I/tmp/afsctool-20220303-4478-1g0rh9p/afsctool-1.7.2/src -m64 -O3 -DNDEBUG -std=gnu++11 -MD -MT CMakeFiles/zfsctool.dir/src/zfsctool.cpp.o -MF CMakeFiles/zfsctool.dir/src/zfsctool.cpp.o.d -o CMakeFiles/zfsctool.dir/src/zfsctool.cpp.o -c /tmp/afsctool-20220303-4478-1g0rh9p/afsctool-1.7.2/src/zfsctool.cpp
/tmp/afsctool-20220303-4478-1g0rh9p/afsctool-1.7.2/src/afsctool.c:90:13: error: variably modified ‘sizeunit10_short’ at file scope
const char *sizeunit10_short[sizeunits] = {"KB", "MB", "GB", "TB", "PB", "EB"};
^
/tmp/afsctool-20220303-4478-1g0rh9p/afsctool-1.7.2/src/afsctool.c:91:13: error: variably modified ‘sizeunit10_long’ at file scope
const char *sizeunit10_long[sizeunits] = {"kilobytes", "megabytes", "gigabytes", "terabytes", "petabytes", "exabytes"};
^
/tmp/afsctool-20220303-4478-1g0rh9p/afsctool-1.7.2/src/afsctool.c:92:21: error: variably modified ‘sizeunit10’ at file scope
const long long int sizeunit10[sizeunits] = {1000, 1000 * 1000, 1000 * 1000 * 1000, (long long int) 1000 * 1000 * 1000 * 1000,
^
/tmp/afsctool-20220303-4478-1g0rh9p/afsctool-1.7.2/src/afsctool.c:94:13: error: variably modified ‘sizeunit2_short’ at file scope
const char *sizeunit2_short[sizeunits] = {"KiB", "MiB", "GiB", "TiB", "PiB", "EiB"};
^
/tmp/afsctool-20220303-4478-1g0rh9p/afsctool-1.7.2/src/afsctool.c:95:13: error: variably modified ‘sizeunit2_long’ at file scope
const char *sizeunit2_long[sizeunits] = {"kibibytes", "mebibytes", "gibibytes", "tebibytes", "pebibytes", "exbibytes"};
^
/tmp/afsctool-20220303-4478-1g0rh9p/afsctool-1.7.2/src/afsctool.c:96:21: error: variably modified ‘sizeunit2’ at file scope
const long long int sizeunit2[sizeunits] = {1024, 1024 * 1024, 1024 * 1024 * 1024, (long long int) 1024 * 1024 * 1024 * 1024,
^
/tmp/afsctool-20220303-4478-1g0rh9p/afsctool-1.7.2/src/afsctool.c:103:19: error: missing binary operator before token "("
#if !__has_builtin(__builtin_available)
^
/tmp/afsctool-20220303-4478-1g0rh9p/afsctool-1.7.2/src/afsctool.c: In function ‘afsctool’:
/tmp/afsctool-20220303-4478-1g0rh9p/afsctool-1.7.2/src/afsctool.c:2483:19: error: missing binary operator before token "("
#if !__has_builtin(__builtin_available)
^
CMakeFiles/afsctool.dir/build.make:78: recipe for target 'CMakeFiles/afsctool.dir/src/afsctool.c.o' failed
make[2]: *** [CMakeFiles/afsctool.dir/src/afsctool.c.o] Error 1
make[2]: Leaving directory '/tmp/afsctool-20220303-4478-1g0rh9p/afsctool-1.7.2'
CMakeFiles/Makefile2:115: recipe for target 'CMakeFiles/afsctool.dir/all' failed
make[1]: *** [CMakeFiles/afsctool.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....
[ 81%] Linking CXX executable zfsctool
/home/linuxbrew/.linuxbrew/Cellar/cmake/3.22.2/bin/cmake -E cmake_link_script CMakeFiles/zfsctool.dir/link.txt --verbose=1
/home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/shims/linux/super/g++-5 -m64 -O3 -DNDEBUG -rdynamic CMakeFiles/zfsctool.dir/src/zfsctool.cpp.o CMakeFiles/PP.dir/src/utils.cpp.o CMakeFiles/PP.dir/src/ParallelProcess.cpp.o CMakeFiles/PP.dir/src/Thread/Thread.cpp.o CMakeFiles/PP.dir/src/CritSectEx/CritSectEx.cpp.o CMakeFiles/PP.dir/src/CritSectEx/msemul.cpp.o CMakeFiles/PP.dir/src/CritSectEx/timing.c.o -o zfsctool -lrt -ldl -lbsd -pthread
make[2]: Leaving directory '/tmp/afsctool-20220303-4478-1g0rh9p/afsctool-1.7.2'
[ 81%] Built target zfsctool
make[1]: Leaving directory '/tmp/afsctool-20220303-4478-1g0rh9p/afsctool-1.7.2'
Makefile:93: recipe for target 'all' failed
make: *** [all] Error 2

6_tests (ubuntu-latest, ghcr.iohomebrewubuntu16.04master, --.txt

[danielnachun]

I believe you can fix the variably modified X at file scope errors by using enum rather than const char or const long long int: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93082. Apparently this was allowed in GCC at some point, but then they became stricter and now it fails, and the clang upstream seems to think they should follow GCC in doing this: https://bugs.llvm.org/show_bug.cgi?id=44406.

[RJVB]

Or maybe I just remove the const or specify a C standard which doesn't include this newfangled protection for peopple who're not aware that C can bite :) However, why compile this with an ancient compiler like GCC 5? Even on the oldest OS X where afsctool makes sense you should be able to run a more recent and more "ecosystem appropriate" clang version...

[rotu]

It’s not newfangled. That it compiles at all is against the C standard. Clang allowing it is accidental. I don’t know if newer GCC supports it.

I ran into this on Linux, not Mac. Maybe it makes sense to make afsctool a Mac-only program and support zfsctool on both platforms? (though right now the code seems to suggest that they should both run on Linux)

[RJVB]

But which C standard?

[danielnachun]

We actually can use GCC 11 to build in Homebrew, but the error still happens there. According to the Clang issue linked above, it seems this may have been allowed in GCC 4, and Clang followed their lead and allowed it as well. But then in GCC 5 they got stricter and didn't allow it anymore, but Clang didn't follow them in disabling it, possibly because it would break existing code.

You can disregard these errors as they go away when we use GCC 11:

/tmp/afsctool-20220303-4478-1g0rh9p/afsctool-1.7.2/src/afsctool.c:103:19: error: missing binary operator before token "("
#if !__has_builtin(__builtin_available)
^
/tmp/afsctool-20220303-4478-1g0rh9p/afsctool-1.7.2/src/afsctool.c: In function ‘afsctool’:
/tmp/afsctool-20220303-4478-1g0rh9p/afsctool-1.7.2/src/afsctool.c:2483:19: error: missing binary operator before token "("
#if !__has_builtin(__builtin_available)
^

[rotu]

But which C standard?

C++17 C17, for one. http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2310.pdf Page 96.
I think it would fix the problem to replace sizeunit10_long[sizeunits] = … with sizeunit10_long[] = …. That way it’s a partial type, not a VLA.

[RJVB]

If you meant C17 that's a standard from 2017, like C++17 - and thus indeed newfangled compared to the age of the code in question and even more so compared to when I started coding. Anyway, as far as I understand the issues that could arise from using `const char *foo = ["boo", "bah"];` are all moot in afsctool. I cannot remember if I introduced the const qualifier or it was already in the code, the idea must have been give the optimiser a hint. If the language doesn't make use of that I can just as well remove the qualifiers and be done with it because the potential gains must be academic at best here.

[rotu]

Sorry yes, I meant C17. It has been illegal from when VLA's were introduced in C99 (http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf Page 118)

I'm in favor of just removing sizeunits :-)

[danielnachun]

@RJVB if it's possible for this fix to be added as a commit we can then use that commit to as a patch for our package. If you think it warrants a new release, we can also bump the version up to that new release.