From 9a5ff8900303fa7abe777c8646987f90c5da915f Mon Sep 17 00:00:00 2001 From: Derek Bailey Date: Sat, 6 Aug 2022 21:06:14 -0700 Subject: [PATCH] Add FLATBUFFERS_STRICT_MODE (#7408) --- .github/workflows/build.yml | 22 +++++++++++----------- .github/workflows/codeql.yml | 2 +- CMakeLists.txt | 28 ++++++++++++++++++++-------- docs/source/Building.md | 28 +++++++++++++--------------- 4 files changed, 45 insertions(+), 35 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 2e409ad8e..b0f66d41c 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -19,7 +19,7 @@ jobs: steps: - uses: actions/checkout@v2 - name: cmake - run: CXX=${{ matrix.cxx }} cmake -G "Unix Makefiles" -DCMAKE_BUILD_TYPE=Release . + run: CXX=${{ matrix.cxx }} cmake -G "Unix Makefiles" -DCMAKE_BUILD_TYPE=Release -DFLATBUFFERS_STRICT_MODE=ON . - name: build run: make -j - name: test @@ -42,7 +42,7 @@ jobs: - name: Add msbuild to PATH uses: microsoft/setup-msbuild@v1.1 - name: cmake - run: cmake -G "Visual Studio 16 2019" -A x64 -DCMAKE_BUILD_TYPE=Release -DFLATBUFFERS_BUILD_CPP17=ON . + run: cmake -G "Visual Studio 16 2019" -A x64 -DCMAKE_BUILD_TYPE=Release -DFLATBUFFERS_BUILD_CPP17=ON -DFLATBUFFERS_STRICT_MODE=ON . - name: build run: msbuild.exe FlatBuffers.sln /p:Configuration=Release /p:Platform=x64 - name: test @@ -61,7 +61,7 @@ jobs: - name: Add msbuild to PATH uses: microsoft/setup-msbuild@v1.1 - name: cmake - run: cmake -G "Visual Studio 16 2019" -A x64 -DCMAKE_BUILD_TYPE=Release . + run: cmake -G "Visual Studio 16 2019" -A x64 -DCMAKE_BUILD_TYPE=Release -DFLATBUFFERS_STRICT_MODE=ON . - name: build tool version 15 (VS 2017) run: msbuild.exe FlatBuffers.sln /p:Configuration=Release /p:Platform=x64 /p:VisualStudioVersion=15.0 - name: test @@ -75,7 +75,7 @@ jobs: - name: Add msbuild to PATH uses: microsoft/setup-msbuild@v1.1 - name: cmake - run: cmake -G "Visual Studio 14 2015" -A x64 -DCMAKE_BUILD_TYPE=Release . + run: cmake -G "Visual Studio 14 2015" -A x64 -DCMAKE_BUILD_TYPE=Release -DFLATBUFFERS_STRICT_MODE=ON . - name: build tool version 14 (VS 2015) run: msbuild.exe FlatBuffers.sln /p:Configuration=Release /p:Platform=x64 /p:VisualStudioVersion=14.0 - name: test @@ -115,7 +115,7 @@ jobs: steps: - uses: actions/checkout@v2 - name: cmake - run: cmake -G "Xcode" -DCMAKE_BUILD_TYPE=Release -DFLATBUFFERS_FLATC_EXECUTABLE=_build/Release/flatc . + run: cmake -G "Xcode" -DCMAKE_BUILD_TYPE=Release -DFLATBUFFERS_FLATC_EXECUTABLE=_build/Release/flatc -DFLATBUFFERS_STRICT_MODE=ON . - name: build # NOTE: we need this _build dir to not have xcodebuild's default ./build dir clash with the BUILD file. run: xcodebuild -toolchain clang -configuration Release -target flattests SYMROOT=$(PWD)/_build @@ -142,7 +142,7 @@ jobs: steps: - uses: actions/checkout@v2 - name: cmake - run: cmake -G "Xcode" -DCMAKE_OSX_ARCHITECTURES="arm64;x86_64" -DCMAKE_BUILD_TYPE=Release -DFLATBUFFERS_FLATC_EXECUTABLE=_build/Release/flatc . + run: cmake -G "Xcode" -DCMAKE_OSX_ARCHITECTURES="arm64;x86_64" -DCMAKE_BUILD_TYPE=Release -DFLATBUFFERS_FLATC_EXECUTABLE=_build/Release/flatc -DFLATBUFFERS_STRICT_MODE=ON . - name: build # NOTE: we need this _build dir to not have xcodebuild's default ./build dir clash with the BUILD file. run: xcodebuild -toolchain clang -configuration Release -target flattests SYMROOT=$(PWD)/_build @@ -176,7 +176,7 @@ jobs: java-version: 1.8 - name: set up flatc run: | - cmake -DFLATBUFFERS_BUILD_TESTS=OFF -DFLATBUFFERS_BUILD_FLATLIB=OFF -DFLATBUFFERS_BUILD_FLATHASH=OFF . + cmake -DFLATBUFFERS_BUILD_TESTS=OFF -DFLATBUFFERS_BUILD_FLATLIB=OFF -DFLATBUFFERS_BUILD_FLATHASH=OFF -DFLATBUFFERS_STRICT_MODE=ON . make -j echo "${PWD}" >> $GITHUB_PATH - name: build @@ -192,7 +192,7 @@ jobs: steps: - uses: actions/checkout@v2 - name: cmake - run: CXX=${{ matrix.cxx }} cmake -G "Unix Makefiles" -DFLATBUFFERS_BUILD_TESTS=OFF -DCMAKE_BUILD_TYPE=Release . && make -j + run: CXX=${{ matrix.cxx }} cmake -G "Unix Makefiles" -DFLATBUFFERS_BUILD_TESTS=OFF -DCMAKE_BUILD_TYPE=Release -DFLATBUFFERS_STRICT_MODE=ON . && make -j - name: Generate run: scripts/check_generate_code.py - name: Generate gRPC @@ -207,7 +207,7 @@ jobs: steps: - uses: actions/checkout@v2 - name: cmake - run: CXX=${{ matrix.cxx }} cmake -G "Unix Makefiles" -DFLATBUFFERS_CXX_FLAGS="-Wno-unused-parameter -fno-aligned-new" -DFLATBUFFERS_BUILD_BENCHMARKS=ON -DCMAKE_BUILD_TYPE=Release . && make -j + run: CXX=${{ matrix.cxx }} cmake -G "Unix Makefiles" -DFLATBUFFERS_CXX_FLAGS="-Wno-unused-parameter -fno-aligned-new" -DFLATBUFFERS_BUILD_BENCHMARKS=ON -DCMAKE_BUILD_TYPE=Release -DFLATBUFFERS_STRICT_MODE=ON . && make -j - name: Run benchmarks run: ./flatbenchmark --benchmark_repetitions=5 --benchmark_display_aggregates_only=true --benchmark_out_format=console --benchmark_out=benchmarks/results_${{matrix.cxx}} - name: Upload benchmarks results @@ -283,7 +283,7 @@ jobs: - uses: actions/checkout@v2 - name: flatc # FIXME: make test script not rely on flatc - run: cmake -G "Unix Makefiles" -DCMAKE_BUILD_TYPE=Release -DFLATBUFFERS_BUILD_TESTS=OFF -DFLATBUFFERS_INSTALL=OFF -DFLATBUFFERS_BUILD_FLATLIB=OFF -DFLATBUFFERS_BUILD_FLATHASH=OFF . && make -j + run: cmake -G "Unix Makefiles" -DCMAKE_BUILD_TYPE=Release -DFLATBUFFERS_BUILD_TESTS=OFF -DFLATBUFFERS_INSTALL=OFF -DFLATBUFFERS_BUILD_FLATLIB=OFF -DFLATBUFFERS_BUILD_FLATHASH=OFF -DFLATBUFFERS_STRICT_MODE=ON . && make -j - name: test working-directory: tests run: bash GoTest.sh @@ -331,7 +331,7 @@ jobs: sdk: stable - name: flatc # FIXME: make test script not rely on flatc - run: cmake -G "Unix Makefiles" -DCMAKE_BUILD_TYPE=Release -DFLATBUFFERS_BUILD_TESTS=OFF -DFLATBUFFERS_INSTALL=OFF -DFLATBUFFERS_BUILD_FLATLIB=OFF -DFLATBUFFERS_BUILD_FLATHASH=OFF . && make -j + run: cmake -G "Unix Makefiles" -DCMAKE_BUILD_TYPE=Release -DFLATBUFFERS_BUILD_TESTS=OFF -DFLATBUFFERS_INSTALL=OFF -DFLATBUFFERS_BUILD_FLATLIB=OFF -DFLATBUFFERS_BUILD_FLATHASH=OFF -DFLATBUFFERS_STRICT_MODE=ON . && make -j - name: test working-directory: tests run: bash DartTest.sh diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index fd38f6ea3..8dcbab8ba 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -64,7 +64,7 @@ jobs: # uses a compiled language - run: | - cmake -G "Unix Makefiles" -DCMAKE_BUILD_TYPE=Release . + cmake -G "Unix Makefiles" -DCMAKE_BUILD_TYPE=Release -DFLATBUFFERS_STRICT_MODE=ON . make -j - name: Perform CodeQL Analysis diff --git a/CMakeLists.txt b/CMakeLists.txt index ff881c05e..98f0c4512 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -74,6 +74,9 @@ option(FLATBUFFERS_ENABLE_PCH option(FLATBUFFERS_SKIP_MONSTER_EXTRA "Skip generating monster_extra.fbs that contains non-supported numerical\" types." OFF) +option(FLATBUFFERS_STRICT_MODE + "Build flatbuffers with all warnings as errors (-Werror or /WX)." + OFF) set(MSVC_LIKE OFF) if(MSVC OR CMAKE_CXX_COMPILER_FRONTEND_VARIANT STREQUAL "MSVC") @@ -344,7 +347,9 @@ if(MSVC_LIKE) target_compile_options(ProjectConfig INTERFACE /W4 - /WX + $<$: + /WX # Treat all compiler warnings as errors + > /wd4512 # C4512: assignment operator could not be generated /wd4316 # C4316: object allocated on the heap may not be aligned /wd4456 # C4456: hides previous local declaration @@ -356,9 +361,10 @@ else() target_compile_options(ProjectConfig INTERFACE -Wall - -Werror + $<$: + -Werror # Treat all compiler warnings as errors + > -pedantic - -Werror -Wextra -Wno-unused-parameter -Wold-style-cast @@ -370,20 +376,26 @@ else() $<$,3.8>: -Wimplicit-fallthrough -Wextra-semi - -Werror=unused-private-field + $<$: + -Werror=unused-private-field + > > > $<$: $<$,4.4>: - -Wunused-result - -Werror=unused-result + -Wunused-result -Wunused-parameter - -Werror=unused-parameter + $<$: + -Werror=unused-result + -Werror=unused-parameter + > > $<$,7.0>: -faligned-new - -Werror=implicit-fallthrough=2 + $<$: + -Werror=implicit-fallthrough=2 + > > $<$,8.0>: -Wextra-semi diff --git a/docs/source/Building.md b/docs/source/Building.md index d7e9ca3ca..892cefc2d 100644 --- a/docs/source/Building.md +++ b/docs/source/Building.md @@ -31,24 +31,22 @@ run 'flattests' or `flatsampletext`, or it will fail to load its files.* ### Make all warnings into errors -By default all Flatbuffers `cmake` targets are build with `-Werror` flag. -With this flag (or `/WX` for MSVC) C++ compiler will treat all warnings as errors. -Additionally `-Wall -pedantic -Wextra` (or `/W4` form MSVC) flags are set. -These flags minimize the number of possible defects in code and keep code highly portable. -Using these flags is considered good practice but sometimes it can break dependent projects -if a compiler is upgraded or a toolset is changed. -Usually, newer compiler versions add new compile-time diagnostics that were unavailable before. -These new diagnostic warnings could stop the build process if `-Werror` flag is set. +By default all Flatbuffers `cmake` targets are **not** built with the `-Werror` +(or `/WX` for MSVC) flag that treats any warning as an error. This allows more +flexibility for users of Flatbuffers to use newer compilers and toolsets that +may add new warnings that would cause a build failure. -It is possible to cancel `warnings as errors` flag at `cmake` configuration stage using -`FLATBUFFERS_CXX_FLAGS` option. Compilation flags declared in `FLATBUFFERS_CXX_FLAGS` will be -appended to the project-level `CMAKE_CXX_FLAGS` variable. -Examples: +To enable a stricter build that does treat warnings as errors, set the +`FLATBUFFERS_STRICT_MODE` `cmake` compliation flag to `ON`. -- GCC and Clang: `cmake . -D FLATBUFFERS_CXX_FLAGS="-Wno-error"` -- MSVC: `cmake . -D FLATBUFFERS_CXX_FLAGS="/WX-"` -- MSVC: `cmake . -D FLATBUFFERS_CXX_FLAGS="/Wv "` +``` +cmake . -DFLATBUFFERS_STRICT_MODE=ON +``` +Our CI builds run with strict mode on, ensuring the code that is committed to +the project is as portable and warning free as possible. Thus developers +contributing to the project should enable strict mode locally before making a +PR. ## Building with VCPKG