From 9ed7b5264676547855e1378d2b9c9d51c8ba7162 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Thu, 7 Aug 2025 01:01:02 -0700 Subject: Add warning for comma operators used outside for-loops and expand expressions in legacy mode (#7984) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR implements a warning system to help users identify potentially unintended comma operator usage in expressions. The comma operator can be confusing when used in contexts like variable initialization where users might have intended to use braces for initialization instead. ## Problem The following code compiles without error but is likely not written as intended: ```slang float4 vColor = (0.f, 0.f, 0.f, 1.f); // Uses comma operators, evaluates to 1.f ``` The intended code should use braces: ```slang float4 vColor = {0.f, 0.f, 0.f, 1.f}; // Proper initialization ``` ## Solution Added a new warning diagnostic (`commaOperatorUsedInExpression`, ID: 41024) that warns when comma operators are used in expressions, with exemptions for contexts where they are commonly intended: - **For-loop side effects**: `for (int i = 0; i < 10; i++, x++)` - no warning - **Expand expressions**: `expand(f(), g(each param))` - no warning - **Slang 2026+ mode**: `let m = (1,2,3)` creates tuples - no warning - **All other expressions**: `float4 v = (a, b, c, d)` and `return a, b` - warns for each comma ## Implementation Details - Added context tracking in `SemanticsContext` with `m_inForLoopSideEffect` flag - Modified `visitForStmt` to use special context when checking side effect expressions - Added comma operator detection in `visitInvokeExpr` for `InfixExpr` nodes - Added language version check using `isSlang2026OrLater()` to disable warnings in Slang 2026+ mode where parentheses create tuples - Performance optimization: language version check is hoisted to avoid unnecessary casting - Warning can be suppressed using `-Wno-41024` command line flag ## Test Coverage Added comprehensive test cases using filecheck format that verify: - Warnings are generated for comma operators in variable initialization (legacy mode only) - Warnings are generated for comma operators in return statements (legacy mode only) - Warnings are generated for comma operators in general expressions (legacy mode only) - No warnings for comma operators in for-loop side effects - No warnings in Slang 2026+ mode where parentheses create tuples - Warning suppression works correctly Example output (legacy mode): ``` warning 41024: comma operator used in expression (may be unintended) float4 vColor = (0.f, 0.f, 0.f, 1.f); ^ warning 41024: comma operator used in expression (may be unintended) return a *= 2, a + 1; ^ ``` Fixes #6732. --- 💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click [here](https://survey.alchemer.com/s3/8343779/Copilot-Coding-agent) to start the survey. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: aidanfnv <198290069+aidanfnv@users.noreply.github.com> Co-authored-by: slangbot Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com> Co-authored-by: aidanfnv Co-authored-by: csyonghe <2652293+csyonghe@users.noreply.github.com> --- tests/compute/comma-operator.slang | 6 +++--- tests/compute/spirv-array-texel-pointer-atomic.slang | 4 ++-- tests/compute/spirv-multisampled-array-texel-pointer-atomic.slang | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) (limited to 'tests/compute') diff --git a/tests/compute/comma-operator.slang b/tests/compute/comma-operator.slang index c6cecf1c7..917484d00 100644 --- a/tests/compute/comma-operator.slang +++ b/tests/compute/comma-operator.slang @@ -1,7 +1,7 @@ // comma-operator.slang -//TEST(compute):COMPARE_COMPUTE:-cpu -shaderobj -//TEST(compute):COMPARE_COMPUTE: -shaderobj +//TEST(compute):COMPARE_COMPUTE:-cpu -shaderobj -xslang -Wno-41024 +//TEST(compute):COMPARE_COMPUTE: -shaderobj -xslang -Wno-41024 // Test that the "comma operator" behaves as expected @@ -9,7 +9,7 @@ // confirm that the generated SPIR-V (and by extension DXIL/DXBC) // doesn't include a subroutine defintion/call for `operator,` -//TEST:CROSS_COMPILE:-target spirv-assembly -entry computeMain -stage compute +//TEST:CROSS_COMPILE:-target spirv-assembly -entry computeMain -stage compute -Wno-41024 int test(int inVal) { diff --git a/tests/compute/spirv-array-texel-pointer-atomic.slang b/tests/compute/spirv-array-texel-pointer-atomic.slang index a0013e284..41ee27dfb 100644 --- a/tests/compute/spirv-array-texel-pointer-atomic.slang +++ b/tests/compute/spirv-array-texel-pointer-atomic.slang @@ -1,5 +1,5 @@ -//TEST:SIMPLE(filecheck=CHECK): -target spirv -entry computeMain -stage compute -//TEST(compute):COMPARE_COMPUTE(filecheck-buffer=BUF): -slang -output-using-type -shaderobj -vk +//TEST:SIMPLE(filecheck=CHECK): -target spirv -entry computeMain -stage compute -Wno-41024 +//TEST(compute):COMPARE_COMPUTE(filecheck-buffer=BUF): -slang -output-using-type -shaderobj -vk -xslang -Wno-41024 //TEST_INPUT: RWTexture2D(format=R32Uint, size=4, content = zero, arrayLength=2, mipMaps = 1):name rwTexture2DArray [[vk::binding(2)]] diff --git a/tests/compute/spirv-multisampled-array-texel-pointer-atomic.slang b/tests/compute/spirv-multisampled-array-texel-pointer-atomic.slang index 48b420b38..d4c90e051 100644 --- a/tests/compute/spirv-multisampled-array-texel-pointer-atomic.slang +++ b/tests/compute/spirv-multisampled-array-texel-pointer-atomic.slang @@ -1,5 +1,5 @@ -//TEST:SIMPLE(filecheck=CHECK): -target spirv -entry computeMain -stage compute -//TEST(compute):COMPARE_COMPUTE(filecheck-buffer=BUF): -slang -output-using-type -shaderobj -vk +//TEST:SIMPLE(filecheck=CHECK): -target spirv -entry computeMain -stage compute -Wno-41024 +//TEST(compute):COMPARE_COMPUTE(filecheck-buffer=BUF): -slang -output-using-type -shaderobj -vk -xslang -Wno-41024 //DISABLE_TEST(cuda):COMPARE_COMPUTE_EX:-cuda -compute -shaderobj -output-using-type //TEST_INPUT: RWTexture2D(format=R32Uint, size=4, content = zero, arrayLength=2, sampleCount=two, mipMaps = 1):name rwTexture2DMSArray -- cgit v1.2.3