-
Notifications
You must be signed in to change notification settings - Fork 377
Return SLANG_UNKNOWN_SIZE or SLANG_UNBOUNDED_SIZE for all type layout api functions where appropriate #9005
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
base: master
Are you sure you want to change the base?
Conversation
jkwak-work
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
|
/format |
|
🌈 Formatted, please merge the changes from this PR |
- Fixed multiplication and comparison operators in various files - Updated getFiniteValue() calls to handle LayoutOffset return type - Added proper invalid state handling in IR generation - Fixed reflection API to handle invalid layout sizes safely
- Fixed comparison operators in slang-parameter-binding.cpp and slang-type-layout.cpp - Updated all uses of size == 0 and count == 0 to use proper compare functions - Fixed _roundToAlignment to handle LayoutOffset properly and check for invalid states - Removed ad-hoc special values in favor of proper invalid state handling
- Fixed all LayoutOffset conversion issues in slang-parameter-binding.cpp - Applied safe pattern of checking isValid() before using getValidValue() - Updated all remaining comparison operators to use compare() function - Handled invalid states properly in string formatting and allocation
This commit completes the implementation of invalid state handling for LayoutSize, matching the pattern used in LayoutOffset. Key changes include: - Fixed all remaining compilation errors from the API changes - Updated getFiniteValue() method to be inline to avoid linking issues - Applied safe error handling patterns throughout the codebase - Ensured invalid states propagate correctly through all operations - Replaced all ad-hoc comparisons with proper compare() function calls - Fixed arithmetic operations between LayoutSize and LayoutOffset types The implementation now provides type-safe layout size calculations with proper error propagation and eliminates the use of magic values.
Fixed GetElementCount() function to return LayoutSize::invalid() instead of magic value 0 in cases where the element count cannot be determined: - Generic parameter bounds - Polynomial integer values - Function call integer values This ensures proper error propagation through the layout system and eliminates the use of magic values in favor of typed invalid states.
60e573c to
d25098d
Compare
|
/format |
|
🌈 Formatted, please merge the changes from this PR |
|
/format |
|
🌈 Formatted, please merge the changes from this PR |
00576f4 to
cc21d0d
Compare
cc21d0d to
a0e39df
Compare
|
/format |
|
🌈 Formatted, please merge the changes from this PR |
Format code for PR shader-slang#9005
jkwak-work
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
But please fill in the details of the PR description.
Because this will be a breaking change, the description will show up at the top of the release note.
People will need to also know how to fix their code if it actually breaks.
| // only useful if `getKind() == Kind::Array` | ||
| /** Get the number of elements in an array or vector type. | ||
| * | ||
| * Only useful if `getKind() == Kind::Array` or `Kind::Vector`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we have an assertion check for this case?
| value |= value >> 4; | ||
| value |= value >> 8; | ||
| value |= value >> 16; | ||
| if (sizeof(size_t) > 4) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some compilers may warn to use if constexpr
Closes #8964