dvidelabs / flatcc

FlatBuffers Compiler and Library in C for C
Apache License 2.0
646 stars 182 forks source link

disable pedantic for GCC >= 8 #228

Closed le91688 closed 2 years ago

le91688 commented 2 years ago

disable -pedantic flag for GCC version > 8

mikkelfj commented 2 years ago

No that is orthogonal.

You can probably make an if then else construct here:

https://github.com/dvidelabs/flatcc/blob/07ae7dca8118f9ab6d900a7d4797881cab708ba6/CMakeLists.txt#L211-L213

PJ-Zetier commented 2 years ago

No that is orthogonal.

You can probably make an if then else construct here:

https://github.com/dvidelabs/flatcc/blob/07ae7dca8118f9ab6d900a7d4797881cab708ba6/CMakeLists.txt#L211-L213

I thought the issue was happening with clang?

le91688 commented 2 years ago

No that is orthogonal.

You can probably make an if then else construct here:

https://github.com/dvidelabs/flatcc/blob/07ae7dca8118f9ab6d900a7d4797881cab708ba6/CMakeLists.txt#L211-L213

Ah my apologies, should have waited for your response in the issue thread!

Im happy to address the GCC version flag disable, but what i was hoping to accomplish was a generalized way to disable pedantic ( in my particular case, im struggling getting flatcc runtime to build for linux-arm / linux-mips with Clang )

mikkelfj commented 2 years ago

We could a add a FLATCC_ flag to disable pedantic. The problem is that every time there is a new GCC release, one or more users report some issue that is is a complete non-issue, and it needs to be investigated and GCC needs to be silenced. And with a flag, the user then needs to be told to add that flag to the build environment. GCC pendantic is just broken in my view.

mikkelfj commented 2 years ago

( in my particular case, im struggling getting flatcc runtime to build for linux-arm / linux-mips with Clang )

Are you saying this a pedantic error raised by Clang? In that case we need to fix the source of the problem in pattributes because we need clang pedantic since it usually find problematic areas. This is also why we can afford to remove on GCC.

le91688 commented 2 years ago

We could a add a FLATCC_ flag to disable pedantic. The problem is that every time there is a new GCC release, one or more users report some issue that is is a complete non-issue, and it needs to be investigated and GCC needs to be silenced. And with a flag, the user then needs to be told to add that flag to the build environment. GCC pendantic is just broken in my view.

Understood. How about a FLATCC_NOPEDANTIC flag, and we set that flag for GCC versions over a specified target?

le91688 commented 2 years ago

( in my particular case, im struggling getting flatcc runtime to build for linux-arm / linux-mips with Clang )

Are you saying this a pedantic error raised by Clang? In that case we need to fix the source of the problem in pattributes because we need clang pedantic since it usually find problematic areas. This is also why we can afford to remove on GCC.

Yes, thats correct, this is a clang error im experiencing with fall through.

mikkelfj commented 2 years ago

OK, lets take this dicussion back to the issue.

le91688 commented 2 years ago

@mikkelfj updated as per our discussion

mikkelfj commented 2 years ago

Can you test this with a few different GCC versions? The Travis build has stopped working so I don't have any insights on CI.

le91688 commented 2 years ago

@mikkelfj , tested this branch (via rikorose/gcc-cmake docker images) gcc 5 , gcc 8, and gcc 11

command used: cmake -DFLATCC_INSTALL=on -DCMAKE_INSTALL_PREFIX="/test/" -DFLATCC_RTONLY=on -DCMAKE_BUILD_TYPE=Release && make install

all successful, and verified cmake message for >= 8 prints on appropriate versions

Also verified master fails on gcc-11

-- GCC_VERSION: 11.1.0                                                                                                                            

-- Configured C_FLAGS:  -DFLATCC_REFLECTION=1 -std=c11 -pedantic -Wall -Wextra -Wno-stringop-truncation -Wno-format-overflow -Wno-misleading-inden
tation -DPORTABLE_POSIX_MEMALIGN=1 -Werror -Wno-unused-function -Wsign-conversion                                                                 
-- Configuring done                                                                                                                               
-- Generating done                                                                                                                                
-- Build files have been written to: /flatcc                                                                                                      
[ 14%] Building C object src/runtime/CMakeFiles/flatccrt.dir/builder.c.o                                                                          
[ 28%] Building C object src/runtime/CMakeFiles/flatccrt.dir/emitter.c.o                                                                          
[ 42%] Building C object src/runtime/CMakeFiles/flatccrt.dir/refmap.c.o                                                                           
[ 57%] Building C object src/runtime/CMakeFiles/flatccrt.dir/verifier.c.o                                                                         
[ 71%] Building C object src/runtime/CMakeFiles/flatccrt.dir/json_parser.c.o                                                                      
In file included from /flatcc/include/flatcc/flatcc_flatbuffers.h:27,                                                                             
                 from /flatcc/include/flatcc/flatcc_builder.h:68,                                                                                 
                 from /flatcc/include/flatcc/flatcc_json_parser.h:19,                                                                             
                 from /flatcc/src/runtime/json_parser.c:2:                                                                                        
/flatcc/include/flatcc/flatcc_json_parser.h: In function 'flatcc_json_parser_symbol_part_ext':                                                    
/flatcc/include/flatcc/portable/pattributes.h:61:33: error: ISO C does not support '[[]]' attributes before C2X [-Werror=pedantic]                
   61 | # define pattribute_fallthrough [[__fallthrough__]]                                                               
mikkelfj commented 2 years ago

Thanks. It would be better if you could run the full tests, or at least a build without RTONLY, see scripts/test.sh, but at least it shows the compilers don't go belly up.

le91688 commented 2 years ago

Thanks. It would be better if you could run the full tests, or at least a build without RTONLY, see scripts/test.sh, but at least it shows the compilers don't go belly up.

sure i can do that quick

le91688 commented 2 years ago

Tested with gcc 5 and gcc 11, running scripts/test.sh successfully

mikkelfj commented 2 years ago

Thanks, looks good!

mikkelfj commented 2 years ago

@pjr-z I missed your comment on clang, could have saved some effort, thanks for the heads up tho :) (even though in the end, it was GCC after all ...)