dvidelabs / flatcc

FlatBuffers Compiler and Library in C for C
Apache License 2.0
632 stars 180 forks source link

Remove strict prototypes from clang build #202

Closed danhorner closed 2 years ago

danhorner commented 2 years ago

The build was failing on test functions missing prototypes.

Sample:

FAILED: test/json_test/CMakeFiles/json_test_uq.dir/test_json.c.o
/Library/Developer/CommandLineTools/usr/bin/cc  -Itest/json_test/generated -I../../include -DFLATCC_REFLECTION=1 -Wstrict-prototypes -Wsign-conversion -Wconversion -std=c11 -pedantic -Wall -Wextra -Werror -g -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX11.3.sdk -DFLATCC_JSON_PARSE_ALLOW_UNQUOTED=1 -MD -MT test/json_test/CMakeFiles/json_test_uq.dir/test_json.c.o -MF test/json_test/CMakeFiles/json_test_uq.dir/test_json.c.o.d -o test/json_test/CMakeFiles/json_test_uq.dir/test_json.c.o -c ../../test/json_test/test_json.c
../../test/json_test/test_json.c:142:20: error: this old-style function definition is not preceded by a prototype [-Werror,-Wstrict-prototypes]
int edge_case_tests()
mikkelfj commented 2 years ago

Thanks for providing a fix for this.

However, the entire build should not be affected by a test case. I think the actual problem is that

int edge_case_tests()
{
...
}

should have been

int edge_case_tests(void)
{
...
}

Can you please check that, and update the PR accordingly if it works.

mikkelfj commented 2 years ago

More specifically here, as reported by the error message:

test/json_test/test_json.c:142:20

danhorner commented 2 years ago

Sure. Btw, I notice the flag is commented out on the gcc build with a note to similar effect, so you might find more of these as tests are added by gcc users:

https://github.com/dvidelabs/flatcc/blob/5bc7c3512109e5fd6ea231fe1d76f09e1ce7a24e/CMakeLists.txt#L244-L246

mikkelfj commented 2 years ago

Thanks.

The thing is, it is useful to have very strict compiler settings as long as they are reasonable because it ensures maximum portability. Otherwise there would be many more issues like the one you just reported because everyone has their policies and compiler versions.

Clang is generally fair. GCC assumes that you only use GCC and adapt to their conventions to a point where you either break code on other compilers, or make the code very ugly. That is why strict settings has to go on GCC and we rely on Clang to ensure the code remains relatively clean.