Skip to content
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

[0010] Allow casting to uint64_t #134

Merged
merged 3 commits into from
Dec 8, 2023
Merged

Conversation

s-perron
Copy link
Collaborator

@s-perron s-perron commented Dec 1, 2023

We have decided to allow casting to uint64_t. It will make create
more opportunities for developers to make mistakes, but they do not
have to use it. The implementation cost does not seem too high.

Fixes #93

@s-perron
Copy link
Collaborator Author

s-perron commented Dec 1, 2023

@greg-lunarg FYI

@s-perron s-perron marked this pull request as ready for review December 4, 2023 19:47
@greg-lunarg
Copy link
Contributor

I am ok with adding this to the current design.

Do we want to eliminate the cast to bool to test for null pointer as this new capability lets us do this with a comparison to zero? This feels a little cleaner to me and removes the ambiguity around what might constitute a null pointer.

@s-perron
Copy link
Collaborator Author

s-perron commented Dec 5, 2023

I was considering removing the cast to bool too. I can do that.

We have decided to allow casting to uint64_t. It will make create
more opportunities for developers to make mistakes, but they do not
have to use it. The implementation cost does not seem too high.

Fixes microsoft#93
As Greg suggested, the cast to bool is no longer needed if we can cast
to an int, so we removed it. This commit also adds the uint64_t cast to
the pseudo class definition for BufferPointers.
@s-perron
Copy link
Collaborator Author

s-perron commented Dec 7, 2023

I rebased, and updated the linked list example now that #108 has been merged.

@s-perron
Copy link
Collaborator Author

s-perron commented Dec 8, 2023

@llvm-beanz Do you have any concerns?

Copy link
Collaborator

@llvm-beanz llvm-beanz left a comment

Choose a reason for hiding this comment

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

LGTM. This is also probably more consistent with HLSL's somewhat magic "flat conversion" casting rules.

@s-perron s-perron merged commit 7098ff0 into microsoft:main Dec 8, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Accepted
Development

Successfully merging this pull request may close these issues.

[0010] Method to obtain uint64_t from BufferPointer
4 participants