From cd28357bbeb56427032fd1e56c8b3749bcfeb854 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Wed, 2 Jul 2025 17:44:43 +0000 Subject: Fix spurious vk::binding warnings when attribute is present (#7581) * Initial plan * Fix spurious vk::binding warnings when attribute is present - Modified _maybeDiagnoseMissingVulkanLayoutModifier to check for GLSLBindingAttribute before warning - Changed function to return bool indicating if warning was actually issued - Updated call sites to properly track warning state to reduce duplicates - Tested fix resolves the issue while preserving correct warnings when needed Co-authored-by: csyonghe <2652293+csyonghe@users.noreply.github.com> * Add test case for vk::binding spurious warning fix Co-authored-by: csyonghe <2652293+csyonghe@users.noreply.github.com> * Update test to use filecheck format as requested Co-authored-by: csyonghe <2652293+csyonghe@users.noreply.github.com> * Remove full file path from CHECK directives as requested Co-authored-by: csyonghe <2652293+csyonghe@users.noreply.github.com> * Fix code formatting with clang-format Co-authored-by: csyonghe <2652293+csyonghe@users.noreply.github.com> * Fix behavioral regression in vk-bindings test caused by warning flag logic Co-authored-by: csyonghe <2652293+csyonghe@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: csyonghe <2652293+csyonghe@users.noreply.github.com> Co-authored-by: Yong He --- source/slang/slang-parameter-binding.cpp | 33 +++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) (limited to 'source/slang') diff --git a/source/slang/slang-parameter-binding.cpp b/source/slang/slang-parameter-binding.cpp index 86d76fc6d..4ba9e4e03 100644 --- a/source/slang/slang-parameter-binding.cpp +++ b/source/slang/slang-parameter-binding.cpp @@ -1069,7 +1069,7 @@ static void addExplicitParameterBindings_HLSL( } } -static void _maybeDiagnoseMissingVulkanLayoutModifier( +static bool _maybeDiagnoseMissingVulkanLayoutModifier( ParameterBindingContext* context, DeclRef const& varDecl) { @@ -1077,7 +1077,7 @@ static void _maybeDiagnoseMissingVulkanLayoutModifier( if (varDecl.getDecl()->hasModifier() || varDecl.getDecl()->hasModifier()) { - return; + return false; } // If the user didn't specify a `binding` (and optional `set`) for Vulkan, @@ -1085,6 +1085,11 @@ static void _maybeDiagnoseMissingVulkanLayoutModifier( // oversight on their part. if (auto registerModifier = varDecl.getDecl()->findModifier()) { + // Don't warn if the declaration already has a vk::binding attribute + if (varDecl.getDecl()->findModifier()) + { + return false; + } auto varType = getType(context->getASTBuilder(), varDecl.as()); if (auto textureType = as(varType)) { @@ -1095,7 +1100,7 @@ static void _maybeDiagnoseMissingVulkanLayoutModifier( registerModifier, Diagnostics::registerModifierButNoVulkanLayout, varDecl.getName()); - return; + return true; } } @@ -1111,7 +1116,9 @@ static void _maybeDiagnoseMissingVulkanLayoutModifier( Diagnostics::registerModifierButNoVkBindingNorShift, varDecl.getName(), registerClassName); + return true; } + return false; } static void addExplicitParameterBindings_GLSL( @@ -1273,8 +1280,16 @@ static void addExplicitParameterBindings_GLSL( // If we are not told how to infer bindings with a compile option, we warn if (hlslToVulkanLayoutOptions == nullptr || !hlslToVulkanLayoutOptions->canInferBindings()) { - warnedMissingVulkanLayoutModifier = true; - _maybeDiagnoseMissingVulkanLayoutModifier(context, varDecl.as()); + warnedMissingVulkanLayoutModifier = + _maybeDiagnoseMissingVulkanLayoutModifier(context, varDecl.as()); + // For behavioral compatibility, if we didn't warn due to a push constant or shader record, + // we still need to set the flag to true to maintain the same binding logic + if (!warnedMissingVulkanLayoutModifier && + (varDecl.getDecl()->hasModifier() || + varDecl.getDecl()->hasModifier())) + { + warnedMissingVulkanLayoutModifier = true; + } } // We need an HLSL register semantic to to infer from @@ -1301,8 +1316,8 @@ static void addExplicitParameterBindings_GLSL( { if (!warnedMissingVulkanLayoutModifier) { - _maybeDiagnoseMissingVulkanLayoutModifier(context, varDecl.as()); - warnedMissingVulkanLayoutModifier = true; + warnedMissingVulkanLayoutModifier = + _maybeDiagnoseMissingVulkanLayoutModifier(context, varDecl.as()); } return; } @@ -1323,8 +1338,8 @@ static void addExplicitParameterBindings_GLSL( { if (!warnedMissingVulkanLayoutModifier) { - _maybeDiagnoseMissingVulkanLayoutModifier(context, varDecl.as()); - warnedMissingVulkanLayoutModifier = true; + warnedMissingVulkanLayoutModifier = + _maybeDiagnoseMissingVulkanLayoutModifier(context, varDecl.as()); } } -- cgit v1.2.3