| Age | Commit message (Collapse) | Author |
|---|
|
Closes #8112. ~~The issue asks for a "C layout", but in this PR I use
the term "CPU layout" because this naming was pre-existing in the
codebase as `kCPULayoutRulesImpl_`. The primary purpose of this layout
is to match CPU-side struct definitions with the shader side. I'm open
to better naming suggestions, though.~~
Edit: switched back to using `CDataLayout` & `-fvk-use-c-layout`, as the
CPU target depends on the object layout rules of existing CPU layout
rules, but they're incompatible with actual shaders. So a new
`kCLayoutRulesImpl_` was needed anyway.
---------
Co-authored-by: Ellie Hermaszewska <ellieh@nvidia.com>
|
|
-Adds semantic SV_VulkanSamplePosition that emits corresponding
gl_SamplePosition and SpvBuiltinSamplePosition
-Adds gl_SamplePosition property to glsl.meta.slang
-Adds SPIRV and GLSL tests for the semantic and property
-Plan is to later implement SV_SamplePosition that follows HLSL range of
-0.5 to +0.5,
and emits GetRenderTargetSamplePosition(SV_SampleIndex) which needs more
complicated IR manipulation for HLSL and Metal
Fixes #7906
---------
Co-authored-by: ArielG-NV <159081215+ArielG-NV@users.noreply.github.com>
|
|
Fixes #8185. The previous implementation is incorrect and basically only
works in the `x = 0` case. `delta` was the smallest possible positive
value representable as a float, but that's below the rounding error of
addition with almost all reasonably sized floats.
This fixed implementation is based on bit twiddling instead. I've
checked the float case against the C++ `nextafterf` with both a -inf ->
inf and inf -> -inf sweep, in addition to the test included in this PR.
|
|
Allows opt-in for sourcing the following dependencies from the system,
instead of using the vendored ones:
- miniz
- lz4
- vulkan-headers
- spirv-tools
- glslang
(some of these already had options that weren't working, as either it
expected them to be static libraries or it was expecting to be embedded
in another CMakeList that should provide the package, instead of finding
the package itself)
This is based on a patch we currently maintain inside nixpkgs, but as it
frequently conflicts with new slang releases, it would be nice to see
get
this upstream.
---------
Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
Co-authored-by: Sam Estep <sam@samestep.com>
|
|
## Summary
- Add Metal platform support for `WaveGetActiveMask()` and
`WaveActiveCountBits()` wave intrinsics
- Update capability requirements to include Metal platform for subgroup
ballot operations
- Implement Metal-specific intrinsic assembly using `simd_ballot()` and
`simd_vote` APIs
## Changes
- **source/slang/hlsl.meta.slang**:
- Add Metal target case for `WaveGetActiveMask()` using
`simd_ballot(true)`
- Update capability requirements from `cuda_glsl_hlsl_spirv` to
`cuda_glsl_hlsl_metal_spirv` for wave ballot functions
- **source/slang/slang-capabilities.capdef**:
- Add `metal` to `subgroup_ballot_activemask` capability alias
|
|
There was a case where a PR passed CI test a long time ago, and when it
is merged, it caused a regression. We like to run via-glsl test when it
gets merged to prevent it.
|
|
Slang compiler doesn't use thread and we should declare the dependency
to the thread library when we don't need it.
The use of Thread is limited to the tools such as slang-test.
|
|
Enable CUDA support for batch 3 tests
- Enhanced wave operations with exclusive support
- Added proper identity values for min/max operations
- Fixed intrinsic name mapping issues
- Updated test configurations
Co-authored-by: Ellie Hermaszewska <ellieh@nvidia.com>
|
|
Commit bdda8a9 from PR #7862 had to disable some slangy tests in ci.yml
|
|
Added a note section under the Installation section that warns users
about potential conflicts when multiple Slang installations are present
on the system. The note specifically addresses:
* The scenario where Slang from Vulkan SDK might conflict with a
standalone installation
* How LD_LIBRARY_PATH on Linux overrides the RUNPATH in the slangc
executable
Closes https://github.com/shader-slang/slang/issues/7405
|
|
Don't include the VULKAN_HEADERS and SPIRV-Headers submodule if they are
already included.
Fix for the https://github.com/shader-slang/slang/issues/7898.
---------
Co-authored-by: Ellie Hermaszewska <ellieh@nvidia.com>
Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
Co-authored-by: Jay Kwak <82421531+jkwak-work@users.noreply.github.com>
|
|
In Metal, if `ParameterBlock` contains `DescriptorHandle` directly, it
would be emitted as DescriptorHandle literal, which is not valid Metal
code,
This fix adds a case for `kIROp_DescriptorHandleType` and directs it to
the Parent's `emitType` function to handle it.
|
|
#7644 added a script that gets run in CI to display a human-readable
diff if `source/slang/slang-ir-insts-stable-names.lua` needs to be
updated. However, the `git diff` commands in that script are wrong,
causing it to instead just display the man page for the `git diff`
command; [here's an
example](https://github.com/shader-slang/slang/actions/runs/16578560460/job/46888822691):
```
=== Updating stable names ===
Added 1 new instructions to source/slang/slang-ir-insts-stable-names.lua
=== Diff of changes made ===
usage: git diff --no-index [<options>] <path> <path>
Diff output format options
```
(followed by many lines)
This PR fixes the script. Here's an example showing it working correctly
after the fix:
-
https://github.com/shader-slang/slang/actions/runs/16578977233/job/46890240012
- https://github.com/shader-slang/slang/actions/runs/16578977233
Co-authored-by: Ellie Hermaszewska <ellieh@nvidia.com>
|
|
This is a followup on #7828 to fix bugs that were causing CodeLLDB to
give wrong values and hang (see vadimcn/codelldb#1302) because I didn't
realize that these data formatters can be passed _either_ a value of a
given type _or_ a pointer to a value of that type, and need to handle
both cases. I also introduced loop bounds to prevent hangs in the case
where these synthetic values are constructed for things like
uninitialized variables.
From looking at the preexisting data formatters from #4272 in
`source/core/core_lldb.py`, it seems like they _technically_ have
similar bugs to this, but since those types are simpler, it's unclear to
me whether that can actually manifest in meaningful ways like these bugs
in `source/slang/slang_lldb.py` were doing.
Anyways, to test this, put a breakpoint here:
https://github.com/shader-slang/slang/blob/6d399804a353154259cf4410940f144db8f9b5cf/source/slang/slang-emit-cpp.cpp#L1733
And use this `.vscode/launch.json` for CodeLLDB:
```json
{
"version": "0.2.0",
"configurations": [
{
"name": "LLDB",
"preLaunchTask": "Debug build",
"type": "lldb",
"request": "launch",
"initCommands": ["command source .lldbinit"],
"program": "build/Debug/bin/slangc",
"args": [
"tests/cpu-program/cpu-hello-world-test.slang",
"-target",
"executable",
"-o",
"hello"
]
}
]
}
```
Before this PR, the `inst` variable will display in the debug pane as
`{kIROp_StringLit 0x00007fffffff5f68}`, which is the wrong pointer
value. You can also check this by running `p inst` in the Debug Console,
which will print this:
```
(Slang::IRInst *) 0x000055555fdac3b8 {kIROp_StringLit 0x00007fffffff5f68}
```
In contrast, running `p *inst` prints the correct pointer value:
```
(Slang::IRInst) {kIROp_StringLit 0x000055555fdac3b8} {
[op] = kIROp_StringLit
[UID] = 76
[type] = 0x000055555fdac348 {kIROp_StringType None}
[decorations/children] = {}
[parent] = 0x000055555fdac2d0 {kIROp_ModuleInst None}
[uses] = 0x000055555fdadf18 {kIROp_StringLit 0x000055555fdac3b8}
}
```
But as you can see, in that case the synthetic `[value]` child is
completely missing.
Then if you try to expand `inst` in the debug pane, CodeLLDB will hang
(or at least it does when I try this).
After this PR, the hex integer for the pointer is always consistent, and
CodeLLDB does not hang in the debug pane when you expand `inst`, and
shows the correct `[value]` child just like when running `v *inst`.
As an aside: after this PR, the `[value]` child is still missing when
specifically running `p *inst` in the Debug Console. It _is_ possible to
fix this:
```diff
diff --git a/source/slang/slang_lldb.py b/source/slang/slang_lldb.py
index 23905d8c5..d2b3a4da9 100644
--- a/source/slang/slang_lldb.py
+++ b/source/slang/slang_lldb.py
@@ -93,13 +93,11 @@ class IRInst_synthetic(lldb.SBSyntheticValueProvider):
value: list[tuple[str, lldb.SBValue]] = []
match op.value:
case "kIROp_StringLit":
- string_lit_t = target.FindFirstType("Slang::IRStringLit")
- string_lit = self.valobj.Cast(string_lit_t)
+ string_lit = self.valobj.EvaluateExpression("(Slang::IRStringLit*)this")
val = string_lit.GetChildMemberWithName("value")
value = [("[value]", val.GetChildMemberWithName("stringVal"))]
case "kIROp_IntLit":
- int_lit_t = target.FindFirstType("Slang::IRIntLit")
- int_lit = self.valobj.Cast(int_lit_t)
+ int_lit = self.valobj.EvaluateExpression("(Slang::IRIntLit*)this")
val = int_lit.GetChildMemberWithName("value")
value = [("[value]", val.GetChildMemberWithName("intVal"))]
diff --git a/typings/lldb.pyi b/typings/lldb.pyi
index 2672ba244..3a08e9141 100644
--- a/typings/lldb.pyi
+++ b/typings/lldb.pyi
@@ -496,7 +496,7 @@ class SBValue:
def Persist(self): ...
def GetDescription(self, description): ...
def GetExpressionPath(self, *args): ...
- def EvaluateExpression(self, *args): ...
+ def EvaluateExpression(self, expr: str) -> SBValue: ...
def Watch(self, *args): ...
def WatchPointee(self, resolve_location, read, write, error): ...
def GetVTable(self): ...
```
However, that makes the debugger run _significantly_ slower, so I'm
choosing not do do it here.
---------
Co-authored-by: Ellie Hermaszewska <ellieh@nvidia.com>
|
|
The documentation added by #6844 included instructions to make sure that
the Fiddle `#include` in a file comes after all the other `#include`s,
but it's easy to accidentally violate this via `clang-format`, as
happened for `source/slang/slang-ast-modifier.h` in #7559. This PR
guards against this sort of violation by separating all Fiddle
`#include`s from other `#include`s via a blank line followed by a `//`
line (as we already do in most cases), and also adds a sentence about
this in `tools/slang-fiddle/README.md`.
As a bonus, I also enabled Markdown syntax highlighting for all the code
blocks in that doc file.
Co-authored-by: Ellie Hermaszewska <ellieh@nvidia.com>
|
|
Close #8054.
For detailed root cause is at:
https://github.com/shader-slang/slang/issues/8054#issuecomment-3189579508
|
|
Close #8090.
When we do type coerce, we use a cache to store the conversion cost
of different type. The key of the cache is defined by
struct BasicTypeKey
{
uint32_t baseType : 8;
uint32_t dim1 : 4;
uint32_t dim2 : 4;
...
}
where dim1 and dim2 is used for dimension of vector and matrix.
However the dim is only 4 bits, so `vector<int, 16>` will have the same
key as `int`, which is wrong.
Fix the issue by extending it to 8 bit.
Also to make the hash key still within 32 bits, we adjust baseType to 5 bits,
and knownConstantBitCount to 6 bits.
---------
Co-authored-by: kaizhangNV <kazhang@nvidia.com>
|
|
Similar to #7887, this PR improves the Fiddle docs a bit by showing how
the `FIDDLE TEMPLATE` example would actually need to include `FIDDLE
END` at some point after `FIDDLE OUTPUT`.
|
|
|
|
This file is automatically overwritten by the build:
https://github.com/shader-slang/slang/blob/b7df3c7aa27301f88e31ed0a7bbf230688adab6a/source/slang/CMakeLists.txt#L68-L78
It is currently out of date (running the build gives rise to unstaged
changes), so this PR updates it.
|
|
Update cuda context creation to support both cuda 12 and cuda 13.
|
|
Add helper functions to create ISlangBlob and load module data from
source.
---------
Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
|
|
Metal's popcount prototype is `T popcount(T x)` but we want to use it to
implement `countbits` where the prototype always returns `uint`.
Using `popcount` directly would implicitly cast successfully to the
32-bit return value in all cases except when the argument is a 64-bit
type. Thus, this change always explicitly casts the result to `$TR`,
which should be one of the `uint[N]` types, and should always be able to
hold the number of bits in the type.
Addresses #6877
|
|
logic (#8168)
Fixes: #8167
Current emitting logic does not work, this has been corrected.
The provided test ensures our CUDA code is valid by compiling PTX from
it.
`m_writer->emit("OptixTraversableHandle");` should be `out <<` since
`out` adds to type-name-cache; otherwise using a type twice will produce
bad type-names (since we filled type-name cache with "" instead of
"typeName")
|
|
Fixes #7011
|
|
debugging (#8192)
fixes: #8188
Changes:
* Fix Indentation
* Add a visualizer for `NodeBase` based on changes to `slang-fiddle`
---------
Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
|
|
AST-parent has target+set the AST-child does not (#8175)
Fixes: #8174
Changes:
* To determine if we propagate capabilities, we need to ensure that a
`join` will do nothing (optimization since `join` is expensive + caching
data for the `join` adds up to be expensive). This logic was changed in
`slang-check-decl.cpp` since the current logic was incorrect.
* A parent could have the set `metal+glsl` and the use-site could have
`glsl`. In this case, we will not remove `metal` from the parent since
`{metal+glsl}.implies({glsl})` is true.
---------
Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
|
|
|
|
Fixes #6785
|
|
#1729 renamed `Val::_toStringOverride` to `Val::_toTextOverride` but did
not update the error message for when it is not overridden. This PR
fixes that.
|
|
When we legalize the entry point param, there are cases where we need to
reconstruct a struct for the parameter and the original struct wouldn't
be used. But if the user tries to use the origianl struct as a type for
a function parameter, we will end up using both the original struct and
the synthesized struct at the same time.
On Metal and WGSL, it causes an error when an identical semtaic is used
on more than one variable.
This commit removes the semantics from the original struct after cloning
the type.
Fixes https://github.com/shader-slang/slang/issues/8141
Related to https://github.com/shader-slang/slang/issues/7693
---------
Co-authored-by: ArielG-NV <159081215+ArielG-NV@users.noreply.github.com>
|
|
Currently the issue template makes the subtitle "bold". It is a little
hard to navigate between the subtitle because the bold is not very
obvious.
This commit makes them to use a bigger font.
Before the change,
<img width="839" height="488" alt="image"
src="https://github.com/user-attachments/assets/08abec42-8389-4404-89a1-5a70670000cd"
/>
After the change,
<img width="840" height="544" alt="image"
src="https://github.com/user-attachments/assets/e2ffdac1-f8dc-40fa-a317-8dda965f3377"
/>
|
|
Enable CUDA for the tests listed in issue #8078
This requires a minor CUDA prelude change, adding some math functions.
|
|
This PR puts the [Khronos official Vulkan
Loader](https://github.com/NixOS/nixpkgs/blob/6027c30c8e9810896b92429f0092f624f7b1aace/pkgs/by-name/vu/vulkan-loader/package.nix#L22-L27)
in `LD_LIBRARY_PATH` for the `flake.nix` dev shell, allowing Vulkan
tests to run.
For example, before this PR:
```
$ build/Debug/bin/slang-test tests/compute/array-param.slang
Supported backends: glslang spirv-dis clang gcc genericcpp llvm spirv-opt
found test: 'tests/compute/array-param.slang'
Check vk,vulkan: Not Supported
Check cuda: Not Supported
ignored test: 'tests/compute/array-param.slang (cuda)'
passed test: 'tests/compute/array-param.slang.1 (cpu)'
ignored test: 'tests/compute/array-param.slang.2 (dx11)'
ignored test: 'tests/compute/array-param.slang.3 (dx12)'
ignored test: 'tests/compute/array-param.slang.4 (vk)'
===
100% of tests passed (1/1), 4 tests ignored
===
```
In contrast, after this PR (on an Ubuntu machine with an NVIDIA GPU):
```
$ nixGLNvidia build/Debug/bin/slang-test tests/compute/array-param.slang
Supported backends: glslang spirv-dis clang gcc genericcpp llvm spirv-opt
found test: 'tests/compute/array-param.slang'
Check vk,vulkan: Supported
Check cuda: Not Supported
ignored test: 'tests/compute/array-param.slang (cuda)'
passed test: 'tests/compute/array-param.slang.1 (cpu)'
ignored test: 'tests/compute/array-param.slang.2 (dx11)'
ignored test: 'tests/compute/array-param.slang.3 (dx12)'
passed test: 'tests/compute/array-param.slang.4 (vk)'
===
100% of tests passed (2/2), 3 tests ignored
===
```
|
|
Related to #7969
In the slangc help text we have two categories, the option category
`Target` and the values category `target`, where the names only differ
by case. The help parser finds the category in the list by the
case-insensitive name, so `slangc -h Target` and `slangc -h target` will
both print the `Target` help text and never the `target` help text.
This commit replaces the case-insensitive name search with a
case-sensitive one, and then only if the case-sensitive check fails do
we do a case-insensitive search.
That way, if the user uses `Target` they get the `Target` text, for
`target` they get the `target` text. If they use something like
`Capability`, a category that does not exist but whose name is
upper-case of the value category `capability`, they will still get the
help text for `capability` as before as a fallback.
---------
Co-authored-by: slangbot <ellieh+slangbot@nvidia.com>
Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
|
|
from API (#8119)
Closes #8110.
Closes #8011.
|
|
Co-authored-by: Jay Kwak <82421531+jkwak-work@users.noreply.github.com>
Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
|
|
Fixes https://github.com/shader-slang/slang/issues/8098
|
|
Fixes #8116
---------
Co-authored-by: Jay Kwak <82421531+jkwak-work@users.noreply.github.com>
|
|
Co-authored-by: Jay Kwak <82421531+jkwak-work@users.noreply.github.com>
|
|
What is fixed:
Since we make `vk` tests compile as GLSL, we must use a capability that
specifies a GLSL equivalent.
If we do not do this, we will get an error since we are not specifying
GLSL capabilities (but are specifying a profile).
Since a profile is specified, we emit capability errors.
|
|
Fixes: #7410
Changes:
1. super-type capabilities must be a super-set of sub-type capabilities
(and support the same shader stages/targets)
* InheritanceDecl visits super-type to inherit it's capabilities;
validate InheritanceDecl capabilities against sub-type
* visit all container decl's with a default case
* clean up functionDeclBase visitor
* Simplify `diagnoseUndeclaredCapability` by moving logic into
capability checking (more correct*)
3. added changed behavior to documentation
4. fixed some incorrect capabilities
5. **we do not** diagnose capability errors on interface
requirement-to-implementation if both lack explicit capability
requirements. This change is to work around a slangpy regression (test
case for the failing situation is in
`tests\language-feature\capability\capability-interface-extension-1.slang`),
Note: maybe for slang-2026 we don't do this?
6. requirement & implementation must support the same shader
stage/target. This was changed because otherwise we can have cases where
`X` inherits from `Y`, but `Y` is only expected to be used in `glsl`
whilst `X` is expected to be used in `hlsl | glsl`
7. removed
`tests/language-feature/capability/capabilitySimplification3.slang`
because it tests nothing special (redundant)
Note: not using rebase due to separate branches depending on this PR
---------
Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
|
|
when we have `GetElementPtr -> load -> GetElement` in our use-chain, the
final `GetElement` may use the `load` as a `Index`, not a base.
This is a non-issue with `getFieldExtract` since a field is a StructKey.
We will still add this check to ensure no bugs down the line.
---------
Co-authored-by: Harsh Aggarwal <haaggarwal@nvidia.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: slangbot <ellieh+slangbot@nvidia.com>
Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
|
|
Closes https://github.com/shader-slang/slang/issues/5750
|
|
When using Slang reflection API to find functions by name in a type,
`FunctionReflection::getName()` would return `nullptr` for overloaded
functions instead of the expected function name.
The issue occurred in `spReflectionFunction_GetName` when
`findFunctionByNameInType` returned a `SlangReflectionFunction` wrapping
an `OverloadedExpr` (for overloaded functions) instead of a single
`DeclRef<FunctionDeclBase>`. The `convertToFunc()` function would fail
for `OverloadedExpr` objects, causing `getName()` to return `nullptr`.
**Example of the bug:**
```cpp
// This code would fail before the fix
public interface IBase {
public void step(inout float f);
}
public struct Impl : IBase {
public void step(inout float f) { f += 1.0f; }
}
// Using reflection API:
auto implType = reflection->findTypeByName("Impl");
auto stepFunction = reflection->findFunctionByNameInType(implType, "step");
auto name = stepFunction->getName(); // Would return nullptr
```
**Fix:**
Modified `spReflectionFunction_GetName` to handle overloaded functions
by falling back to the name of the first overload candidate when
`convertToFunc` fails. This follows the same pattern already used by
`spReflectionFunction_specializeWithArgTypes`.
The fix ensures that:
- Overloaded function containers return the correct function name
- Individual overload candidates also return their names correctly
- Non-overloaded functions continue to work as before
- No regression in existing functionality
**Testing:**
Added comprehensive test cases covering both the original issue scenario
and explicit function overloading. All existing reflection tests
continue to pass.
Fixes #8047.
<!-- START COPILOT CODING AGENT TIPS -->
---
💡 You can make Copilot smarter by setting up custom instructions,
customizing its development environment and configuring Model Context
Protocol (MCP) servers. Learn more [Copilot coding agent
tips](https://gh.io/copilot-coding-agent-tips) in the docs.
---------
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 <yonghe@outlook.com>
Co-authored-by: ArielG-NV <159081215+ArielG-NV@users.noreply.github.com>
Co-authored-by: slangbot <ellieh+slangbot@nvidia.com>
Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
|
|
Due to an older version of spec referred there was an inconsitency v1.29
2/20/2025 - [HitObject LoadLocalRootArgumentsConstant]
Latest spec
https://microsoft.github.io/DirectX-Specs/d3d/Raytracing.html#hitobject-loadlocalroottableconstant
Refer:
OptiX backend support for Shader Execution Reordering (SER) features as
outlined in issue #6647. -
|
|
Full set of mutually exclusive choices for upgrading LLVM:
- #8031 (you are here)
- #8035
- #8036
- #8034
- #8038
- #8039
- #8033
Alternative to #8028. Required some minor changes due to these upstream
commits:
- llvm/llvm-project@e463b69736da8b0a950ecd937cf990401bdfcdeb
- llvm/llvm-project@89b57061f7b769e9ea9bf6ed686e284f3e55affe
|
|
Closes #8061.
Along with the fix, also enhanced coercion/overload resolution to filter
candidates based on the target type, allowing
`tests\language-feature\higher-order-functions\overloaded.slang` to
pass.
|
|
Fixes #7479
This adds a new variant of `_getName(ExtensionDecl* decl)` to our
markdown doc writer that takes an ExtensionDecl* and returns the name in
the format `extension <name> : <interface>`.
This is required to display "extension T : ITexelElement" properly in
the core module docs, as the existing `_getName(Decl* decl)` returns an
empty string because the name does not come from the `decl` itself, but
rather its `targetType`.
The target type alone is not enough, as that would return `T`, which
will be erroneously interpreted as the name for the generic parameter,
and the doc system will link every mention of `T` to the extension's
page.
ReadTheDocs already displays the name correctly in the TOC of the docs
there, but that is because it falls back to the page title when the name
in the TOC is empty.
---------
Co-authored-by: slangbot <ellieh+slangbot@nvidia.com>
Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
|
|
expressions in legacy mode (#7984)
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.
<!-- START COPILOT CODING AGENT TIPS -->
---
💬 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 <ellieh+slangbot@nvidia.com>
Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
Co-authored-by: aidanfnv <aidanf@nvidia.com>
Co-authored-by: csyonghe <2652293+csyonghe@users.noreply.github.com>
|
