Skip to content

Fix #6544: Properly format nested type names in extensions #6769

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 24, 2025

Conversation

szihs
Copy link
Contributor

@szihs szihs commented Apr 9, 2025

Modify DeclRefBase::toText to properly handle types defined in extensions by qualifying them with their parent type name. This ensures getFullName() returns the full name like 'FullPrecisionOptimizer.State' instead of just '.State'.

Also add diagnostic tests for the same

@szihs szihs requested a review from Copilot April 9, 2025 14:28
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot wasn't able to review any files in this pull request.

Files not reviewed (1)
  • source/slang/slang-ast-type.cpp: Language not supported

@szihs szihs added the pr: non-breaking PRs without breaking changes label Apr 9, 2025
@szihs szihs force-pushed the haaggarwal/fix-6544-declref-extn branch 3 times, most recently from 01f572b to 8f85ffd Compare April 11, 2025 14:10
@szihs szihs self-assigned this Apr 11, 2025
@szihs szihs marked this pull request as ready for review April 11, 2025 15:36
@szihs szihs requested a review from a team as a code owner April 11, 2025 15:36
@szihs szihs linked an issue Apr 11, 2025 that may be closed by this pull request
kaizhangNV
kaizhangNV previously approved these changes Apr 11, 2025
@kaizhangNV
Copy link
Contributor

Is there an issue number associated with this?

@szihs
Copy link
Contributor Author

szihs commented Apr 12, 2025

Is there an issue number associated with this?

#6544

This would require update of pypi slangpy / nv-sgl and new release of slang repo for the dlls/libs to get picked @kaizhangNV

Copy link
Collaborator

@csyonghe csyonghe left a comment

Choose a reason for hiding this comment

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

Requesting changes since I think it is incorrect to drop declref info in toText.

szihs added 2 commits April 16, 2025 20:48
Modify DeclRefBase::toText to properly handle types defined
in extensions by qualifying them with their parent type name.
This ensures getFullName() returns the full name like
'FullPrecisionOptimizer<half>.State' instead of just '.State'.
Also handle other nested types in structs/classes similarly.
- stopping namespace inclusion for extension members
- Update to use getTargetType() to handle the generic arguments
- update test cases
@szihs szihs force-pushed the haaggarwal/fix-6544-declref-extn branch from 8f85ffd to 3093c2a Compare April 16, 2025 15:19
@szihs szihs requested a review from csyonghe April 16, 2025 15:22
@szihs szihs requested review from expipiplus1 and csyonghe April 21, 2025 07:54
Copy link
Collaborator

@expipiplus1 expipiplus1 left a comment

Choose a reason for hiding this comment

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

Might be nice in future to split up really large tests like this so that each file tests exactly one thing

@szihs szihs enabled auto-merge (squash) April 24, 2025 03:41
@szihs szihs dismissed csyonghe’s stale review April 24, 2025 03:57

Updated with the changes.

@szihs szihs merged commit b78a8ba into shader-slang:master Apr 24, 2025
16 checks passed
szihs added a commit to szihs/slang that referenced this pull request May 7, 2025
shader-slang#6769)

* Fix shader-slang#6544: Properly format nested type names in extensions

Modify DeclRefBase::toText to properly handle types defined
in extensions by qualifying them with their parent type name.
This ensures getFullName() returns the full name like
'FullPrecisionOptimizer<half>.State' instead of just '.State'.
Also handle other nested types in structs/classes similarly.

* Update extension reflection handling - with generics args and namespaces

- stopping namespace inclusion for extension members
- Update to use getTargetType() to handle the generic arguments
- update test cases

* Simplify code to remove using parentDecl
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: non-breaking PRs without breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

getFullName on type defined in extension returns incorrect string
4 participants