Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Aug 7, 2025

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:

// 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.


💡 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 in the docs.

Copilot AI and others added 2 commits August 7, 2025 19:43
Co-authored-by: csyonghe <2652293+csyonghe@users.noreply.github.com>
Co-authored-by: csyonghe <2652293+csyonghe@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] Slang reflection not returning function name Fix FunctionReflection getName() returning null for overloaded functions Aug 7, 2025
@Copilot Copilot AI requested a review from csyonghe August 7, 2025 19:46
Copilot finished work on behalf of csyonghe August 7, 2025 19:46
@@ -0,0 +1,90 @@
// unit-test-find-function-by-name-in-type-overloaded.cpp
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to add a new test file, just amend unit-test-function-lookup-resolution.cpp with a SLANG_CHECK() to call getName on func1.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Removed the separate test files and added SLANG_CHECK() calls to verify getName() works on func1 in the existing unit-test-function-lookup-resolution.cpp. The test passes successfully. (commit: 9934ebd)

@csyonghe
Copy link
Collaborator

csyonghe commented Aug 7, 2025

@copilot

Co-authored-by: csyonghe <2652293+csyonghe@users.noreply.github.com>
@Copilot Copilot AI requested a review from csyonghe August 7, 2025 22:25
Copilot finished work on behalf of csyonghe August 7, 2025 22:25
@csyonghe csyonghe marked this pull request as ready for review August 7, 2025 22:30
@csyonghe csyonghe requested a review from a team as a code owner August 7, 2025 22:30
@csyonghe csyonghe added the pr: non-breaking PRs without breaking changes label Aug 7, 2025
@csyonghe
Copy link
Collaborator

csyonghe commented Aug 7, 2025

@tdavidovicNV for viz.

@ArielG-NV can you review?

@ArielG-NV
Copy link
Collaborator

I'll review the PR right away.

ArielG-NV
ArielG-NV previously approved these changes Aug 7, 2025
Copy link
Collaborator

@ArielG-NV ArielG-NV left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 suggestion for code-styling

Changes look good.

Co-authored-by: ArielG-NV <159081215+ArielG-NV@users.noreply.github.com>
ArielG-NV
ArielG-NV previously approved these changes Aug 7, 2025
Copy link
Collaborator

@ArielG-NV ArielG-NV left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re-approval

@csyonghe
Copy link
Collaborator

csyonghe commented Aug 7, 2025

/format

@slangbot
Copy link
Contributor

slangbot commented Aug 7, 2025

🌈 Formatted, please merge the changes from this PR

Automated code formatting for
#8113

Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
Copy link
Collaborator

@ArielG-NV ArielG-NV left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reapproval

@csyonghe csyonghe enabled auto-merge August 7, 2025 23:38
@csyonghe csyonghe added this pull request to the merge queue Aug 8, 2025
Merged via the queue into master with commit c7c4816 Aug 8, 2025
18 checks passed
szihs pushed a commit to szihs/slang that referenced this pull request Aug 19, 2025
…ons (shader-slang#8113)

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 shader-slang#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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CoPilot pr: non-breaking PRs without breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Slang reflection not returning function name
4 participants