Add BUILD_WERROR option

Adds a cmake BUILD_WERROR option to enable warnings as errors. This option is off by default. This change also cleans up a few more compiler warnings.
KhronosGroup · Nov 29, 2023 · 3f615ad · 3f615ad
1 parent 719b6b7
commit 3f615ad
Show file tree

Hide file tree

Showing 6 changed files with 22 additions and 12 deletions.
diff --git a/.github/workflows/continuous_integration.yml b/.github/workflows/continuous_integration.yml
@@ -28,7 +28,7 @@ jobs:
           key: ubuntu-22-${{ matrix.cmake_build_type }}-${{ matrix.compiler.cc }}-${{matrix.compiler.cxx}}
       - run: ./update_glslang_sources.py
       - name: Configure
-        run: cmake -S . -B build -D CMAKE_BUILD_TYPE=${{ matrix.cmake_build_type }}
+        run: cmake -S . -B build -D CMAKE_BUILD_TYPE=${{ matrix.cmake_build_type }} -DBUILD_WERROR=ON
         env:
           CC: ${{matrix.compiler.cc}}
           CXX: ${{matrix.compiler.cxx}}
@@ -126,7 +126,7 @@ jobs:
       - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
       - uses: lukka/get-cmake@8be6cca406b575906541e8e3b885d46f416bba39 # v3.27.7
       - run: ./update_glslang_sources.py
-      - run: cmake -S . -B build -D CMAKE_BUILD_TYPE=${{matrix.cmake_build_type}} -G Ninja
+      - run: cmake -S . -B build -D CMAKE_BUILD_TYPE=${{matrix.cmake_build_type}} -G Ninja -DBUILD_WERROR=ON
         env:
           CC: ${{matrix.compiler.cc}}
           CXX: ${{matrix.compiler.cxx}}
@@ -155,7 +155,7 @@ jobs:
       - run: python update_glslang_sources.py
       - name: Build
         run: |
-          cmake -S. -Bbuild -G "Visual Studio 16 2019" -A x64 -DCMAKE_INSTALL_PREFIX="$PWD/build/install"
+          cmake -S. -Bbuild -G "Visual Studio 16 2019" -A x64 -DCMAKE_INSTALL_PREFIX="$PWD/build/install" -DBUILD_WERROR=ON
           cmake --build build --config ${{matrix.cmake_build_type}} --target install
       - name: Test
         run: ctest -C ${{matrix.cmake_build_type}} --output-on-failure --test-dir build

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -44,6 +44,7 @@ include(CMakeDependentOption)
 
 option(BUILD_SHARED_LIBS "Build Shared Libraries")
 option(BUILD_EXTERNAL "Build external dependencies in /External" ON)
+option(BUILD_WERROR "Enable warnings as errors (default is OFF)" OFF)
 
 set(LIB_TYPE STATIC)
 
@@ -177,6 +178,15 @@ elseif(MSVC)
     endif()
 endif()
 
+# NOTE we could potentially replace this logic with COMPILE_WARNING_AS_ERROR if cmake minimum is bumped to >= 3.24
+if (BUILD_WERROR)
+    if (NOT MSVC)
+        add_compile_options(-Werror)
+    else()
+        add_compile_options(/WX)
+    endif()
+endif()
+
 if(ENABLE_GLSLANG_JS)
     if(MSVC)
         add_compile_options(/Os /GR-)

diff --git a/glslang/MachineIndependent/ParseHelper.cpp b/glslang/MachineIndependent/ParseHelper.cpp
@@ -7458,7 +7458,7 @@ static void ForEachOpaque(const TType& type, const TString& path, Function callb
                  ++flatIndex)
             {
                 TString subscriptPath = path;
-                for (int dimIndex = 0; dimIndex < indices.size(); ++dimIndex)
+                for (size_t dimIndex = 0; dimIndex < indices.size(); ++dimIndex)
                 {
                     int index = indices[dimIndex];
                     subscriptPath.append("[");
@@ -7468,7 +7468,7 @@ static void ForEachOpaque(const TType& type, const TString& path, Function callb
 
                 recursion(type, subscriptPath, true, recursion);
 
-                for (int dimIndex = 0; dimIndex < indices.size(); ++dimIndex)
+                for (size_t dimIndex = 0; dimIndex < indices.size(); ++dimIndex)
                 {
                     ++indices[dimIndex];
                     if (indices[dimIndex] < type.getArraySizes()->getDimSize(dimIndex))
@@ -7537,7 +7537,7 @@ void TParseContext::vkRelaxedRemapUniformMembers(const TSourceLoc& loc, const TP
                   });
 }
 
-void TParseContext::vkRelaxedRemapFunctionParameter(const TSourceLoc& loc, TFunction* function, TParameter& param, std::vector<int>* newParams)
+void TParseContext::vkRelaxedRemapFunctionParameter(TFunction* function, TParameter& param, std::vector<int>* newParams)
 {
     function->addParameter(param);
 
@@ -7615,7 +7615,7 @@ TIntermNode* TParseContext::vkRelaxedRemapFunctionArgument(const TSourceLoc& loc
     param.type->shallowCopy(intermTyped->getType());
 
     std::vector<int> newParams = {};
-    vkRelaxedRemapFunctionParameter(loc, function, param, &newParams);
+    vkRelaxedRemapFunctionParameter(function, param, &newParams);
 
     if (intermTyped->getType().isOpaque())
     {

diff --git a/glslang/MachineIndependent/ParseHelper.h b/glslang/MachineIndependent/ParseHelper.h
@@ -369,7 +369,7 @@ class TParseContext : public TParseContextBase {
     // returns true if the variable was remapped to something else
     bool vkRelaxedRemapUniformVariable(const TSourceLoc&, TString&, const TPublicType&, TArraySizes*, TIntermTyped*, TType&);
     void vkRelaxedRemapUniformMembers(const TSourceLoc&, const TPublicType&, const TType&, const TString&);
-    void vkRelaxedRemapFunctionParameter(const TSourceLoc&, TFunction*, TParameter&, std::vector<int>* newParams = nullptr);
+    void vkRelaxedRemapFunctionParameter(TFunction*, TParameter&, std::vector<int>* newParams = nullptr);
     TIntermNode* vkRelaxedRemapFunctionArgument(const TSourceLoc&, TFunction*, TIntermTyped*);
     TIntermTyped* vkRelaxedRemapDotDereference(const TSourceLoc&, TIntermTyped&, const TType&, const TString&);
 

diff --git a/glslang/MachineIndependent/glslang.y b/glslang/MachineIndependent/glslang.y
@@ -1007,7 +1007,7 @@ function_header_with_parameters
             if (!(parseContext.spvVersion.vulkan > 0 && parseContext.spvVersion.vulkanRelaxed))
                 $1->addParameter($2.param);
             else
-                parseContext.vkRelaxedRemapFunctionParameter($2.loc, $1, $2.param);
+                parseContext.vkRelaxedRemapFunctionParameter($1, $2.param);
         }
         else
             delete $2.param.type;
@@ -1029,7 +1029,7 @@ function_header_with_parameters
             if (!(parseContext.spvVersion.vulkan > 0 && parseContext.spvVersion.vulkanRelaxed))
                 $1->addParameter($3.param);
             else
-                parseContext.vkRelaxedRemapFunctionParameter($3.loc, $1, $3.param);
+                parseContext.vkRelaxedRemapFunctionParameter($1, $3.param);
         }
     }
     ;

diff --git a/glslang/MachineIndependent/glslang_tab.cpp b/glslang/MachineIndependent/glslang_tab.cpp
@@ -6319,7 +6319,7 @@ YYSTYPE yylval YY_INITIAL_VALUE (= yyval_default);
             if (!(parseContext.spvVersion.vulkan > 0 && parseContext.spvVersion.vulkanRelaxed))
                 (yyvsp[-1].interm.function)->addParameter((yyvsp[0].interm).param);
             else
-                parseContext.vkRelaxedRemapFunctionParameter((yyvsp[0].interm).loc, (yyvsp[-1].interm.function), (yyvsp[0].interm).param);
+                parseContext.vkRelaxedRemapFunctionParameter((yyvsp[-1].interm.function), (yyvsp[0].interm).param);
         }
         else
             delete (yyvsp[0].interm).param.type;
@@ -6346,7 +6346,7 @@ YYSTYPE yylval YY_INITIAL_VALUE (= yyval_default);
             if (!(parseContext.spvVersion.vulkan > 0 && parseContext.spvVersion.vulkanRelaxed))
                 (yyvsp[-2].interm.function)->addParameter((yyvsp[0].interm).param);
             else
-                parseContext.vkRelaxedRemapFunctionParameter((yyvsp[0].interm).loc, (yyvsp[-2].interm.function), (yyvsp[0].interm).param);
+                parseContext.vkRelaxedRemapFunctionParameter((yyvsp[-2].interm.function), (yyvsp[0].interm).param);
         }
     }
 #line 6353 "MachineIndependent/glslang_tab.cpp"