-
Notifications
You must be signed in to change notification settings - Fork 379
Return slang_unknown_size from api where appropriate #9070
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
Conversation
- 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.
|
/format |
|
🌈 Formatted, please merge the changes from this PR |
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 can we have test cases?
| else | ||
| { | ||
| // Invalid size - use a different sentinel value | ||
| sizeValue = IRIntegerValue(-2); |
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.
The use of magic numbers in this function is a bit confusing.
In slang.h, there are two exceptions,
#define SLANG_UNBOUNDED_SIZE (~size_t(0))
#define SLANG_UNKNOWN_SIZE (~size_t(0) - 1)
Can we use those macros instead of hard-coding the numbers?
And I am not sure if it is necessary to introduce a new exception with -2 here.
It might be fine with 0 instead?
| range.begin = begin; | ||
| range.end = end.isFinite() ? end.getFiniteValue() : UInt(-1); | ||
| auto endOffset = end.getFiniteValue(); | ||
| range.end = end.isFinite() && endOffset.isValid() ? endOffset.getValidValue() : UInt(-1); |
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.
Let's make the intention a bit more clear with parenthesis.
range.end = ((end.isFinite() && endOffset.isValid()) ? endOffset.getValidValue() : UInt(-1));
With the parenthesis, It could be seen as a mistake of,
range.end = end.isFinite() && (endOffset.isValid() ? endOffset.getValidValue() : UInt(-1));
Automated code formatting for #9070 Co-authored-by: slangbot <[email protected]>
Closes #8965, depends on #9005