Add -Wshadow to align with windows compiler warnings (#2735)

ld-kerley · web-flow · commit c342c0bf7f1a · 2025-12-26T15:26:15.000-08:00
Continuing the work proposed in #2385. I noticed this as more compiler warnings were raised in other work for windows CI. Add `-Wshadow` to non windows compile path. This aligns us a little closer with the strict Windows path warnings, making it easier to catch build errors locally before hitting CI. Fixed some shadowing issues in Metal code-path that were missed previously because of mis-matched compiler warnings. Need to add `-Wno-shadow` for dependency builds (IMGui/NanoGui). Also adding `-Wno-nontrivial-memcall` because of recent clang warnings building IMGui.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -312,7 +312,7 @@ if(MSVC)
         add_compile_options(/WX)
     endif()
 else()
-    add_compile_options(-Wall -Wunused-parameter -Wno-missing-braces)
+    add_compile_options(-Wall -Wunused-parameter -Wshadow -Wno-missing-braces)
     if(MATERIALX_WARNINGS_AS_ERRORS)
         add_compile_options(-Werror)
     endif()
diff --git a/source/MaterialXGenOsl/LibsToOso.cpp b/source/MaterialXGenOsl/LibsToOso.cpp
@@ -24,7 +24,7 @@
 
 namespace mx = MaterialX;
 
-const std::string options =
+const std::string argOptions =
     " Options: \n"
     "    --outputOsoPath [DIRPATH]       TODO\n"
     "    --libraryRelativeOsoPath [DIRPATH]       TODO\n"
@@ -263,7 +263,7 @@ int main(int argc, char* const argv[])
             std::cout << " - Compiled with liboslcomp";
 #endif
             std::cout << std::endl;
-            std::cout << options << std::endl;
+            std::cout << argOptions << std::endl;
 
             return 0;
         }
diff --git a/source/MaterialXGraphEditor/CMakeLists.txt b/source/MaterialXGraphEditor/CMakeLists.txt
@@ -9,9 +9,9 @@ endif()
 if(MSVC)
     add_compile_options(-wd4100 -wd4152 -wd4201 -wd4244 -wd4456)
 elseif(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
-    add_compile_options(-Wno-unused -Wno-deprecated -Wno-comment -Wno-unused-variable -Wno-unused-parameter)
+    add_compile_options(-Wno-unused -Wno-deprecated -Wno-comment -Wno-unused-variable -Wno-unused-parameter -Wno-shadow)
 elseif(CMAKE_CXX_COMPILER_ID MATCHES "GNU")
-    add_compile_options(-Wno-format-truncation -Wno-use-after-free -Wno-comment -Wno-unused-but-set-variable -Wno-unused-parameter)
+    add_compile_options(-Wno-format-truncation -Wno-use-after-free -Wno-comment -Wno-unused-but-set-variable -Wno-unused-parameter -Wno-shadow)
 endif()
 
 file(GLOB materialx_source "${CMAKE_CURRENT_SOURCE_DIR}/*.cpp")
diff --git a/source/MaterialXRenderGlsl/GlslMaterial.cpp b/source/MaterialXRenderGlsl/GlslMaterial.cpp
@@ -324,30 +324,30 @@ VariableBlock* GlslMaterial::getPublicUniforms() const
 
 ShaderPort* GlslMaterial::findUniform(const std::string& path) const
 {
-    ShaderPort* port = nullptr;
+    ShaderPort* shaderPort = nullptr;
     VariableBlock* publicUniforms = getPublicUniforms();
     if (publicUniforms)
     {
         // Scan block based on path match predicate
-        port = publicUniforms->find([path](ShaderPort* port)
+        shaderPort = publicUniforms->find([path](ShaderPort* port)
         {
             return (port && stringEndsWith(port->getPath(), path));
         });
-        if (!port)
+        if (!shaderPort)
         {
-            port = publicUniforms->find([path](ShaderPort* port)
+            shaderPort = publicUniforms->find([path](ShaderPort* port)
             {
                 return (port && stringEndsWith(path, port->getName()));
             });
         }
 
         // Check if the uniform exists in the shader program
-        if (port && !_glProgram->getUniformsList().count(port->getVariable()))
+        if (shaderPort && !_glProgram->getUniformsList().count(shaderPort->getVariable()))
         {
-            port = nullptr;
+            shaderPort = nullptr;
         }
     }
-    return port;
+    return shaderPort;
 }
 
 void GlslMaterial::modifyUniform(const std::string& path, ConstValuePtr value, std::string valueString)
diff --git a/source/MaterialXRenderGlsl/GlslProgram.cpp b/source/MaterialXRenderGlsl/GlslProgram.cpp
@@ -942,9 +942,9 @@ const GlslProgram::InputMap& GlslProgram::updateUniformsList()
                 continue;
             }
 
-            for (size_t i = 0; i < uniforms.size(); ++i)
+            for (size_t uniformIndex = 0; uniformIndex < uniforms.size(); ++uniformIndex)
             {
-                const ShaderPort* v = uniforms[i];
+                const ShaderPort* v = uniforms[uniformIndex];
 
                 const auto& variablePath = v->getPath();
                 const auto& variableUnit = v->getUnit();
@@ -957,15 +957,15 @@ const GlslProgram::InputMap& GlslProgram::updateUniformsList()
                 {
                     auto populateUniformInput_impl =
                         [this, variablePath, variableUnit, variableColorspace, variableSemantic, &errors, uniforms, &uniformTypeMismatchFound]
-                        (TypeDesc typedesc, const string& variableName, ConstValuePtr variableValue, auto& populateUniformInput_ref) -> void
+                        (TypeDesc typedesc_impl, const string& variableName_impl, ConstValuePtr variableValue_impl, auto& populateUniformInput_ref) -> void
                     {
-                        if (!typedesc.isStruct())
+                        if (!typedesc_impl.isStruct())
                         {
                             // Handle non-struct types
-                            int glType = mapTypeToOpenGLType(typedesc);
+                            int glType = mapTypeToOpenGLType(typedesc_impl);
 
                             // There is no way to match with an unnamed variable
-                            if (variableName.empty())
+                            if (variableName_impl.empty())
                             {
                                 return;
                             }
@@ -976,26 +976,26 @@ const GlslProgram::InputMap& GlslProgram::updateUniformsList()
                                 return;
                             }
 
-                            auto inputIt = _uniformList.find(variableName);
+                            auto inputIt = _uniformList.find(variableName_impl);
                             if (inputIt != _uniformList.end())
                             {
                                 Input* input = inputIt->second.get();
                                 input->path = variablePath;
                                 input->unit = variableUnit;
                                 input->colorspace = variableColorspace;
-                                input->value = variableValue;
+                                input->value = variableValue_impl;
                                 if (input->gltype == glType)
                                 {
-                                    input->typeString = typedesc.getName();
+                                    input->typeString = typedesc_impl.getName();
                                 }
                                 else
                                 {
                                     errors.push_back(
                                         "Pixel shader uniform block type mismatch [" + uniforms.getName() + "]. "
-                                        + "Name: \"" + variableName
-                                        + "\". Type: \"" + typedesc.getName()
+                                        + "Name: \"" + variableName_impl
+                                        + "\". Type: \"" + typedesc_impl.getName()
                                         + "\". Semantic: \"" + variableSemantic
-                                        + "\". Value: \"" + (variableValue ? variableValue->getValueString() : "<none>")
+                                        + "\". Value: \"" + (variableValue_impl ? variableValue_impl->getValueString() : "<none>")
                                         + "\". Unit: \"" + (!variableUnit.empty() ? variableUnit : "<none>")
                                         + "\". Colorspace: \"" + (!variableColorspace.empty() ? variableColorspace : "<none>")
                                         + "\". GLType: " + std::to_string(glType));
@@ -1005,16 +1005,16 @@ const GlslProgram::InputMap& GlslProgram::updateUniformsList()
                         }
                         else
                         {
-                            auto variableStructMembers = typedesc.getStructMembers();
+                            auto variableStructMembers = typedesc_impl.getStructMembers();
                             if (variableStructMembers)
                             {
                                 // If we're a struct - we need to loop over each member
-                                auto aggregateValue = std::static_pointer_cast<const AggregateValue>(variableValue);
+                                auto aggregateValue = std::static_pointer_cast<const AggregateValue>(variableValue_impl);
 
                                 for (size_t i = 0, n = variableStructMembers->size(); i < n; ++i)
                                 {
                                     const auto& structMember = (*variableStructMembers)[i];
-                                    auto memberVariableName = variableName + "." + structMember.getName();
+                                    auto memberVariableName = variableName_impl + "." + structMember.getName();
                                     auto memberVariableValue = aggregateValue->getMemberValue(i);
 
                                     populateUniformInput_ref(structMember.getType(), memberVariableName, memberVariableValue, populateUniformInput_ref);
diff --git a/source/MaterialXRenderMsl/MslPipelineStateObject.mm b/source/MaterialXRenderMsl/MslPipelineStateObject.mm
@@ -687,8 +687,8 @@ int GetStrideOfMetalType(MTLDataType type)
 
     // Set the number of active light sources
     size_t lightCount = lightHandler->getLightSources().size();
-    auto input = uniformList.find(HW::NUM_ACTIVE_LIGHT_SOURCES);
-    if (input == uniformList.end())
+    auto numActiveLightSourcesInput = uniformList.find(HW::NUM_ACTIVE_LIGHT_SOURCES);
+    if (numActiveLightSourcesInput == uniformList.end())
     {
         // No lighting information so nothing further to do
         lightCount = 0;
@@ -746,8 +746,8 @@ int GetStrideOfMetalType(MTLDataType type)
     // Bind direct lighting properties.
     if (hasUniform(HW::NUM_ACTIVE_LIGHT_SOURCES))
     {
-        int lightCount = lightHandler->getDirectLighting() ? (int) lightHandler->getLightSources().size() : 0;
-        bindUniform(HW::NUM_ACTIVE_LIGHT_SOURCES, Value::createValue(lightCount));
+        lightCount = lightHandler->getDirectLighting() ? lightCount : 0;
+        bindUniform(HW::NUM_ACTIVE_LIGHT_SOURCES, Value::createValue(static_cast<int>(lightCount)));
         LightIdMap idMap = lightHandler->computeLightIdMap(lightHandler->getLightSources());
         size_t index = 0;
         for (NodePtr light : lightHandler->getLightSources())
diff --git a/source/MaterialXTest/MaterialXRender/RenderUtil.cpp b/source/MaterialXTest/MaterialXRender/RenderUtil.cpp
@@ -450,9 +450,9 @@ void ShaderRenderTester::addAdditionalTestStreams(mx::MeshPtr mesh)
         mesh->addStream(geomColor4Stream);
     }
 
-    auto sineData = [](float uv, float freq){
+    auto sineData = [](float texCoord, float freq){
         const float PI = std::acos(-1.0f);
-        float angle = uv * 2 * PI * freq;
+        float angle = texCoord * 2 * PI * freq;
         return std::sin(angle) / 2.0f + 1.0f;
     };
     if (!uv.empty())
diff --git a/source/MaterialXView/CMakeLists.txt b/source/MaterialXView/CMakeLists.txt
@@ -71,9 +71,9 @@ else()
     if(MSVC)
         add_compile_options(-wd4389)
     elseif(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
-        add_compile_options(-Wno-deprecated -Wno-unused-parameter)
+        add_compile_options(-Wno-deprecated -Wno-unused-parameter -Wno-shadow)
     elseif(CMAKE_CXX_COMPILER_ID MATCHES "GNU")
-        add_compile_options(-Wno-format-truncation -Wno-stringop-overflow -Wno-use-after-free -Wno-unused-parameter)
+        add_compile_options(-Wno-format-truncation -Wno-stringop-overflow -Wno-use-after-free -Wno-unused-parameter -Wno-shadow)
     endif()
 
     # Disable NanoGUI compiler modifications for Clang

Original file line number	Diff line number	Diff line change
`@@ -324,30 +324,30 @@ VariableBlock* GlslMaterial::getPublicUniforms() const`
`324`	`324`
`325`	`325`	`ShaderPort* GlslMaterial::findUniform(const std::string& path) const`
`326`	`326`	`{`
`327`		`- ShaderPort* port = nullptr;`
	`327`	`+ ShaderPort* shaderPort = nullptr;`
`328`	`328`	`VariableBlock* publicUniforms = getPublicUniforms();`
`329`	`329`	`if (publicUniforms)`
`330`	`330`	`{`
`331`	`331`	`// Scan block based on path match predicate`
`332`		`- port = publicUniforms->find([path](ShaderPort* port)`
	`332`	`+ shaderPort = publicUniforms->find([path](ShaderPort* port)`
`333`	`333`	`{`
`334`	`334`	`return (port && stringEndsWith(port->getPath(), path));`
`335`	`335`	`});`
`336`		`- if (!port)`
	`336`	`+ if (!shaderPort)`
`337`	`337`	`{`
`338`		`- port = publicUniforms->find([path](ShaderPort* port)`
	`338`	`+ shaderPort = publicUniforms->find([path](ShaderPort* port)`
`339`	`339`	`{`
`340`	`340`	`return (port && stringEndsWith(path, port->getName()));`
`341`	`341`	`});`
`342`	`342`	`}`
`343`	`343`
`344`	`344`	`// Check if the uniform exists in the shader program`
`345`		`- if (port && !_glProgram->getUniformsList().count(port->getVariable()))`
	`345`	`+ if (shaderPort && !_glProgram->getUniformsList().count(shaderPort->getVariable()))`
`346`	`346`	`{`
`347`		`- port = nullptr;`
	`347`	`+ shaderPort = nullptr;`
`348`	`348`	`}`
`349`	`349`	`}`
`350`		`- return port;`
	`350`	`+ return shaderPort;`
`351`	`351`	`}`
`352`	`352`
`353`	`353`	`void GlslMaterial::modifyUniform(const std::string& path, ConstValuePtr value, std::string valueString)`
Original file line number	Diff line number	Diff line change
`@@ -942,9 +942,9 @@ const GlslProgram::InputMap& GlslProgram::updateUniformsList()`
`942`	`942`	`continue;`
`943`	`943`	`}`
`944`	`944`
`945`		`- for (size_t i = 0; i < uniforms.size(); ++i)`
	`945`	`+ for (size_t uniformIndex = 0; uniformIndex < uniforms.size(); ++uniformIndex)`
`946`	`946`	`{`
`947`		`- const ShaderPort* v = uniforms[i];`
	`947`	`+ const ShaderPort* v = uniforms[uniformIndex];`
`948`	`948`
`949`	`949`	`const auto& variablePath = v->getPath();`
`950`	`950`	`const auto& variableUnit = v->getUnit();`
`@@ -957,15 +957,15 @@ const GlslProgram::InputMap& GlslProgram::updateUniformsList()`
`957`	`957`	`{`
`958`	`958`	`auto populateUniformInput_impl =`
`959`	`959`	`[this, variablePath, variableUnit, variableColorspace, variableSemantic, &errors, uniforms, &uniformTypeMismatchFound]`
`960`		`- (TypeDesc typedesc, const string& variableName, ConstValuePtr variableValue, auto& populateUniformInput_ref) -> void`
	`960`	`+ (TypeDesc typedesc_impl, const string& variableName_impl, ConstValuePtr variableValue_impl, auto& populateUniformInput_ref) -> void`
`961`	`961`	`{`
`962`		`- if (!typedesc.isStruct())`
	`962`	`+ if (!typedesc_impl.isStruct())`
`963`	`963`	`{`
`964`	`964`	`// Handle non-struct types`
`965`		`- int glType = mapTypeToOpenGLType(typedesc);`
	`965`	`+ int glType = mapTypeToOpenGLType(typedesc_impl);`
`966`	`966`
`967`	`967`	`// There is no way to match with an unnamed variable`
`968`		`- if (variableName.empty())`
	`968`	`+ if (variableName_impl.empty())`
`969`	`969`	`{`
`970`	`970`	`return;`
`971`	`971`	`}`
`@@ -976,26 +976,26 @@ const GlslProgram::InputMap& GlslProgram::updateUniformsList()`
`976`	`976`	`return;`
`977`	`977`	`}`
`978`	`978`
`979`		`- auto inputIt = _uniformList.find(variableName);`
	`979`	`+ auto inputIt = _uniformList.find(variableName_impl);`
`980`	`980`	`if (inputIt != _uniformList.end())`
`981`	`981`	`{`
`982`	`982`	`Input* input = inputIt->second.get();`
`983`	`983`	`input->path = variablePath;`
`984`	`984`	`input->unit = variableUnit;`
`985`	`985`	`input->colorspace = variableColorspace;`
`986`		`- input->value = variableValue;`
	`986`	`+ input->value = variableValue_impl;`
`987`	`987`	`if (input->gltype == glType)`
`988`	`988`	`{`
`989`		`- input->typeString = typedesc.getName();`
	`989`	`+ input->typeString = typedesc_impl.getName();`
`990`	`990`	`}`
`991`	`991`	`else`
`992`	`992`	`{`
`993`	`993`	`errors.push_back(`
`994`	`994`	`"Pixel shader uniform block type mismatch [" + uniforms.getName() + "]. "`
`995`		`- + "Name: \"" + variableName`
`996`		`- + "\". Type: \"" + typedesc.getName()`
	`995`	`+ + "Name: \"" + variableName_impl`
	`996`	`+ + "\". Type: \"" + typedesc_impl.getName()`
`997`	`997`	`+ "\". Semantic: \"" + variableSemantic`
`998`		`- + "\". Value: \"" + (variableValue ? variableValue->getValueString() : "<none>")`
	`998`	`+ + "\". Value: \"" + (variableValue_impl ? variableValue_impl->getValueString() : "<none>")`
`999`	`999`	`+ "\". Unit: \"" + (!variableUnit.empty() ? variableUnit : "<none>")`
`1000`	`1000`	`+ "\". Colorspace: \"" + (!variableColorspace.empty() ? variableColorspace : "<none>")`
`1001`	`1001`	`+ "\". GLType: " + std::to_string(glType));`
`@@ -1005,16 +1005,16 @@ const GlslProgram::InputMap& GlslProgram::updateUniformsList()`
`1005`	`1005`	`}`
`1006`	`1006`	`else`
`1007`	`1007`	`{`
`1008`		`- auto variableStructMembers = typedesc.getStructMembers();`
	`1008`	`+ auto variableStructMembers = typedesc_impl.getStructMembers();`
`1009`	`1009`	`if (variableStructMembers)`
`1010`	`1010`	`{`
`1011`	`1011`	`// If we're a struct - we need to loop over each member`
`1012`		`- auto aggregateValue = std::static_pointer_cast<const AggregateValue>(variableValue);`
	`1012`	`+ auto aggregateValue = std::static_pointer_cast<const AggregateValue>(variableValue_impl);`
`1013`	`1013`
`1014`	`1014`	`for (size_t i = 0, n = variableStructMembers->size(); i < n; ++i)`
`1015`	`1015`	`{`
`1016`	`1016`	`const auto& structMember = (*variableStructMembers)[i];`
`1017`		`- auto memberVariableName = variableName + "." + structMember.getName();`
	`1017`	`+ auto memberVariableName = variableName_impl + "." + structMember.getName();`
`1018`	`1018`	`auto memberVariableValue = aggregateValue->getMemberValue(i);`
`1019`	`1019`
`1020`	`1020`	`populateUniformInput_ref(structMember.getType(), memberVariableName, memberVariableValue, populateUniformInput_ref);`