dvidelabs/flatcc

flatcc cli compiles with clang but not with gcc on Linux

MartinKlang opened this issue · 8 comments

Current master branch doesn't build with gcc without -Wno-sign-conversion:

$ CC=gcc cmake ..
$ make
...
[ 16%] Building C object src/compiler/CMakeFiles/flatcc.dir/semantics.c.o
/home/mars/devel/evp-device-agent/evp_agent/sdkenc/flatcc/src/compiler/semantics.c: In function ‘analyze_struct’:
/home/mars/devel/evp-device-agent/evp_agent/sdkenc/flatcc/src/compiler/semantics.c:620:25: error: unsigned conversion from ‘int’ to ‘uint16_t’ {aka ‘short unsigned int’} changes the value of ‘-2’ [-Werror=sign-conversion]
  620 |     ct->symbol.flags &= ~fb_circular_open;
      |                         ^
cc1: all warnings being treated as errors
...

It does build with clang:

$ CC=clang cmake ..
$ make
...
[ 16%] Building C object src/compiler/CMakeFiles/flatcc.dir/semantics.c.o
[ 17%] Building C object src/compiler/CMakeFiles/flatcc.dir/coerce.c.o
[ 18%] Building C object src/compiler/CMakeFiles/flatcc.dir/flatcc.c.o
...
[100%] Built target bfbs2json

I'm on Ubuntu 22.04, gcc 11.2.0, clang 14.0.0, cmake 3.22.1, GNU Make 4.3

Normally I would silence gcc warnings, but this error / warning is sort of correct since you can have a large flag that appears negative.

There are many places where flags are defined as integer enums that ought to have been unsigned so I'm not sure if it makes sense to change them all.

Just searching for fb_circular_open in repo also indicates that |= and & operators are being used on this flag but I don't know if you get errors here.

In the simplest case this can be fixed be casting just the problematic ~ operator.

How does it look on your side if you fix that line?
And, are there more errors or warnings if you run scripts/test.sh and inspect output?

Also, even if it is unsigned, it would still be a truncation to uint16_t on most platforms, so a cast is probably needed regardless.

It's just that one line, if I cast to uint16_t then it builds correctly.

With the cast, the tests pass

CXX=g++ CC=gcc scripts/test.sh
...
-- The C compiler identification is GNU 11.2.0
-- The CXX compiler identification is GNU 11.2.0
...
100% tests passed, 0 tests failed out of 22

Total Test time (real) =   0.46 sec
TEST PASSED
diff --git a/src/compiler/semantics.c b/src/compiler/semantics.c
index 96d4b94..09b98d3 100644
--- a/src/compiler/semantics.c
+++ b/src/compiler/semantics.c
@@ -617,7 +617,7 @@ static int analyze_struct(fb_parser_t *P, fb_compound_type_t *ct)
     }
 
     ct->symbol.flags |= fb_circular_closed;
-    ct->symbol.flags &= ~fb_circular_open;
+    ct->symbol.flags &= (uint16_t)~fb_circular_open;
     ct->order = P->schema.ordered_structs;
     P->schema.ordered_structs = ct;
     return ret;

Please test master, just to be sure, applied as suggested.

yep that works, ty!

Thanks